Skip to content
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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

PR notifications #1146

wants to merge 1 commit into from

Conversation

SquidXTV
Copy link
Member

About

closes #1142

Implements the functionality for PR notifications
link-gh-project: links a channel to GitHub repositories to announce new pull request
unlink-gh-project: unlinks a previous pull request notification configuration

The 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:

CREATE TABLE pr_notifications
(
    id INTEGER   NOT NULL PRIMARY KEY AUTOINCREMENT,
    channel_id BIGINT NOT NULL,
    repository_owner TEXT NOT NULL,
    repository_name TEXT NOT NULL
)

@SquidXTV SquidXTV requested a review from a team as a code owner July 31, 2024 18:12
Copy link

sonarcloud bot commented Jul 31, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
13.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud


try {
if (!isRepositoryAccessible(github, repositoryOwner, repositoryName)) {
event.reply("Repository is not publicly available.").setEphemeral(true).queue();
Copy link
Contributor

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();
Copy link
Contributor

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();
Copy link
Contributor

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.

logger.info("Failed to find channel {} to send pull request notification.", channelId);
return;
}
channel.sendMessage("New pull request from " + pr.getUser().getLogin() + ".").queue();
Copy link
Contributor

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.

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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better error message please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sending PR notifications to project channels
2 participants