From 15c154b65855deed71a60d55cc3aa74a6c5d385b Mon Sep 17 00:00:00 2001 From: Ivan Semenov Date: Sat, 7 Feb 2026 12:46:12 +0100 Subject: [PATCH] refactor [gitlab]: a better reviewers selector when adjusting merge request reviewers GitOrigin-RevId: 8a21855157011a40ffece74f9fcc45c0f417d255 --- .../GitLabMergeRequestRequestReviewAction.kt | 35 +++++++++++- .../GitLabMergeRequestReviewFlowViewModel.kt | 53 ++++++------------- .../util/GitLabMergeRequestChoosersUtil.kt | 45 ---------------- 3 files changed, 50 insertions(+), 83 deletions(-) diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/action/GitLabMergeRequestRequestReviewAction.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/action/GitLabMergeRequestRequestReviewAction.kt index b7ae1d0b065b..1e14f5efa842 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/action/GitLabMergeRequestRequestReviewAction.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/action/GitLabMergeRequestRequestReviewAction.kt @@ -3,10 +3,17 @@ package org.jetbrains.plugins.gitlab.mergerequest.action import com.intellij.collaboration.async.combineAndCollect import com.intellij.collaboration.messages.CollaborationToolsBundle +import com.intellij.collaboration.ui.codereview.list.search.ShowDirection +import com.intellij.openapi.application.UI +import com.intellij.platform.util.coroutines.sync.OverflowSemaphore import com.intellij.ui.awt.RelativePoint import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.channels.BufferOverflow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import org.jetbrains.plugins.gitlab.mergerequest.ui.details.model.GitLabMergeRequestReviewFlowViewModel +import org.jetbrains.plugins.gitlab.mergerequest.util.GitLabMergeRequestChoosersUtil import java.awt.event.ActionEvent import javax.swing.AbstractAction import javax.swing.JComponent @@ -15,6 +22,8 @@ internal class GitLabMergeRequestRequestReviewAction( private val scope: CoroutineScope, private val reviewFlowVm: GitLabMergeRequestReviewFlowViewModel ) : AbstractAction(CollaborationToolsBundle.message("review.details.action.request")) { + private val sem = OverflowSemaphore(1, BufferOverflow.DROP_OLDEST) + init { scope.launch { combineAndCollect(reviewFlowVm.isBusy, reviewFlowVm.userCanManage) { isBusy, userCanManageReview -> @@ -26,6 +35,30 @@ internal class GitLabMergeRequestRequestReviewAction( override fun actionPerformed(event: ActionEvent) { val parentComponent = event.source as? JComponent ?: return val point = RelativePoint.getSouthWestOf(parentComponent) - reviewFlowVm.adjustReviewers(point) + scope.launch(Dispatchers.UI) { + val allowsMultipleReviewers = reviewFlowVm.allowsMultipleReviewers.first() + val currentReviewers = reviewFlowVm.reviewers.value + val updatedReviewers = sem.withPermit { + if (allowsMultipleReviewers) { + GitLabMergeRequestChoosersUtil.chooseUsers( + point, + currentReviewers, + reviewFlowVm.projectMembers, + reviewFlowVm.avatarIconsProvider, + ShowDirection.ABOVE + ) + } + else { + GitLabMergeRequestChoosersUtil.chooseUser( + point, + reviewFlowVm.projectMembers, + reviewFlowVm.avatarIconsProvider, + ShowDirection.ABOVE + )?.let { listOfNotNull(it) } + } + } + updatedReviewers ?: return@launch + reviewFlowVm.setReviewers(updatedReviewers) + } } } diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/details/model/GitLabMergeRequestReviewFlowViewModel.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/details/model/GitLabMergeRequestReviewFlowViewModel.kt index 92194fdf89f8..5c61e63bc334 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/details/model/GitLabMergeRequestReviewFlowViewModel.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/details/model/GitLabMergeRequestReviewFlowViewModel.kt @@ -2,9 +2,9 @@ package org.jetbrains.plugins.gitlab.mergerequest.ui.details.model import com.intellij.collaboration.async.childScope -import com.intellij.collaboration.async.launchNow import com.intellij.collaboration.async.mapState import com.intellij.collaboration.async.modelFlow +import com.intellij.collaboration.async.withInitial import com.intellij.collaboration.messages.CollaborationToolsBundle import com.intellij.collaboration.ui.codereview.action.ReviewMergeCommitMessageDialog import com.intellij.collaboration.ui.codereview.details.data.ReviewRequestState @@ -12,24 +12,20 @@ import com.intellij.collaboration.ui.codereview.details.data.ReviewRole import com.intellij.collaboration.ui.codereview.details.data.ReviewState import com.intellij.collaboration.ui.codereview.details.model.CodeReviewFlowViewModel import com.intellij.collaboration.ui.icon.IconsProvider +import com.intellij.collaboration.util.IncrementallyComputedValue import com.intellij.collaboration.util.SingleCoroutineLauncher +import com.intellij.collaboration.util.collectIncrementallyTo import com.intellij.openapi.application.EDT import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.project.Project import com.intellij.openapi.util.text.StringUtil -import com.intellij.ui.awt.RelativePoint import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.cancel import kotlinx.coroutines.currentCoroutineContext -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.SharedFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asFlow -import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.* import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import org.jetbrains.annotations.ApiStatus @@ -45,9 +41,7 @@ import org.jetbrains.plugins.gitlab.mergerequest.ui.review.GitLabMergeRequestSub import org.jetbrains.plugins.gitlab.mergerequest.ui.review.GitLabMergeRequestSubmitReviewViewModel.SubmittableReview import org.jetbrains.plugins.gitlab.mergerequest.ui.review.GitLabMergeRequestSubmitReviewViewModelImpl import org.jetbrains.plugins.gitlab.mergerequest.ui.review.getSubmittableReview -import org.jetbrains.plugins.gitlab.mergerequest.util.GitLabMergeRequestChoosersUtil import org.jetbrains.plugins.gitlab.util.GitLabBundle -import org.jetbrains.plugins.gitlab.util.GitLabCoroutineUtil @ApiStatus.Internal interface GitLabMergeRequestReviewFlowViewModel : CodeReviewFlowViewModel { @@ -57,6 +51,7 @@ interface GitLabMergeRequestReviewFlowViewModel : CodeReviewFlowViewModel val reviewRequestState: SharedFlow val reviewers: StateFlow> @@ -73,8 +68,7 @@ interface GitLabMergeRequestReviewFlowViewModel : CodeReviewFlowViewModel var submitReviewInputHandler: (suspend (GitLabMergeRequestSubmitReviewViewModel) -> Unit)? - //TODO: extract reviewers update VM - val potentialReviewers: Flow>> + val projectMembers: StateFlow>> /** * Request the start of a submission process @@ -93,11 +87,10 @@ interface GitLabMergeRequestReviewFlowViewModel : CodeReviewFlowViewModel) + @SinceGitLab("13.8") fun removeReviewer(reviewer: GitLabUserDTO) @@ -106,13 +99,14 @@ interface GitLabMergeRequestReviewFlowViewModel : CodeReviewFlowViewModel() +@OptIn(ExperimentalCoroutinesApi::class) internal class GitLabMergeRequestReviewFlowViewModelImpl( private val project: Project, parentScope: CoroutineScope, override val currentUser: GitLabUserDTO, projectData: GitLabProject, private val mergeRequest: GitLabMergeRequest, - private val avatarIconsProvider: IconsProvider + override val avatarIconsProvider: IconsProvider ) : GitLabMergeRequestReviewFlowViewModel { private val scope = parentScope.childScope(this::class) private val taskLauncher = SingleCoroutineLauncher(scope) @@ -189,8 +183,10 @@ internal class GitLabMergeRequestReviewFlowViewModelImpl( override val submittableReview: SharedFlow = mergeRequest.getSubmittableReview(currentUser).modelFlow(scope, LOG) override var submitReviewInputHandler: (suspend (GitLabMergeRequestSubmitReviewViewModel) -> Unit)? = null - override val potentialReviewers: Flow>> = - GitLabCoroutineUtil.batchesResultsFlow(projectData.dataReloadSignal, projectData::getMembersBatches) + override val projectMembers: StateFlow>> = + projectData.dataReloadSignal.withInitial(Unit).transformLatest { + projectData.getMembersBatches().collectIncrementallyTo(this) + }.stateIn(scope, SharingStarted.Lazily, IncrementallyComputedValue.loading()) override fun submitReview() { scope.launch { @@ -271,22 +267,6 @@ internal class GitLabMergeRequestReviewFlowViewModelImpl( mergeRequest.postReview() } - override fun adjustReviewers(point: RelativePoint) { - scope.launchNow(Dispatchers.Main) { - val allowsMultipleReviewers = allowsMultipleReviewers.first() - val updatedReviewers = if (allowsMultipleReviewers) { - GitLabMergeRequestChoosersUtil.chooseUsers(point, reviewers.value, potentialReviewers, avatarIconsProvider) - } - else { - GitLabMergeRequestChoosersUtil.chooseUser(point, potentialReviewers, avatarIconsProvider)?.let { listOfNotNull(it) } - } - - updatedReviewers ?: return@launchNow - setReviewers(updatedReviewers) - } - } - - @SinceGitLab("13.8") override fun setMyselfAsReviewer() = runAction { val allowsMultipleReviewers = allowsMultipleReviewers.first() if (allowsMultipleReviewers) { @@ -309,8 +289,7 @@ internal class GitLabMergeRequestReviewFlowViewModelImpl( mergeRequest.reviewerRereview(requestedReviewers) } - @SinceGitLab("13.8") - private fun setReviewers(reviewers: List) = runAction { + override fun setReviewers(reviewers: List) = runAction { mergeRequest.setReviewers(reviewers) } diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/util/GitLabMergeRequestChoosersUtil.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/util/GitLabMergeRequestChoosersUtil.kt index 0141d275c76d..b1bc936dfc50 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/util/GitLabMergeRequestChoosersUtil.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/util/GitLabMergeRequestChoosersUtil.kt @@ -9,55 +9,10 @@ import com.intellij.collaboration.ui.icon.IconsProvider import com.intellij.collaboration.ui.util.popup.PopupItemPresentation import com.intellij.collaboration.util.IncrementallyComputedValue import com.intellij.ui.awt.RelativePoint -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.flow import org.jetbrains.plugins.gitlab.api.dto.GitLabUserDTO internal object GitLabMergeRequestChoosersUtil { - suspend fun chooseUser( - point: RelativePoint, - users: Flow>>, - avatarIconsProvider: IconsProvider, - ): GitLabUserDTO? { - return ChooserPopupUtil.showAsyncChooserPopup( - point, - users, - presenter = { reviewer -> - PopupItemPresentation.Simple( - reviewer.username, - avatarIconsProvider.getIcon(reviewer, Avatar.Sizes.BASE), - reviewer.name, - ) - } - ) - } - - suspend fun chooseUsers( - point: RelativePoint, - chosenUsers: List, - users: Flow>>, - avatarIconsProvider: IconsProvider, - ): List { - val usersBatch = flow { - val batch = users.first() - emit(batch) - } - return ChooserPopupUtil.showAsyncMultipleChooserPopup( - point, - chosenUsers, - usersBatch, - presenter = { reviewer -> - PopupItemPresentation.Simple( - reviewer.username, - avatarIconsProvider.getIcon(reviewer, Avatar.Sizes.BASE), - reviewer.name, - ) - } - ) - } - suspend fun chooseUser( point: RelativePoint, users: StateFlow>>,