Skip to content
Snippets Groups Projects
Verified Commit 4e262cc9 authored by Phil Hughes's avatar Phil Hughes
Browse files

Updated merge request dashboard lists

Updated the dashboard lists to be the correct lists.
Added a new query to query for assignee or reviewer which is required for some of
the lists on the dashboard.

#461217
parent 83708caa
No related branches found
No related tags found
1 merge request!160593Updated merge request dashboard lists
Showing
with 391 additions and 49 deletions
......@@ -91,7 +91,7 @@ export default {
const currentUser = this.isCurrentUser(user);
const thisIsYouText = currentUser ? __('This is you.') : '';
let titleType = 'default';
let titleType;
if (this.type === 'ASSIGNEES') {
titleType = this.type;
......@@ -100,7 +100,7 @@ export default {
}
return sprintf(
USER_TOOLTIP_TITLES[titleType],
USER_TOOLTIP_TITLES[titleType] || USER_TOOLTIP_TITLES.default,
{
name: user.name,
you: thisIsYouText,
......
<script>
import reviewerQuery from '../queries/reviewer.query.graphql';
import assigneeQuery from '../queries/assignee.query.graphql';
import assigneeOrReviewerQuery from '../queries/assignee_or_reviewer.query.graphql';
const PER_PAGE = 20;
const QUERIES = {
reviewRequestedMergeRequests: reviewerQuery,
assignedMergeRequests: assigneeQuery,
assigneeOrReviewerMergeRequests: assigneeOrReviewerQuery,
};
export default {
apollo: {
mergeRequests: {
query() {
return this.query === 'reviewRequestedMergeRequests' ? reviewerQuery : assigneeQuery;
return QUERIES[this.query];
},
update(d) {
return d.currentUser?.[this.query] || {};
......
#import "~/graphql_shared/fragments/page_info.fragment.graphql"
#import "./merge_request.fragment.graphql"
query assigneeOrReviewer(
$state: MergeRequestState = opened
$assignedReviewStates: [MergeRequestReviewState!]
$reviewerReviewStates: [MergeRequestReviewState!]
$mergedAfter: Time
$perPage: Int!
$afterCursor: String
) {
currentUser {
id
assigneeOrReviewerMergeRequests(
state: $state
assignedReviewStates: $assignedReviewStates
reviewerReviewStates: $reviewerReviewStates
mergedAfter: $mergedAfter
first: $perPage
after: $afterCursor
) {
count
pageInfo {
...PageInfo
}
nodes {
...MergeRequestDashboardFragment
}
}
}
}
......@@ -82,6 +82,7 @@ def filter_items(_items)
items = by_review_state(items)
items = by_source_project_id(items)
items = by_resource_event_state(items)
items = by_assignee_or_reviewer(items)
by_approved(items)
end
......@@ -268,6 +269,17 @@ def by_negated_reviewer(items)
end
end
def by_assignee_or_reviewer(items)
return items unless current_user&.merge_request_dashboard_enabled?
return items unless params.assigned_user
items.assignee_or_reviewer(
params.assigned_user,
params.assigned_review_states,
params.reviewer_review_states
)
end
def parse_datetime(input)
# NOTE: Input from GraphQL query is a Time object already.
# Just return DateTime object for consistency instead of trying to parse it.
......
......@@ -20,6 +20,14 @@ def reviewer
end
end
def assigned_user
strong_memoize(:assigned_user) do
next unless params[:assigned_user_id].present?
User.find_by_id(params[:assigned_user_id])
end
end
def merge_user
strong_memoize(:merge_user) do
if merge_user_id?
......@@ -30,6 +38,18 @@ def merge_user
end
end
def assigned_review_states
return unless params[:assigned_review_states].present?
params[:assigned_review_states].map { |state| MergeRequestReviewer.states[state] }
end
def reviewer_review_states
return unless params[:reviewer_review_states].present?
params[:reviewer_review_states].map { |state| MergeRequestReviewer.states[state] }
end
def review_state
if params[:review_state].present?
MergeRequestReviewer.states[params[:review_state]]
......
# frozen_string_literal: true
module Resolvers
module MergeRequests
class AssigneeOrReviewerMergeRequestsResolver < UserMergeRequestsResolverBase
type ::Types::MergeRequestType.connection_type, null: true
argument :assigned_review_states, [::Types::MergeRequestReviewStateEnum],
required: false,
description: 'Reviewer states for merge requests the current user is assigned to.'
argument :reviewer_review_states, [::Types::MergeRequestReviewStateEnum],
required: false,
description: 'Reviewer states for the merge requests the current user is a reviewer of.'
def resolve(**args)
return unless current_user.merge_request_dashboard_enabled?
super(**args)
end
def user_role
:assigned_user
end
end
end
end
......@@ -5,6 +5,12 @@ module Types
class CurrentUserType < ::Types::UserType
graphql_name 'CurrentUser'
description 'The currently authenticated GitLab user.'
field :assignee_or_reviewer_merge_requests,
resolver: Resolvers::MergeRequests::AssigneeOrReviewerMergeRequestsResolver,
description: 'Merge requests the current user is an assignee or a reviewer of.' \
'Ignored if `merge_request_dashboard` feature flag is disabled.',
alpha: { milestone: '17.4' }
end
# rubocop:enable Graphql/AuthorizeTypes
end
......
......@@ -361,71 +361,56 @@ def merge_request_dashboard_data
{
lists: [
{
title: _('Open'),
title: _('Returned to you'),
query: 'assignedMergeRequests',
variables: {
reviewerWildcardId: 'NONE'
reviewStates: %w[REVIEWED REQUESTED_CHANGES]
}
},
{
title: _('Reviews requested'),
query: 'reviewRequestedMergeRequests',
variables: {
reviewStates: %w[UNREVIEWED REVIEW_STARTED]
}
},
{
title: _('Returned to you'),
query: 'assignedMergeRequests',
variables: {
reviewStates: %w[REQUESTED_CHANGES REVIEWED]
}
},
{
title: _('Waiting for reviewers'),
query: 'assignedMergeRequests',
variables: {
reviewStates: %w[UNREVIEWED UNAPPROVED REVIEW_STARTED]
reviewStates: %w[UNAPPROVED UNREVIEWED REVIEW_STARTED]
}
},
{
title: _('Yours approved'),
title: _('Assigned to you'),
query: 'assignedMergeRequests',
variables: {
reviewState: 'APPROVED'
reviewerWildcardId: 'NONE'
}
},
{
title: _('Reviewed'),
query: 'reviewRequestedMergeRequests',
title: _('Waiting for others'),
query: 'assigneeOrReviewerMergeRequests',
variables: {
reviewStates: %w[REQUESTED_CHANGES REVIEWED]
reviewerReviewStates: %w[REVIEWED REQUESTED_CHANGES],
assignedReviewStates: %w[UNREVIEWED UNAPPROVED REVIEW_STARTED]
}
},
{
title: _('Reviews approved'),
title: _('Approved by you'),
query: 'reviewRequestedMergeRequests',
variables: {
reviewState: 'APPROVED'
}
},
{
title: _('Merged as assignee (1 week ago)'),
title: _('Approved by others'),
query: 'assignedMergeRequests',
variables: {
state: 'merged',
mergedAfter: 1.week.ago.to_time.iso8601
reviewState: 'APPROVED'
}
},
{
title: _('Merged as reviewer (1 week ago)'),
query: 'reviewRequestedMergeRequests',
title: _('Merged recently'),
query: 'assigneeOrReviewerMergeRequests',
variables: {
state: 'merged',
mergedAfter: 1.week.ago.to_time.iso8601
mergedAfter: 2.weeks.ago.to_time.iso8601
}
}
]
}
end
......
......@@ -468,6 +468,16 @@ def public_merge_status
)
end
scope :assignee_or_reviewer, ->(user, assigned_review_states, reviewer_state) do
assigned_to_scope = assigned_to(user)
assigned_to_scope = assigned_to_scope.review_states(assigned_review_states) if assigned_review_states
from_union(
assigned_to_scope,
review_requested_to(user, reviewer_state)
)
end
scope :without_hidden, -> {
if Feature.enabled?(:hide_merge_requests_from_banned_users)
where_not_exists(Users::BannedUser.where('merge_requests.author_id = banned_users.user_id'))
......
......@@ -19807,6 +19807,57 @@ four standard [pagination arguments](#pagination-arguments):
| <a id="currentuserassignedmergerequestsupdatedafter"></a>`updatedAfter` | [`Time`](#time) | Merge requests updated after the timestamp. |
| <a id="currentuserassignedmergerequestsupdatedbefore"></a>`updatedBefore` | [`Time`](#time) | Merge requests updated before the timestamp. |
 
##### `CurrentUser.assigneeOrReviewerMergeRequests`
Merge requests the current user is an assignee or a reviewer of.Ignored if `merge_request_dashboard` feature flag is disabled.
DETAILS:
**Introduced** in GitLab 17.4.
**Status**: Experiment.
Returns [`MergeRequestConnection`](#mergerequestconnection).
This field returns a [connection](#connections). It accepts the
four standard [pagination arguments](#pagination-arguments):
`before: String`, `after: String`, `first: Int`, and `last: Int`.
###### Arguments
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="currentuserassigneeorreviewermergerequestsapproved"></a>`approved` | [`Boolean`](#boolean) | Limit results to approved merge requests. Available only when the feature flag `mr_approved_filter` is enabled. |
| <a id="currentuserassigneeorreviewermergerequestsapprovedby"></a>`approvedBy` | [`[String!]`](#string) | Usernames of the approvers. |
| <a id="currentuserassigneeorreviewermergerequestsassignedreviewstates"></a>`assignedReviewStates` | [`[MergeRequestReviewState!]`](#mergerequestreviewstate) | Reviewer states for merge requests the current user is assigned to. |
| <a id="currentuserassigneeorreviewermergerequestscreatedafter"></a>`createdAfter` | [`Time`](#time) | Merge requests created after the timestamp. |
| <a id="currentuserassigneeorreviewermergerequestscreatedbefore"></a>`createdBefore` | [`Time`](#time) | Merge requests created before the timestamp. |
| <a id="currentuserassigneeorreviewermergerequestsdeployedafter"></a>`deployedAfter` | [`Time`](#time) | Merge requests deployed after the timestamp. |
| <a id="currentuserassigneeorreviewermergerequestsdeployedbefore"></a>`deployedBefore` | [`Time`](#time) | Merge requests deployed before the timestamp. |
| <a id="currentuserassigneeorreviewermergerequestsdeploymentid"></a>`deploymentId` | [`String`](#string) | ID of the deployment. |
| <a id="currentuserassigneeorreviewermergerequestsdraft"></a>`draft` | [`Boolean`](#boolean) | Limit result to draft merge requests. |
| <a id="currentuserassigneeorreviewermergerequestsgroupid"></a>`groupId` | [`GroupID`](#groupid) | The global ID of the group the authored merge requests should be in. Merge requests in subgroups are included. |
| <a id="currentuserassigneeorreviewermergerequestsiids"></a>`iids` | [`[String!]`](#string) | Array of IIDs of merge requests, for example `[1, 2]`. |
| <a id="currentuserassigneeorreviewermergerequestslabelname"></a>`labelName` | [`[String]`](#string) | Labels applied to the merge request. |
| <a id="currentuserassigneeorreviewermergerequestslabels"></a>`labels` **{warning-solid}** | [`[String!]`](#string) | **Deprecated** in GitLab 17.1. Use `labelName`. |
| <a id="currentuserassigneeorreviewermergerequestsmergedafter"></a>`mergedAfter` | [`Time`](#time) | Merge requests merged after the date. |
| <a id="currentuserassigneeorreviewermergerequestsmergedbefore"></a>`mergedBefore` | [`Time`](#time) | Merge requests merged before the date. |
| <a id="currentuserassigneeorreviewermergerequestsmergedby"></a>`mergedBy` | [`String`](#string) | Username of the merger. |
| <a id="currentuserassigneeorreviewermergerequestsmilestonetitle"></a>`milestoneTitle` | [`String`](#string) | Title of the milestone. Incompatible with milestoneWildcardId. |
| <a id="currentuserassigneeorreviewermergerequestsmilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. Incompatible with milestoneTitle. |
| <a id="currentuserassigneeorreviewermergerequestsmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by your reaction emoji. |
| <a id="currentuserassigneeorreviewermergerequestsnot"></a>`not` | [`MergeRequestsResolverNegatedParams`](#mergerequestsresolvernegatedparams) | List of negated arguments. Warning: this argument is experimental and a subject to change in future. |
| <a id="currentuserassigneeorreviewermergerequestsprojectid"></a>`projectId` | [`ProjectID`](#projectid) | The global ID of the project the authored merge requests should be in. Incompatible with projectPath. |
| <a id="currentuserassigneeorreviewermergerequestsprojectpath"></a>`projectPath` | [`String`](#string) | The full-path of the project the authored merge requests should be in. Incompatible with projectId. |
| <a id="currentuserassigneeorreviewermergerequestsreleasetag"></a>`releaseTag` | [`String`](#string) | Filter by release tag. |
| <a id="currentuserassigneeorreviewermergerequestsreviewstate"></a>`reviewState` **{warning-solid}** | [`MergeRequestReviewState`](#mergerequestreviewstate) | **Introduced** in GitLab 17.0. **Status**: Experiment. Reviewer state of the merge request. |
| <a id="currentuserassigneeorreviewermergerequestsreviewstates"></a>`reviewStates` **{warning-solid}** | [`[MergeRequestReviewState!]`](#mergerequestreviewstate) | **Introduced** in GitLab 17.0. **Status**: Experiment. Reviewer states of the merge request. |
| <a id="currentuserassigneeorreviewermergerequestsreviewerreviewstates"></a>`reviewerReviewStates` | [`[MergeRequestReviewState!]`](#mergerequestreviewstate) | Reviewer states for the merge requests the current user is a reviewer of. |
| <a id="currentuserassigneeorreviewermergerequestssort"></a>`sort` | [`MergeRequestSort`](#mergerequestsort) | Sort merge requests by the criteria. |
| <a id="currentuserassigneeorreviewermergerequestssourcebranches"></a>`sourceBranches` | [`[String!]`](#string) | Array of source branch names. All resolved merge requests will have one of these branches as their source. |
| <a id="currentuserassigneeorreviewermergerequestsstate"></a>`state` | [`MergeRequestState`](#mergerequeststate) | Merge request state. If provided, all resolved merge requests will have the state. |
| <a id="currentuserassigneeorreviewermergerequeststargetbranches"></a>`targetBranches` | [`[String!]`](#string) | Array of target branch names. All resolved merge requests will have one of these branches as their target. |
| <a id="currentuserassigneeorreviewermergerequestsupdatedafter"></a>`updatedAfter` | [`Time`](#time) | Merge requests updated after the timestamp. |
| <a id="currentuserassigneeorreviewermergerequestsupdatedbefore"></a>`updatedBefore` | [`Time`](#time) | Merge requests updated before the timestamp. |
##### `CurrentUser.authoredMergeRequests`
 
Merge requests authored by the user.
......@@ -7026,6 +7026,12 @@ msgstr ""
msgid "Approved"
msgstr ""
 
msgid "Approved by others"
msgstr ""
msgid "Approved by you"
msgstr ""
msgid "Approved members will use an additional seat in your subscription, which may override your user cap."
msgid_plural "Approved members will use an additional %d seats in your subscription, which may override your user cap."
msgstr[0] ""
......@@ -7423,6 +7429,9 @@ msgstr ""
msgid "Assigned to me"
msgstr ""
 
msgid "Assigned to you"
msgstr ""
msgid "Assignee"
msgid_plural "%d Assignees"
msgstr[0] ""
......@@ -33324,18 +33333,15 @@ msgstr ""
msgid "Merged %{timeAgo} by"
msgstr ""
 
msgid "Merged as assignee (1 week ago)"
msgstr ""
msgid "Merged as reviewer (1 week ago)"
msgstr ""
msgid "Merged branches are being deleted. This can take some time depending on the number of branches. Please refresh the page to see changes."
msgstr ""
 
msgid "Merged by"
msgstr ""
 
msgid "Merged recently"
msgstr ""
msgid "Merged this merge request."
msgstr ""
 
......@@ -45282,9 +45288,6 @@ msgstr ""
msgid "Review time is the amount of time since the first comment in a merge request."
msgstr ""
 
msgid "Reviewed"
msgstr ""
msgid "Reviewer"
msgid_plural "%d Reviewers"
msgstr[0] ""
......@@ -45299,9 +45302,6 @@ msgstr ""
msgid "Reviewing (merge request !%{mergeRequestId})"
msgstr ""
 
msgid "Reviews approved"
msgstr ""
msgid "Reviews requested"
msgstr ""
 
......@@ -59533,7 +59533,7 @@ msgstr ""
msgid "Waiting for merge (open and assigned)"
msgstr ""
 
msgid "Waiting for reviewers"
msgid "Waiting for others"
msgstr ""
 
msgid "Want to see the data? Please ask an administrator for access."
......@@ -62397,9 +62397,6 @@ msgstr ""
msgid "Your work items are being imported. Once finished, you'll receive a confirmation email."
msgstr ""
 
msgid "Yours approved"
msgstr ""
msgid "You’re about to permanently delete the %{issuableType} ‘%{strongOpen}%{issuableTitle}%{strongClose}’. To avoid data loss, consider %{strongOpen}closing this %{issuableType}%{strongClose} instead. Once deleted, it cannot be undone or recovered."
msgstr ""
 
......@@ -569,6 +569,73 @@ def find(label_name)
it { is_expected.to contain_exactly(*expected_mr) }
end
context 'assignee or reviewer filtering' do
let(:dashboard_flag_enabled) { true }
let(:params) { { assigned_user_id: user.id } }
let(:expected_mrs) { [merge_request1, merge_request2, merge_request3] }
subject { described_class.new(user, params).execute }
before do
stub_feature_flags(merge_request_dashboard: dashboard_flag_enabled)
end
context 'when merge_request_dashboard feature flag is disabled' do
let(:dashboard_flag_enabled) { false }
let(:expected_mrs) { [merge_request1, merge_request2, merge_request3, merge_request4, merge_request5] }
it { is_expected.to contain_exactly(*expected_mrs) }
end
it { is_expected.to contain_exactly(*expected_mrs) }
end
context 'assignee or reviewer filtering with assigned_review_states' do
let(:params) { { assigned_user_id: user.id, assigned_review_states: [:reviewed] } }
let(:expected_mr) { [merge_request1, merge_request3] }
subject { described_class.new(user, params).execute }
before do
stub_feature_flags(merge_request_dashboard: true)
merge_request1.merge_request_reviewers.update_all(state: :reviewed)
end
it { is_expected.to contain_exactly(*expected_mr) }
end
context 'assignee or reviewer filtering with reviewer_review_states' do
let(:params) { { assigned_user_id: user2.id, reviewer_review_states: [:reviewed] } }
let(:expected_mr) { [merge_request1, merge_request3] }
subject { described_class.new(user2, params).execute }
before do
stub_feature_flags(merge_request_dashboard: true)
merge_request1.merge_request_reviewers.update_all(state: :reviewed)
end
it { is_expected.to contain_exactly(*expected_mr) }
end
context 'assignee or reviewer filtering with assigned_review_states and reviewer_review_states' do
let(:params) { { assigned_user_id: user.id, assigned_review_states: [:requested_changes], reviewer_review_states: [:reviewed] } }
let(:expected_mr) { [merge_request1, merge_request3] }
subject { described_class.new(user, params).execute }
before do
stub_feature_flags(merge_request_dashboard: true)
merge_request1.merge_request_reviewers.update_all(state: :requested_changes)
merge_request3.merge_request_reviewers.update_all(state: :reviewed)
end
it { is_expected.to contain_exactly(*expected_mr) }
end
context 'filtering by group milestone' do
let(:group_milestone) { create(:milestone, group: group) }
......
......@@ -233,6 +233,33 @@
end
end
describe '.assignee_or_reviewer' do
let_it_be(:merge_request5) do
create(:merge_request, :prepared, :unique_branches, assignees: [user1], reviewers: [user2], created_at:
2.days.ago)
end
it 'returns merge requests that the user is a reviewer or an assignee of' do
expect(described_class.assignee_or_reviewer(user1, nil, nil)).to match_array([merge_request1, merge_request2, merge_request5])
end
context 'when the user is an assignee and a reviewer reviewed' do
before_all do
merge_request5.merge_request_reviewers.update_all(state: :reviewed)
end
it { expect(described_class.assignee_or_reviewer(user1, MergeRequestReviewer.states[:reviewed], nil)).to match_array([merge_request1, merge_request2, merge_request5]) }
it { expect(described_class.assignee_or_reviewer(user1, MergeRequestReviewer.states[:requested_changes], nil)).to match_array([merge_request1, merge_request2]) }
end
context 'when the user is a reviewer and left a review' do
it { expect(described_class.assignee_or_reviewer(user1, nil, MergeRequestReviewer.states[:reviewed])).to match_array([merge_request2, merge_request5]) }
it { expect(described_class.assignee_or_reviewer(user1, nil, MergeRequestReviewer.states[:requested_changes])).to match_array([merge_request1, merge_request5]) }
end
end
describe '.drafts' do
it 'returns MRs where draft == true' do
expect(described_class.drafts).to eq([merge_request4])
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'getting current users assigned or review requested merge requests', feature_category: :code_review_workflow do
include GraphqlHelpers
let_it_be(:project) { create(:project, :repository, :public) }
let_it_be(:current_user) { create(:user) }
let_it_be(:merge_request_1) do
create(:merge_request, :unique_branches, source_project: project, reviewers: [current_user])
end
let_it_be(:reviewer_change_requested) do
create(:merge_request, :unique_branches, source_project: project, assignees: [current_user],
reviewers: create_list(:user, 2))
end
let_it_be(:merge_request_3) do
create(:merge_request, :unique_branches, source_project: project, assignees: [current_user])
end
let(:merge_requests) { graphql_data.dig('currentUser', 'assigneeOrReviewerMergeRequests', 'nodes') }
let(:fields) do
<<~GRAPHQL
nodes { id }
GRAPHQL
end
def query(params = {})
graphql_query_for('currentUser', {}, query_graphql_field('assigneeOrReviewerMergeRequests', params, fields))
end
before_all do
project.add_developer(current_user)
reviewer_change_requested.merge_request_reviewers[0].update!(state: :requested_changes)
end
context 'when merge_request_dashboard feature flag is disabled' do
before do
stub_feature_flags(merge_request_dashboard: false)
end
it do
post_graphql(query, current_user: current_user)
expect(merge_requests).to be_nil
end
end
context 'when merge_request_dashboard feature flag is enabled' do
before do
stub_feature_flags(merge_request_dashboard: true)
end
it do
post_graphql(query, current_user: current_user)
expect(merge_requests).to contain_exactly(
a_hash_including('id' => merge_request_1.to_global_id.to_s),
a_hash_including('id' => reviewer_change_requested.to_global_id.to_s),
a_hash_including('id' => merge_request_3.to_global_id.to_s)
)
end
context 'when assigned_review_states argument is sent' do
it do
post_graphql(query({ assigned_review_states: [:REVIEWED] }), current_user: current_user)
expect(merge_requests).to contain_exactly(
a_hash_including('id' => merge_request_1.to_global_id.to_s)
)
end
end
context 'when reviewer_review_states argument is sent' do
it do
post_graphql(query({ reviewer_review_states: [:REQUESTED_CHANGES] }), current_user: current_user)
expect(merge_requests).to contain_exactly(
a_hash_including('id' => reviewer_change_requested.to_global_id.to_s),
a_hash_including('id' => merge_request_3.to_global_id.to_s)
)
end
end
context 'when reviewer_review_states and assigned_review_states arguments are sent' do
it do
post_graphql(query({ reviewer_review_states: [:UNREVIEWED], assigned_review_states: [:REQUESTED_CHANGES] }),
current_user: current_user)
expect(merge_requests).to contain_exactly(
a_hash_including('id' => merge_request_1.to_global_id.to_s),
a_hash_including('id' => reviewer_change_requested.to_global_id.to_s)
)
end
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment