From 7f30a2e9b86ff50618ba894ae2becd495a1e81c0 Mon Sep 17 00:00:00 2001 From: JG9 Date: Fri, 21 Feb 2025 14:38:36 +0200 Subject: [PATCH] **Add actions to open existing PRs/MRs in tool windows** Introduce actions for opening existing pull/merge requests directly in the respective tool windows. This improves usability by allowing users to interact with existing requests if detected, avoiding duplicate creation workflows. --- .../messages/GithubBundle.properties | 1 + .../GHPushNotificationCustomizer.kt | 119 +++++++++++------- .../action/GHPRCreatePullRequestAction.kt | 31 ++++- .../messages/GitLabBundle.properties | 1 + .../GitLabMergeRequestOpenCreateTabAction.kt | 34 +++-- .../GitLabPushNotificationCustomizer.kt | 71 +++++++---- 6 files changed, 177 insertions(+), 80 deletions(-) diff --git a/plugins/github/github-core/resources/messages/GithubBundle.properties b/plugins/github/github-core/resources/messages/GithubBundle.properties index 0e0c1f5cad600..941925a0bb615 100644 --- a/plugins/github/github-core/resources/messages/GithubBundle.properties +++ b/plugins/github/github-core/resources/messages/GithubBundle.properties @@ -165,6 +165,7 @@ pull.request.create.creating=Creating a pull request pull.request.create.setting.metadata=Updating pull request fields pull.request.create.error.details=Details pull.request.notification.create.action=Create pull request +pull.request.notification.open.action=Open pull request # TW content toolwindow.stripe.Pull_Requests=Pull Requests diff --git a/plugins/github/github-core/src/org/jetbrains/plugins/github/notification/GHPushNotificationCustomizer.kt b/plugins/github/github-core/src/org/jetbrains/plugins/github/notification/GHPushNotificationCustomizer.kt index d28ca72e77014..89dacc3998c9f 100644 --- a/plugins/github/github-core/src/org/jetbrains/plugins/github/notification/GHPushNotificationCustomizer.kt +++ b/plugins/github/github-core/src/org/jetbrains/plugins/github/notification/GHPushNotificationCustomizer.kt @@ -1,4 +1,5 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code +// is governed by the Apache 2.0 license. package org.jetbrains.plugins.github.notification import com.intellij.dvcs.push.VcsPushOptionValue @@ -16,11 +17,13 @@ import org.jetbrains.plugins.github.api.GithubApiRequestExecutor import org.jetbrains.plugins.github.api.GithubApiRequests.Repos.PullRequests import org.jetbrains.plugins.github.api.data.GithubIssueState import org.jetbrains.plugins.github.api.data.pullrequest.GHPullRequestRestIdOnly +import org.jetbrains.plugins.github.api.data.pullrequest.toPRIdentifier import org.jetbrains.plugins.github.api.executeSuspend import org.jetbrains.plugins.github.authentication.accounts.GHAccountManager import org.jetbrains.plugins.github.authentication.accounts.GithubAccount import org.jetbrains.plugins.github.authentication.accounts.GithubProjectDefaultAccountHolder import org.jetbrains.plugins.github.pullrequest.GHRepositoryConnectionManager +import org.jetbrains.plugins.github.pullrequest.action.GHOpenPullRequestExistingTabNotificationAction import org.jetbrains.plugins.github.pullrequest.action.GHPRCreatePullRequestNotificationAction import org.jetbrains.plugins.github.pullrequest.config.GithubPullRequestsProjectUISettings import org.jetbrains.plugins.github.util.GHGitRepositoryMapping @@ -35,9 +38,11 @@ internal class GHPushNotificationCustomizer(private val project: Project) : GitP pushResult: GitPushRepoResult, customParams: Map, ): List { + if (!pushResult.isSuccessful) return emptyList() val remoteBranch = pushResult.findRemoteBranch(repository) ?: return emptyList() + // If we already have a GH connection open, make sure it matches val connection = project.serviceAsync().connectionState.value if (connection != null && (connection.repo.gitRepository != repository || connection.repo.gitRemote != remoteBranch.remote)) { return emptyList() @@ -45,70 +50,94 @@ internal class GHPushNotificationCustomizer(private val project: Project) : GitP val (projectMapping, account) = connection?.let { it.repo to it.account - } ?: GitPushNotificationUtil.findRepositoryAndAccount( - project.serviceAsync().knownRepositories, - repository, remoteBranch.remote, - serviceAsync().accountsState.value, - project.serviceAsync().selectedUrlAndAccount?.second, - project.serviceAsync().account - ) ?: return emptyList() - - val canCreate = canCreateReview(projectMapping, account, remoteBranch) - if (!canCreate) return emptyList() - - return listOf(GHPRCreatePullRequestNotificationAction(project, projectMapping, account)) + } + ?: GitPushNotificationUtil.findRepositoryAndAccount(project.serviceAsync().knownRepositories, repository, remoteBranch.remote, project.serviceAsync().accountsState.value, project.serviceAsync().selectedUrlAndAccount?.second, project.serviceAsync().account) + ?: return emptyList() + + + if (!canCreateReview(projectMapping, account, remoteBranch)) { + return emptyList() + } + + val existingPrs = findExistingPullRequests(projectMapping, account, remoteBranch) + return when (existingPrs.size) { + 0 -> { + listOf(GHPRCreatePullRequestNotificationAction(project, projectMapping, account)) + } + 1 -> { + val singlePr = existingPrs.first() + listOf(GHOpenPullRequestExistingTabNotificationAction(project, projectMapping, account, singlePr.toPRIdentifier())) + } + else -> { + emptyList() + } + } } - private suspend fun canCreateReview(repositoryMapping: GHGitRepositoryMapping, account: GithubAccount, branch: GitRemoteBranch): Boolean { + /** + * Checks if it's even valid to create a PR. + * For instance: + * - The repository must exist + * - The branch cannot be the default branch (we don't allow creating a PR from default -> default) + */ + private suspend fun canCreateReview( + repositoryMapping: GHGitRepositoryMapping, + account: GithubAccount, + branch: GitRemoteBranch, + ): Boolean { val accountManager = serviceAsync() val token = accountManager.findCredentials(account) ?: return false val executor = GithubApiRequestExecutor.Factory.getInstance().create(account.server, token) val repository = repositoryMapping.repository val repositoryInfo = getRepositoryInfo(executor, repository) ?: run { - LOG.warn("Repository not found $repository") + LOG.warn("Repository not found: $repository") return false } + // Don't allow creating PRs targeting the default branch val remoteBranchName = branch.nameForRemoteOperations - if (repositoryInfo.defaultBranch == remoteBranchName) { - return false - } - - if (findExistingPullRequests(executor, repository, remoteBranchName).isNotEmpty()) { - return false - } - - return true + return repositoryInfo.defaultBranch != remoteBranchName } - private suspend fun getRepositoryInfo(executor: GithubApiRequestExecutor, repository: GHRepositoryCoordinates) = - try { - executor.executeSuspend(GHGQLRequests.Repo.find(repository)) - } - catch (ce: CancellationException) { - throw ce - } - catch (e: Exception) { - LOG.warn("Failed to lookup a repository $repository", e) - null - } - - private suspend fun findExistingPullRequests( + private suspend fun getRepositoryInfo( executor: GithubApiRequestExecutor, repository: GHRepositoryCoordinates, - remoteBranchName: String, - ): List = try { - executor.executeSuspend(PullRequests.find(repository, - GithubIssueState.open, - null, - repository.repositoryPath.owner + ":" + remoteBranchName)).items + ) = try { + executor.executeSuspend(GHGQLRequests.Repo.find(repository)) } catch (ce: CancellationException) { throw ce } catch (e: Exception) { - LOG.warn("Failed to lookup an existing pull request for $remoteBranchName in $repository", e) - emptyList() + LOG.warn("Failed to lookup repository $repository", e) + null + } + + /** + * Look up any existing open pull requests on the given remote branch. + */ + private suspend fun findExistingPullRequests( + repositoryMapping: GHGitRepositoryMapping, + account: GithubAccount, + branch: GitRemoteBranch, + ): List { + val accountManager = serviceAsync() + val token = accountManager.findCredentials(account) ?: return emptyList() + val executor = GithubApiRequestExecutor.Factory.getInstance().create(account.server, token) + + val repository = repositoryMapping.repository + val remoteBranchName = branch.nameForRemoteOperations + + return try { + executor.executeSuspend(PullRequests.find(repository, GithubIssueState.open, baseRef = null, headRef = repository.repositoryPath.owner + ":" + remoteBranchName)).items + } + catch (ce: CancellationException) { + throw ce + } + catch (e: Exception) { + LOG.warn("Failed to lookup existing pull requests for branch $remoteBranchName in $repository", e) + emptyList() + } } } \ No newline at end of file diff --git a/plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/action/GHPRCreatePullRequestAction.kt b/plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/action/GHPRCreatePullRequestAction.kt index a47993281f44b..32317115395c8 100644 --- a/plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/action/GHPRCreatePullRequestAction.kt +++ b/plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/action/GHPRCreatePullRequestAction.kt @@ -12,6 +12,8 @@ import com.intellij.openapi.project.DumbAwareAction import com.intellij.openapi.project.Project import org.jetbrains.plugins.github.authentication.accounts.GithubAccount import org.jetbrains.plugins.github.i18n.GithubBundle +import org.jetbrains.plugins.github.pullrequest.data.GHPRIdentifier +import org.jetbrains.plugins.github.pullrequest.ui.toolwindow.GHPRToolWindowTab import org.jetbrains.plugins.github.pullrequest.ui.toolwindow.model.GHPRToolWindowViewModel import org.jetbrains.plugins.github.util.GHGitRepositoryMapping @@ -40,14 +42,35 @@ internal class GHPRCreatePullRequestAction : DumbAwareAction() { override fun actionPerformed(e: AnActionEvent) = tryToCreatePullRequest(e) } +// NOTE: no need to register in plugin.xml +internal class GHOpenPullRequestExistingTabNotificationAction( + private val project: Project, + private val projectMapping: GHGitRepositoryMapping, + private val account: GithubAccount, + private val existingPrOrNull: GHPRIdentifier, +) : NotificationAction(GithubBundle.message("pull.request.notification.open.action")) { + override fun actionPerformed(e: AnActionEvent, notification: Notification) { + val twVm = project.service() + val selectorVm = twVm.selectorVm + selectorVm.selectRepoAndAccount(projectMapping, account) + selectorVm.submitSelection() + openExistingTab(e, existingPrOrNull) + } +} + +private fun openExistingTab(event: AnActionEvent, prId: GHPRIdentifier) { + event.project!!.service().activateAndAwaitProject { + selectTab(GHPRToolWindowTab.PullRequest(prId)) + } +} + + // NOTE: no need to register in plugin.xml internal class GHPRCreatePullRequestNotificationAction( private val project: Project, private val projectMapping: GHGitRepositoryMapping, - private val account: GithubAccount -) : NotificationAction( - GithubBundle.message("pull.request.notification.create.action") -) { + private val account: GithubAccount, +) : NotificationAction(GithubBundle.message("pull.request.notification.create.action")) { override fun actionPerformed(e: AnActionEvent, notification: Notification) { val twVm = project.service() val selectorVm = twVm.selectorVm diff --git a/plugins/gitlab/gitlab-core/resources/messages/GitLabBundle.properties b/plugins/gitlab/gitlab-core/resources/messages/GitLabBundle.properties index df6a92d36095b..69a9bf1d2395d 100644 --- a/plugins/gitlab/gitlab-core/resources/messages/GitLabBundle.properties +++ b/plugins/gitlab/gitlab-core/resources/messages/GitLabBundle.properties @@ -199,3 +199,4 @@ snippet.create.error.log-in=Log In snippet.create.error.no-contents=No non-empty contents selected snippet.create.error.some-empty-contents=Some files have empty contents snippet.create.error.some-empty-contents.tooltip=Empty files are excluded from snippet: {0} +merge.request.notification.open.action=Open merge request diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/create/action/GitLabMergeRequestOpenCreateTabAction.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/create/action/GitLabMergeRequestOpenCreateTabAction.kt index e5c76939c7c06..2c44dc9d2c9b7 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/create/action/GitLabMergeRequestOpenCreateTabAction.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/create/action/GitLabMergeRequestOpenCreateTabAction.kt @@ -12,6 +12,7 @@ import com.intellij.openapi.project.DumbAwareAction import com.intellij.openapi.project.Project import org.jetbrains.plugins.gitlab.GitlabIcons import org.jetbrains.plugins.gitlab.authentication.accounts.GitLabAccount +import org.jetbrains.plugins.gitlab.mergerequest.ui.toolwindow.GitLabReviewTab import org.jetbrains.plugins.gitlab.mergerequest.ui.toolwindow.model.GitLabToolWindowViewModel import org.jetbrains.plugins.gitlab.util.GitLabBundle import org.jetbrains.plugins.gitlab.util.GitLabProjectMapping @@ -40,22 +41,39 @@ internal class GitLabMergeRequestOpenCreateTabAction : DumbAwareAction() { } override fun actionPerformed(e: AnActionEvent) { - val place = if (e.place == ActionPlaces.TOOLWINDOW_TITLE) - GitLabStatistics.ToolWindowOpenTabActionPlace.TOOLWINDOW - else - GitLabStatistics.ToolWindowOpenTabActionPlace.ACTION + val place = if (e.place == ActionPlaces.TOOLWINDOW_TITLE) GitLabStatistics.ToolWindowOpenTabActionPlace.TOOLWINDOW + else GitLabStatistics.ToolWindowOpenTabActionPlace.ACTION openCreationTab(e, place) } } +internal class GitLabOpenMergeRequestExistingTabNotificationAction( + private val project: Project, + private val projectMapping: GitLabProjectMapping, + private val account: GitLabAccount, + private val existingMrOrNull: String, +) : NotificationAction(GitLabBundle.message("merge.request.notification.open.action")) { + override fun actionPerformed(e: AnActionEvent, notification: Notification) { + val twVm = project.service() + val selectorVm = twVm.selectorVm.value ?: error("Tool window has not been initialized") + selectorVm.selectRepoAndAccount(projectMapping, account) + selectorVm.submitSelection() + openExistingTab(e, existingMrOrNull) + } +} + +private fun openExistingTab(event: AnActionEvent, mrId: String) { + event.project!!.service().activateAndAwaitProject { + selectTab(GitLabReviewTab.ReviewSelected(mrId)) + } +} + // NOTE: no need to register in plugin.xml (or any xml-file replacing plugin.xml) internal class GitLabMergeRequestOpenCreateTabNotificationAction( private val project: Project, private val projectMapping: GitLabProjectMapping, - private val account: GitLabAccount -) : NotificationAction( - GitLabBundle.message("merge.request.create.notification.action.text") -) { + private val account: GitLabAccount, +) : NotificationAction(GitLabBundle.message("merge.request.create.notification.action.text")) { override fun actionPerformed(e: AnActionEvent, notification: Notification) { val twVm = project.service() val selectorVm = twVm.selectorVm.value ?: error("Tool window has not been initialized") diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/notification/GitLabPushNotificationCustomizer.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/notification/GitLabPushNotificationCustomizer.kt index 7d83bdf1dacb1..dbe53845b1719 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/notification/GitLabPushNotificationCustomizer.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/notification/GitLabPushNotificationCustomizer.kt @@ -28,6 +28,7 @@ import org.jetbrains.plugins.gitlab.mergerequest.api.dto.GitLabMergeRequestByBra import org.jetbrains.plugins.gitlab.mergerequest.api.request.findMergeRequestsByBranch import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequestState import org.jetbrains.plugins.gitlab.mergerequest.ui.create.action.GitLabMergeRequestOpenCreateTabNotificationAction +import org.jetbrains.plugins.gitlab.mergerequest.ui.create.action.GitLabOpenMergeRequestExistingTabNotificationAction import org.jetbrains.plugins.gitlab.util.GitLabProjectMapping private val LOG = logger() @@ -36,11 +37,12 @@ internal class GitLabPushNotificationCustomizer(private val project: Project) : override suspend fun getActions( repository: GitRepository, pushResult: GitPushRepoResult, - customParams: Map + customParams: Map, ): List { if (!pushResult.isSuccessful) return emptyList() val remoteBranch = pushResult.findRemoteBranch(repository) ?: return emptyList() + // If we already have a GitLab connection open, make sure it matches val connection = project.serviceAsync().connectionState.value if (connection != null && (connection.repo.gitRepository != repository || connection.repo.gitRemote != remoteBranch.remote)) { return emptyList() @@ -62,12 +64,31 @@ internal class GitLabPushNotificationCustomizer(private val project: Project) : ) } ?: return emptyList() - val canCreate = canCreateReview(projectMapping, account, remoteBranch) - if (!canCreate) return emptyList() + if (!canCreateReview(projectMapping, account, remoteBranch)) { + return emptyList() + } - return listOf(GitLabMergeRequestOpenCreateTabNotificationAction(project, projectMapping, account)) + val existingMRs = findExistingMergeRequests(projectMapping, account, remoteBranch) + return when (existingMRs.size) { + 0 -> { + listOf(GitLabMergeRequestOpenCreateTabNotificationAction(project, projectMapping, account)) + } + 1 -> { + val singleMR = existingMRs.first() + listOf(GitLabOpenMergeRequestExistingTabNotificationAction(project, projectMapping, account, singleMR.iid)) + } + else -> { + emptyList() + } + } } + /** + * Checks if it's even valid to create a merge request. + * For instance: + * - The repository must exist + * - The branch cannot be the default branch (we don't allow creating an MR from default -> default) + */ private suspend fun canCreateReview(projectMapping: GitLabProjectMapping, account: GitLabAccount, branch: GitRemoteBranch): Boolean { val accountManager = serviceAsync() val token = accountManager.findCredentials(account) ?: return false @@ -75,20 +96,13 @@ internal class GitLabPushNotificationCustomizer(private val project: Project) : val repository = projectMapping.repository val repositoryInfo = getRepositoryInfo(api, repository) ?: run { - LOG.warn("Repository not found $repository") + LOG.warn("Repository not found: $repository") return false } + // Don't allow creating MRs targeting the default branch val remoteBranchName = branch.nameForRemoteOperations - if (repositoryInfo.rootRef == remoteBranchName) { - return false - } - - if (findExistingMergeRequests(api, repository, remoteBranchName).isNotEmpty()) { - return false - } - - return true + return repositoryInfo.rootRef != remoteBranchName } private suspend fun getRepositoryInfo(api: GitLabApi, project: GitLabProjectCoordinates) = @@ -99,27 +113,38 @@ internal class GitLabPushNotificationCustomizer(private val project: Project) : throw ce } catch (e: Exception) { - LOG.warn("Failed to lookup a repository $project", e) + LOG.warn("Failed to lookup repository $project", e) null } + /** + * Look up any existing open merge requests on the given remote branch. + */ private suspend fun findExistingMergeRequests( - api: GitLabApi, - project: GitLabProjectCoordinates, - remoteBranchName: String, - ): List = - withContext(Dispatchers.IO) { - val targetProjectPath = project.projectPath.fullPath() + projectMapping: GitLabProjectMapping, + account: GitLabAccount, + branch: GitRemoteBranch, + ): List { + val accountManager = serviceAsync() + val token = accountManager.findCredentials(account) ?: return emptyList() + val api = serviceAsync().getClient(account.server, token) + + val repository = projectMapping.repository + val remoteBranchName = branch.nameForRemoteOperations + + return withContext(Dispatchers.IO) { + val targetProjectPath = repository.projectPath.fullPath() try { - val mrs = api.graphQL.findMergeRequestsByBranch(project, GitLabMergeRequestState.OPENED, remoteBranchName).body()!!.nodes + val mrs = api.graphQL.findMergeRequestsByBranch(repository, GitLabMergeRequestState.OPENED, remoteBranchName).body()!!.nodes mrs.filter { it.targetProject.fullPath == targetProjectPath && it.sourceProject?.fullPath == targetProjectPath } } catch (ce: CancellationException) { throw ce } catch (e: Exception) { - LOG.warn("Failed to lookup an existing merge request for $remoteBranchName in $project", e) + LOG.warn("Failed to lookup existing merge requests for branch $remoteBranchName in $repository", e) emptyList() + } } } } \ No newline at end of file