-
-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PR notifications #1146
base: develop
Are you sure you want to change the base?
PR notifications #1146
Conversation
Quality Gate failedFailed conditions |
application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java
Show resolved
Hide resolved
|
||
try { | ||
if (!isRepositoryAccessible(github, repositoryOwner, repositoryName)) { | ||
event.reply("Repository is not publicly available.").setEphemeral(true).queue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opt to provide the owner and name in this error message. It could have been they made a typo. Maybe include a link to the docs on how they can set the visibility too.
} | ||
} catch (IOException e) { | ||
logger.error("Failed to check if GitHub repository is available.", e); | ||
event.reply("Failed to link repository.").setEphemeral(true).queue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, opt for the user friendly message. Make it a constant above since it can be reused.
|
||
try { | ||
saveNotificationToDatabase(channelId, repositoryOwner, repositoryName); | ||
event.reply("Successfully linked repository.").setEphemeral(true).queue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this response a pretty embed for everyone in the project thread to see.
...ion/src/main/java/org/togetherjava/tjbot/features/github/PullRequestNotificationRoutine.java
Show resolved
Hide resolved
...ion/src/main/java/org/togetherjava/tjbot/features/github/PullRequestNotificationRoutine.java
Show resolved
Hide resolved
logger.info("Failed to find channel {} to send pull request notification.", channelId); | ||
return; | ||
} | ||
channel.sendMessage("New pull request from " + pr.getUser().getLogin() + ".").queue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this a pretty embed. We're missing key details here such as the PR name, description, number, creation date and URL.
application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java
Show resolved
Hide resolved
event.reply("Successfully linked repository.").setEphemeral(true).queue(); | ||
} catch (DatabaseException e) { | ||
logger.error("Failed to save pull request notification to database.", e); | ||
event.reply("Failed to link repository.").setEphemeral(true).queue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better error message please
About
closes #1142
Implements the functionality for PR notifications
link-gh-project
: links a channel to GitHub repositories to announce new pull requestunlink-gh-project
: unlinks a previous pull request notification configurationThe
PullRequestNotificationRoutine
handles pulling the new updates from repositories and sending a notification.Database
This PR creates a new database table to store the notification configuration: