PR notifications #1146
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| package org.togetherjava.tjbot.features.github; | ||
|
|
||
| import net.dv8tion.jda.api.events.interaction.command.SlashCommandInteractionEvent; | ||
| import net.dv8tion.jda.api.interactions.commands.OptionMapping; | ||
| import net.dv8tion.jda.api.interactions.commands.OptionType; | ||
| import org.kohsuke.github.GHRepository; | ||
| import org.kohsuke.github.GitHub; | ||
| import org.kohsuke.github.GitHubBuilder; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import org.togetherjava.tjbot.config.Config; | ||
| import org.togetherjava.tjbot.db.Database; | ||
| import org.togetherjava.tjbot.db.DatabaseException; | ||
| import org.togetherjava.tjbot.db.generated.tables.PrNotifications; | ||
| import org.togetherjava.tjbot.db.generated.tables.records.PrNotificationsRecord; | ||
| import org.togetherjava.tjbot.features.CommandVisibility; | ||
| import org.togetherjava.tjbot.features.SlashCommandAdapter; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * Slash command used to link a GitHub project to a discord channel to post pull request | ||
| * notifications. | ||
| */ | ||
| public class GitHubLinkCommand extends SlashCommandAdapter { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(GitHubLinkCommand.class); | ||
|
|
||
| private static final String REPOSITORY_OWNER_OPTION = "owner"; | ||
| private static final String REPOSITORY_NAME_OPTION = "name"; | ||
|
|
||
| private final Database database; | ||
| private final String githubApiKey; | ||
|
|
||
| /** | ||
| * Creates new GitHub link command. | ||
| * | ||
| * @param database the database to store the new linked pull request notifications | ||
| * @param config the config to get the GitHub API key | ||
| */ | ||
| public GitHubLinkCommand(Database database, Config config) { | ||
| super("link-gh-project", | ||
| "Links a GitHub repository to this project post to receive pull request notifications", | ||
| CommandVisibility.GUILD); | ||
| this.database = database; | ||
| this.githubApiKey = config.getGitHubApiKey(); | ||
|
|
||
| getData() | ||
| .addOption(OptionType.STRING, REPOSITORY_OWNER_OPTION, | ||
| "The owner of the repository to be linked", true) | ||
| .addOption(OptionType.STRING, REPOSITORY_NAME_OPTION, | ||
| "The name of the repository to be linked", true); | ||
| } | ||
|
|
||
| @Override | ||
| public void onSlashCommand(SlashCommandInteractionEvent event) { | ||
| OptionMapping repositoryOwnerOption = event.getOption(REPOSITORY_OWNER_OPTION); | ||
| OptionMapping repositoryNameOption = event.getOption(REPOSITORY_NAME_OPTION); | ||
|
|
||
| if (repositoryOwnerOption == null || repositoryNameOption == null) { | ||
| event.reply("You must specify a repository owner and a repository name") | ||
| .setEphemeral(true) | ||
| .queue(); | ||
| return; | ||
| } | ||
|
|
||
| long channelId = event.getChannelIdLong(); | ||
| String repositoryOwner = repositoryOwnerOption.getAsString(); | ||
| String repositoryName = repositoryNameOption.getAsString(); | ||
|
|
||
| GitHub github; | ||
| try { | ||
| github = new GitHubBuilder().withOAuthToken(githubApiKey).build(); | ||
| } catch (IOException e) { | ||
| logger.error("Failed to initialize GitHub API wrapper.", e); | ||
|
Contributor
There was a problem hiding this comment. The error message, since duplicate should be stored in a |
||
| event.reply("Failed to initialize GitHub API wrapper.").setEphemeral(true).queue(); | ||
|
Contributor
There was a problem hiding this comment. 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" |
||
| return; | ||
| } | ||
|
|
||
| try { | ||
| if (!isRepositoryAccessible(github, repositoryOwner, repositoryName)) { | ||
| event.reply("Repository is not publicly available.").setEphemeral(true).queue(); | ||
|
Contributor
There was a problem hiding this comment. 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. |
||
| logger.info("Repository {}/{} is not accessible.", repositoryOwner, repositoryName); | ||
| return; | ||
| } | ||
| } catch (IOException e) { | ||
| logger.error("Failed to check if GitHub repository is available.", e); | ||
| event.reply("Failed to link repository.").setEphemeral(true).queue(); | ||
|
Contributor
There was a problem hiding this comment. Same here, opt for the user friendly message. Make it a constant above since it can be reused. |
||
| return; | ||
| } | ||
|
|
||
| try { | ||
| saveNotificationToDatabase(channelId, repositoryOwner, repositoryName); | ||
| event.reply("Successfully linked repository.").setEphemeral(true).queue(); | ||
|
Contributor
There was a problem hiding this comment. Let's make this response a pretty embed for everyone in the project thread to see. |
||
| } catch (DatabaseException e) { | ||
| logger.error("Failed to save pull request notification to database.", e); | ||
| event.reply("Failed to link repository.").setEphemeral(true).queue(); | ||
|
Contributor
There was a problem hiding this comment. Better error message please |
||
| } | ||
| } | ||
|
|
||
| private boolean isRepositoryAccessible(GitHub github, String owner, String name) | ||
| throws IOException { | ||
| GHRepository repository = github.getRepository(owner + "/" + name); | ||
|
Contributor
There was a problem hiding this comment. this can be static and also just |
||
| return repository != null; | ||
| } | ||
|
|
||
| private void saveNotificationToDatabase(long channelId, String repositoryOwner, | ||
|
Contributor
There was a problem hiding this comment. are we saving "notifications" to the database? It's more so, saving GH names for a specific project. Notification blurs that intent. |
||
| String repositoryName) { | ||
| database.write(context -> { | ||
| PrNotificationsRecord prNotificationsRecord = | ||
| context.newRecord(PrNotifications.PR_NOTIFICATIONS); | ||
| prNotificationsRecord.setChannelId(channelId); | ||
| prNotificationsRecord.setRepositoryOwner(repositoryOwner); | ||
| prNotificationsRecord.setRepositoryName(repositoryName); | ||
| prNotificationsRecord.insert(); | ||
| }); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| package org.togetherjava.tjbot.features.github; | ||
|
|
||
| import net.dv8tion.jda.api.events.interaction.command.SlashCommandInteractionEvent; | ||
| import net.dv8tion.jda.api.interactions.commands.OptionMapping; | ||
| import net.dv8tion.jda.api.interactions.commands.OptionType; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import org.togetherjava.tjbot.db.Database; | ||
| import org.togetherjava.tjbot.db.DatabaseException; | ||
| import org.togetherjava.tjbot.db.generated.tables.PrNotifications; | ||
| import org.togetherjava.tjbot.features.CommandVisibility; | ||
| import org.togetherjava.tjbot.features.SlashCommandAdapter; | ||
|
|
||
| /** | ||
| * Slash command to unlink a project from a channel. | ||
| */ | ||
| public class GitHubUnlinkCommand extends SlashCommandAdapter { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(GitHubUnlinkCommand.class); | ||
|
|
||
| private static final String REPOSITORY_OWNER_OPTION = "owner"; | ||
| private static final String REPOSITORY_NAME_OPTION = "name"; | ||
|
|
||
| private final Database database; | ||
|
|
||
| /** | ||
| * Creates new GitHub unlink command. | ||
| * | ||
| * @param database the database to remove linked pull request notifications | ||
| */ | ||
| public GitHubUnlinkCommand(Database database) { | ||
| super("unlink-gh-project", "Unlinks a GitHub repository", CommandVisibility.GUILD); | ||
| this.database = database; | ||
|
|
||
| getData() | ||
|
Contributor
There was a problem hiding this comment. We don't need the repo name/channel for unlinking or any options. Unless you're designing in a way that multiple repositories can be linked to a single project, in that case you'll also need a |
||
| .addOption(OptionType.STRING, REPOSITORY_OWNER_OPTION, | ||
| "The owner of the repository to get unlinked", true) | ||
|
Contributor
There was a problem hiding this comment. instead of |
||
| .addOption(OptionType.STRING, REPOSITORY_NAME_OPTION, | ||
| "The name of the repository to get unlinked", true); | ||
| } | ||
|
|
||
| @Override | ||
| public void onSlashCommand(SlashCommandInteractionEvent event) { | ||
| OptionMapping repositoryOwnerOption = event.getOption(REPOSITORY_OWNER_OPTION); | ||
| OptionMapping repositoryNameOption = event.getOption(REPOSITORY_NAME_OPTION); | ||
|
|
||
| if (repositoryOwnerOption == null || repositoryNameOption == null) { | ||
| event.reply("You must specify a repository owner and a repository name") | ||
| .setEphemeral(true) | ||
| .queue(); | ||
| return; | ||
| } | ||
|
|
||
| long channelId = event.getChannelIdLong(); | ||
| String repositoryOwner = repositoryOwnerOption.getAsString(); | ||
| String repositoryName = repositoryNameOption.getAsString(); | ||
|
|
||
| try { | ||
| int deleted = deleteNotification(channelId, repositoryOwner, repositoryName); | ||
|
|
||
| if (deleted == 0) { | ||
| event.reply("The provided repository wasn't linked to this channel previously.") | ||
| .setEphemeral(true) | ||
| .queue(); | ||
| } else { | ||
| event.reply("Successfully unlinked repository.").setEphemeral(true).queue(); | ||
| } | ||
| } catch (DatabaseException e) { | ||
| logger.error("Failed to delete pull request notification link from database.", e); | ||
| event.reply("Failed to unlink repository.").setEphemeral(true).queue(); | ||
| } | ||
| } | ||
|
|
||
| private int deleteNotification(long channelId, String repositoryOwner, String repositoryName) { | ||
|
Contributor
There was a problem hiding this comment. it's not a notification so please rename |
||
| return database | ||
| .writeAndProvide(context -> context.deleteFrom(PrNotifications.PR_NOTIFICATIONS) | ||
| .where(PrNotifications.PR_NOTIFICATIONS.CHANNEL_ID.eq(channelId)) | ||
| .and(PrNotifications.PR_NOTIFICATIONS.REPOSITORY_OWNER.eq(repositoryOwner)) | ||
| .and(PrNotifications.PR_NOTIFICATIONS.REPOSITORY_NAME.eq(repositoryName)) | ||
| .execute()); | ||
| } | ||
|
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| package org.togetherjava.tjbot.features.github; | ||
|
|
||
| import net.dv8tion.jda.api.JDA; | ||
| import net.dv8tion.jda.api.entities.channel.concrete.ThreadChannel; | ||
| import org.kohsuke.github.GHIssueState; | ||
| import org.kohsuke.github.GHPullRequest; | ||
| import org.kohsuke.github.GHRepository; | ||
| import org.kohsuke.github.GitHub; | ||
| import org.kohsuke.github.GitHubBuilder; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import org.togetherjava.tjbot.config.Config; | ||
| import org.togetherjava.tjbot.db.Database; | ||
| import org.togetherjava.tjbot.db.generated.tables.PrNotifications; | ||
| import org.togetherjava.tjbot.db.generated.tables.records.PrNotificationsRecord; | ||
| import org.togetherjava.tjbot.features.Routine; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Date; | ||
| import java.util.List; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| /** | ||
| * Routine to send a notification about new pull request. | ||
| */ | ||
| public class PullRequestNotificationRoutine implements Routine { | ||
|
|
||
| private static final Logger logger = | ||
| LoggerFactory.getLogger(PullRequestNotificationRoutine.class); | ||
|
|
||
| private final Database database; | ||
| private final String githubApiKey; | ||
| private Date lastExecution; | ||
|
Contributor
There was a problem hiding this comment. no need for a |
||
|
|
||
| /** | ||
| * Creates new notification routine. | ||
| * | ||
| * @param database the database to get the pull request notifications | ||
| * @param config the config to get the GitHub API key | ||
| */ | ||
| public PullRequestNotificationRoutine(Database database, Config config) { | ||
| this.database = database; | ||
| this.githubApiKey = config.getGitHubApiKey(); | ||
| this.lastExecution = new Date(); | ||
| } | ||
|
|
||
| @Override | ||
| public Schedule createSchedule() { | ||
| return new Schedule(ScheduleMode.FIXED_RATE, 0, 15, TimeUnit.MINUTES); | ||
|
Contributor
There was a problem hiding this comment. Make the time an option in the config or make it a static const. |
||
| } | ||
|
|
||
| @Override | ||
| public void runRoutine(JDA jda) { | ||
| GitHub github; | ||
| try { | ||
| github = new GitHubBuilder().withOAuthToken(githubApiKey).build(); | ||
| } catch (IOException e) { | ||
| logger.error("Failed to initialize GitHub API wrapper.", e); | ||
| return; | ||
| } | ||
|
|
||
| for (PrNotificationsRecord notification : getAllNotifications()) { | ||
| long channelId = notification.getChannelId(); | ||
| String repositoryOwner = notification.getRepositoryOwner(); | ||
| String repositoryName = notification.getRepositoryName(); | ||
|
|
||
| try { | ||
| GHRepository repository = | ||
| github.getRepository(repositoryOwner + "/" + repositoryName); | ||
|
|
||
| if (repository == null) { | ||
| logger.info("Failed to find repository {}/{}.", repositoryOwner, | ||
|
Contributor
There was a problem hiding this comment.
|
||
| repositoryName); | ||
| continue; | ||
| } | ||
|
|
||
| List<GHPullRequest> pullRequests = repository.getPullRequests(GHIssueState.OPEN); | ||
| for (GHPullRequest pr : pullRequests) { | ||
| if (pr.getCreatedAt().after(lastExecution)) { | ||
|
Contributor
There was a problem hiding this comment.
|
||
| sendNotification(jda, channelId, pr); | ||
| } | ||
| } | ||
| } catch (IOException e) { | ||
| logger.error("Failed to send notification for repository {}/{}.", repositoryOwner, | ||
| repositoryName, e); | ||
| } | ||
| } | ||
|
|
||
| lastExecution = new Date(); | ||
|
Contributor
There was a problem hiding this comment.
|
||
| } | ||
|
|
||
| private List<PrNotificationsRecord> getAllNotifications() { | ||
| return database | ||
| .read(context -> context.selectFrom(PrNotifications.PR_NOTIFICATIONS).fetch()); | ||
| } | ||
|
|
||
| private void sendNotification(JDA jda, long channelId, GHPullRequest pr) throws IOException { | ||
| ThreadChannel channel = jda.getThreadChannelById(channelId); | ||
| if (channel == null) { | ||
| logger.info("Failed to find channel {} to send pull request notification.", channelId); | ||
| return; | ||
| } | ||
| channel.sendMessage("New pull request from " + pr.getUser().getLogin() + ".").queue(); | ||
|
Contributor
There was a problem hiding this comment. Let's make this a pretty embed. We're missing key details here such as the PR name, description, number, creation date and URL. |
||
| } | ||
|
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| 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 | ||
| ) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 move this into the constructor and log. So we can fail fast. Or even better, do the init inside
Features.javaand only then bind the routine/commands if we can successfully create aGitHubobject.