PR notifications #1146

SquidXTV · 2024-07-31T18:12:53Z

About

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
)

sonarqubecloud · 2024-07-31T18:16:13Z

Quality Gate failed

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

See analysis details on SonarCloud

surajkumar · 2024-07-31T18:24:26Z

application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java

+        try {
+            github = new GitHubBuilder().withOAuthToken(githubApiKey).build();
+        } catch (IOException e) {
+            logger.error("Failed to initialize GitHub API wrapper.", e);


The error message, since duplicate should be stored in a String

surajkumar · 2024-07-31T18:25:26Z

application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java

+            github = new GitHubBuilder().withOAuthToken(githubApiKey).build();
+        } catch (IOException e) {
+            logger.error("Failed to initialize GitHub API wrapper.", e);
+            event.reply("Failed to initialize GitHub API wrapper.").setEphemeral(true).queue();


Actually, this isn't a meaningful error message to the user. Instead opt for something user friendly e.g. "An internal error occurred while linking your repository, please try again later"

surajkumar · 2024-07-31T18:26:29Z

application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java

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


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.

surajkumar · 2024-07-31T18:27:13Z

application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java

+            }
+        } catch (IOException e) {
+            logger.error("Failed to check if GitHub repository is available.", e);
+            event.reply("Failed to link repository.").setEphemeral(true).queue();


Same here, opt for the user friendly message. Make it a constant above since it can be reused.

surajkumar · 2024-07-31T18:27:36Z

application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java

+
+        try {
+            saveNotificationToDatabase(channelId, repositoryOwner, repositoryName);
+            event.reply("Successfully linked repository.").setEphemeral(true).queue();


Let's make this response a pretty embed for everyone in the project thread to see.

surajkumar · 2024-07-31T18:36:06Z

...ion/src/main/java/org/togetherjava/tjbot/features/github/PullRequestNotificationRoutine.java

+            }
+        }
+
+        lastExecution = new Date();


System.currentTimeMillis() please to avoid creating a new Date object each time

surajkumar · 2024-07-31T18:36:53Z

...ion/src/main/java/org/togetherjava/tjbot/features/github/PullRequestNotificationRoutine.java

+
+                List<GHPullRequest> pullRequests = repository.getPullRequests(GHIssueState.OPEN);
+                for (GHPullRequest pr : pullRequests) {
+                    if (pr.getCreatedAt().after(lastExecution)) {


pr.getCreatedAt() > lastExecution

surajkumar · 2024-07-31T18:38:09Z

...ion/src/main/java/org/togetherjava/tjbot/features/github/PullRequestNotificationRoutine.java

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


Let's make this a pretty embed. We're missing key details here such as the PR name, description, number, creation date and URL.

surajkumar · 2024-07-31T18:40:49Z

application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java

+
+        GitHub github;
+        try {
+            github = new GitHubBuilder().withOAuthToken(githubApiKey).build();


Let's move this into the constructor and log. So we can fail fast. Or even better, do the init inside Features.java and only then bind the routine/commands if we can successfully create a GitHub object.

surajkumar · 2024-07-31T18:42:02Z

application/src/main/java/org/togetherjava/tjbot/features/github/GitHubLinkCommand.java

+            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();


Better error message please

sonarqubecloud · 2025-07-01T13:24:58Z

Quality Gate failed

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

See analysis details on SonarQube Cloud

SquidXTV · 2025-07-03T08:09:27Z

Closing this PR.
GitHub already supports native Discord webhook notifications through the GitHub repo settings, which makes this kinda redundant. The feature also isn't critical and likely wouldn't be used much in practice (probably).

The webhook alternative approach involves sharing the sensitive webhook link with a third party. My suggestion would be sharing them with very active #projects posts and trusted users if needed. If anything goes wrong, we can just disable the webhook.

Quick note:
When sharing webhook links in forum-based threads, you need to append a thread_id query parameter to the webhook link: https://discord.com/developers/docs/resources/webhook#execute-webhook

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

surajkumar suggested changes Jul 31, 2024

View reviewed changes

SquidXTV self-assigned this Jul 1, 2025

Zabuzard added the priority: normal label Jul 1, 2025

Implement PR notifications for channels

1e1717b

SquidXTV force-pushed the projects-pr-notifications branch from 20b0898 to 1e1717b Compare July 1, 2025 13:21

SquidXTV closed this Jul 3, 2025

SquidXTV mentioned this pull request Jul 3, 2025

Sending PR notifications to project channels #1142

Closed

SquidXTV deleted the projects-pr-notifications branch July 3, 2025 08:19

Uh oh!

PR notifications #1146

PR notifications #1146

Uh oh!

Conversation

SquidXTV commented Jul 31, 2024

About

Database

Uh oh!

sonarqubecloud bot commented Jul 31, 2024

Quality Gate failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jul 1, 2025

Quality Gate failed

Uh oh!

SquidXTV commented Jul 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants