diff --git a/app/assets/javascripts/pages/dashboard/merge_requests/index.js b/app/assets/javascripts/pages/dashboard/merge_requests/index.js index 3145e31d41ca010cb4668621d8630c8fe184fc3b..13f57d0348084e33c4c284ccafcce710671a6d05 100644 --- a/app/assets/javascripts/pages/dashboard/merge_requests/index.js +++ b/app/assets/javascripts/pages/dashboard/merge_requests/index.js @@ -1,16 +1,11 @@ +import { initNewResourceDropdown } from '~/vue_shared/components/new_resource_dropdown/init_new_resource_dropdown'; import { RESOURCE_TYPE_MERGE_REQUEST } from '~/vue_shared/components/new_resource_dropdown/constants'; import searchUserProjectsWithMergeRequestsEnabled from '~/vue_shared/components/new_resource_dropdown/graphql/search_user_projects_with_merge_requests_enabled.query.graphql'; -import { initMergeRequestDashboard } from '~/merge_request_dashboard'; +import { initMergeRequestsDashboard } from './page'; -initMergeRequestDashboard(document.getElementById('js-merge-request-dashboard')); - -requestIdleCallback(async () => { - const { initNewResourceDropdown } = await import( - '~/vue_shared/components/new_resource_dropdown/init_new_resource_dropdown' - ); - - initNewResourceDropdown({ - resourceType: RESOURCE_TYPE_MERGE_REQUEST, - query: searchUserProjectsWithMergeRequestsEnabled, - }); +initNewResourceDropdown({ + resourceType: RESOURCE_TYPE_MERGE_REQUEST, + query: searchUserProjectsWithMergeRequestsEnabled, }); + +initMergeRequestsDashboard(); diff --git a/app/assets/javascripts/pages/dashboard/merge_requests_v2/index.js b/app/assets/javascripts/pages/dashboard/merge_requests_v2/index.js new file mode 100644 index 0000000000000000000000000000000000000000..3145e31d41ca010cb4668621d8630c8fe184fc3b --- /dev/null +++ b/app/assets/javascripts/pages/dashboard/merge_requests_v2/index.js @@ -0,0 +1,16 @@ +import { RESOURCE_TYPE_MERGE_REQUEST } from '~/vue_shared/components/new_resource_dropdown/constants'; +import searchUserProjectsWithMergeRequestsEnabled from '~/vue_shared/components/new_resource_dropdown/graphql/search_user_projects_with_merge_requests_enabled.query.graphql'; +import { initMergeRequestDashboard } from '~/merge_request_dashboard'; + +initMergeRequestDashboard(document.getElementById('js-merge-request-dashboard')); + +requestIdleCallback(async () => { + const { initNewResourceDropdown } = await import( + '~/vue_shared/components/new_resource_dropdown/init_new_resource_dropdown' + ); + + initNewResourceDropdown({ + resourceType: RESOURCE_TYPE_MERGE_REQUEST, + query: searchUserProjectsWithMergeRequestsEnabled, + }); +}); diff --git a/app/assets/javascripts/super_sidebar/components/global_search/store/getters.js b/app/assets/javascripts/super_sidebar/components/global_search/store/getters.js index 4c69e5d182ce75a1ab2c34af06973fef2f391bad..0d61a24f7465600a14bebf700f82a2bb112f038f 100644 --- a/app/assets/javascripts/super_sidebar/components/global_search/store/getters.js +++ b/app/assets/javascripts/super_sidebar/components/global_search/store/getters.js @@ -6,6 +6,9 @@ import { MERGE_REQUEST_CATEGORY, MSG_ISSUES_ASSIGNED_TO_ME, MSG_ISSUES_IVE_CREATED, + MSG_MR_ASSIGNED_TO_ME, + MSG_MR_IM_REVIEWER, + MSG_MR_IVE_CREATED, MSG_IN_ALL_GITLAB, MSG_MR_IM_WORKING_ON, PROJECTS_CATEGORY, @@ -108,13 +111,32 @@ export const defaultSearchOptions = (state, getters) => { }, ]; - return [ - ...(getters.scopedIssuesPath ? issues : []), + const mergeRequestDashboardEnabled = window.gon.features?.mergeRequestDashboard; + let mergeRequests = [ { - text: MSG_MR_IM_WORKING_ON, - href: getters.scopedMRPath, + text: MSG_MR_ASSIGNED_TO_ME, + href: `${getters.scopedMRPath}/?assignee_username=${userName}`, + }, + { + text: MSG_MR_IM_REVIEWER, + href: `${getters.scopedMRPath}/?reviewer_username=${userName}`, + }, + { + text: MSG_MR_IVE_CREATED, + href: `${getters.scopedMRPath}/?author_username=${userName}`, }, ]; + + if (mergeRequestDashboardEnabled) { + mergeRequests = [ + { + text: MSG_MR_IM_WORKING_ON, + href: getters.scopedMRPath, + }, + ]; + } + + return [...(getters.scopedIssuesPath ? issues : []), ...mergeRequests]; }; export const projectUrl = (state) => { diff --git a/app/assets/javascripts/super_sidebar/components/merge_request_menu.vue b/app/assets/javascripts/super_sidebar/components/merge_request_menu.vue new file mode 100644 index 0000000000000000000000000000000000000000..e6935809f14a77f2184835e69ede13ce8e31b5dc --- /dev/null +++ b/app/assets/javascripts/super_sidebar/components/merge_request_menu.vue @@ -0,0 +1,42 @@ + + + diff --git a/app/assets/javascripts/super_sidebar/components/user_counts.vue b/app/assets/javascripts/super_sidebar/components/user_counts.vue index 4bc794cf73b1d5c6a154b7cf621c0ece19aa25a0..a497770f6b8a52553b8d0bb63c00a419eda5ccba 100644 --- a/app/assets/javascripts/super_sidebar/components/user_counts.vue +++ b/app/assets/javascripts/super_sidebar/components/user_counts.vue @@ -9,11 +9,13 @@ import { } from '~/super_sidebar/user_counts_manager'; import { fetchUserCounts } from '~/super_sidebar/user_counts_fetch'; import Counter from './counter.vue'; +import MergeRequestMenu from './merge_request_menu.vue'; export default { name: 'UserCounts', components: { Counter, + MergeRequestMenu, }, directives: { GlTooltip: GlTooltipDirective, @@ -31,10 +33,14 @@ export default { }, data() { return { + mrMenuShown: false, userCounts, }; }, computed: { + mergeRequestMenuComponent() { + return this.sidebarData.merge_request_menu ? 'merge-request-menu' : 'div'; + }, issuesTitle() { return n__('%d assigned issue', '%d assigned issues', this.userCounts.assigned_issues); }, @@ -60,6 +66,14 @@ export default { beforeDestroy() { destroyUserCountsManager(); }, + methods: { + onMergeRequestMenuShown() { + this.mrMenuShown = true; + }, + onMergeRequestMenuHidden() { + this.mrMenuShown = false; + }, + }, }; @@ -78,11 +92,20 @@ export default { data-track-label="issues_link" data-track-property="nav_core_menu" /> -
+ -
+ }} +**Introduced** in GitLab 17.4. +**Status**: Experiment. +{{< /details >}} + +Merge requests the current user is an assignee or a reviewer of.Ignored if `merge_request_dashboard` feature flag is disabled. Returns [`MergeRequestConnection`](#mergerequestconnection). diff --git a/ee/app/services/merge_requests/destroy_requested_changes_service.rb b/ee/app/services/merge_requests/destroy_requested_changes_service.rb index 22ab2135e4e47070cc7a45c4548ca9d31ea0b992..32c69878d61d25a7d6bb362b3cce63e36f1cd6b4 100644 --- a/ee/app/services/merge_requests/destroy_requested_changes_service.rb +++ b/ee/app/services/merge_requests/destroy_requested_changes_service.rb @@ -13,10 +13,12 @@ def execute(merge_request) trigger_merge_request_reviewers_updated(merge_request) trigger_merge_request_merge_status_updated(merge_request) - invalidate_cache_counts(merge_request, users: merge_request.assignees) - invalidate_cache_counts(merge_request, users: merge_request.reviewers) + if current_user.merge_request_dashboard_enabled? + invalidate_cache_counts(merge_request, users: merge_request.assignees) + invalidate_cache_counts(merge_request, users: merge_request.reviewers) - current_user.invalidate_merge_request_cache_counts + current_user.invalidate_merge_request_cache_counts + end success else diff --git a/ee/spec/features/dashboards/merge_requests_spec.rb b/ee/spec/features/dashboards/merge_requests_spec.rb index 7c5f74f194ebedeaf9b852ad1c2a9c54bbc9cf42..621b85050b7632ea909f24e521dfd429ecd80b88 100644 --- a/ee/spec/features/dashboards/merge_requests_spec.rb +++ b/ee/spec/features/dashboards/merge_requests_spec.rb @@ -4,7 +4,7 @@ RSpec.describe 'Dashboard merge requests', feature_category: :code_review_workflow do let_it_be(:user) { create(:user) } - let_it_be(:page_path) { merge_requests_search_dashboard_path(assignee_username: [user.username]) } + let_it_be(:page_path) { merge_requests_dashboard_path(assignee_username: [user.username]) } context 'when quarantined test', quarantine: "https://gitlab.com/gitlab-org/gitlab/-/issues/512586" do it_behaves_like 'dashboard ultimate trial callout' diff --git a/ee/spec/helpers/ee/sidebars_helper_spec.rb b/ee/spec/helpers/ee/sidebars_helper_spec.rb index 5166db7dc0679d1ae12f962cca17f2c315e0e946..cf4d417c2e9122b05f7c988118e8f45145bece01 100644 --- a/ee/spec/helpers/ee/sidebars_helper_spec.rb +++ b/ee/spec/helpers/ee/sidebars_helper_spec.rb @@ -297,12 +297,22 @@ expect(super_sidebar_context).to match(hash_including({ sign_out_link: '/users/sign_out', issues_dashboard_path: "/dashboard/issues?assignee_username=#{user.username}", - merge_request_dashboard_path: '/dashboard/merge_requests', + merge_request_dashboard_path: nil, todos_dashboard_path: '/dashboard/todos', projects_path: '/dashboard/projects', groups_path: '/dashboard/groups' })) end + + context 'with merge_request_dashboard feature flag enabled' do + before do + stub_feature_flags(merge_request_dashboard: user) + end + + it 'has merge_request_dashboard_path' do + expect(super_sidebar_context[:merge_request_dashboard_path]).to eq('/dashboard/merge_requests') + end + end end describe '#super_sidebar_default_pins', :experiment do diff --git a/ee/spec/services/merge_requests/destroy_requested_changes_service_spec.rb b/ee/spec/services/merge_requests/destroy_requested_changes_service_spec.rb index 4e53bf4cb0179014c22c43e9bca2acea371969d9..d0ea70f38470712d49a686556b86155b58b8622b 100644 --- a/ee/spec/services/merge_requests/destroy_requested_changes_service_spec.rb +++ b/ee/spec/services/merge_requests/destroy_requested_changes_service_spec.rb @@ -86,22 +86,28 @@ let(:action) { result } end - it 'invalidates cache counts for all assignees' do - expect(merge_request.assignees).to all(receive(:invalidate_merge_request_cache_counts)) + context 'when merge_request_dashboard feature flag is enabled' do + before do + stub_feature_flags(merge_request_dashboard: true) + end - expect(result[:status]).to eq :success - end + it 'invalidates cache counts for all assignees' do + expect(merge_request.assignees).to all(receive(:invalidate_merge_request_cache_counts)) - it 'invalidates cache counts for all reviewers' do - expect(merge_request.reviewers).to all(receive(:invalidate_merge_request_cache_counts)) + expect(result[:status]).to eq :success + end - expect(result[:status]).to eq :success - end + it 'invalidates cache counts for all reviewers' do + expect(merge_request.reviewers).to all(receive(:invalidate_merge_request_cache_counts)) - it 'invalidates cache counts for current user' do - expect(user).to receive(:invalidate_merge_request_cache_counts) + expect(result[:status]).to eq :success + end - expect(result[:status]).to eq :success + it 'invalidates cache counts for current user' do + expect(user).to receive(:invalidate_merge_request_cache_counts) + + expect(result[:status]).to eq :success + end end end end diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index f4fe364c85796710ddb50942fd621c0ff3623234..0d3a8de52ad0025f90bb615bf9e5a140069beb5a 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -98,6 +98,7 @@ def add_gon_feature_flags # To be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/399248 push_frontend_feature_flag(:remove_monitor_metrics) push_frontend_feature_flag(:work_item_view_for_issues) + push_frontend_feature_flag(:merge_request_dashboard, current_user, type: :beta) push_frontend_feature_flag(:new_project_creation_form, current_user, type: :wip) push_frontend_feature_flag(:work_items_client_side_boards, current_user) push_frontend_feature_flag(:glql_work_items, current_user, type: :wip) diff --git a/lib/sidebars/your_work/menus/merge_requests_menu.rb b/lib/sidebars/your_work/menus/merge_requests_menu.rb index 8ca4e2c57f688b42829c0413ad1eb987725dbe7d..d40e24a7a9bcbb848799a45fe537c1a1ec157b7f 100644 --- a/lib/sidebars/your_work/menus/merge_requests_menu.rb +++ b/lib/sidebars/your_work/menus/merge_requests_menu.rb @@ -9,7 +9,11 @@ class MergeRequestsMenu < ::Sidebars::Menu override :link def link - merge_requests_dashboard_path + unless context.current_user.merge_request_dashboard_enabled? + assignee_username = @context.current_user.username + end + + merge_requests_dashboard_path(assignee_username: assignee_username) end override :title @@ -24,7 +28,12 @@ def sprite_icon override :configure_menu_items def configure_menu_items - false + return false if context.current_user.merge_request_dashboard_enabled? + + add_item(assigned_mrs_menu_item) + add_item(reviewer_mrs_menu_item) + + true end override :render? @@ -46,6 +55,34 @@ def has_pill? def pill_count_field "total_merge_requests" end + + private + + def assigned_mrs_menu_item + link = merge_requests_dashboard_path(assignee_username: context.current_user.username) + + ::Sidebars::MenuItem.new( + title: _('Assigned'), + link: link, + active_routes: { page: link }, + has_pill: true, + pill_count_field: "assigned_merge_requests", + item_id: :merge_requests_assigned + ) + end + + def reviewer_mrs_menu_item + link = merge_requests_dashboard_path(reviewer_username: context.current_user.username) + + ::Sidebars::MenuItem.new( + title: _('Review requests'), + link: link, + active_routes: { page: link }, + has_pill: true, + pill_count_field: "review_requested_merge_requests", + item_id: :merge_requests_to_review + ) + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0e442d3a41b4e8842f163a13ac2a722a3f28decf..ac656158f3432777a55a16876d8dd4e49e95a411 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -54756,6 +54756,9 @@ msgstr "" msgid "Review requested from %{reviewerName}" msgstr "" +msgid "Review requests" +msgstr "" + msgid "Review started" msgstr "" diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index 1e43e10ab0025304a8ec71a0517aabb88082a161..8d92fcbe60b2e0d250949c1cbcd19766c6f21808 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -56,6 +56,69 @@ end end + describe 'GET merge requests' do + it_behaves_like 'issuables list meta-data', :merge_request, :merge_requests + it_behaves_like 'issuables requiring filter', :merge_requests + + context 'when an ActiveRecord::QueryCanceled is raised' do + before do + allow_next_instance_of(Gitlab::IssuableMetadata) do |instance| + allow(instance).to receive(:data).and_raise(ActiveRecord::QueryCanceled) + end + end + + it 'sets :search_timeout_occurred' do + get :merge_requests, params: { author_id: user.id } + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:search_timeout_occurred)).to eq(true) + end + + context 'rendering views' do + render_views + + it 'shows error message' do + get :merge_requests, params: { author_id: user.id } + + expect(response.body).to have_content('Too many results to display. Edit your search or add a filter.') + end + + it 'does not display MR counts in nav' do + get :merge_requests, params: { author_id: user.id } + + expect(response.body).to have_content('Open Merged Closed All') + expect(response.body).not_to have_content('Open 0 Merged 0 Closed 0 All 0') + end + end + + it 'logs the exception' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original + + get :merge_requests, params: { author_id: user.id } + end + end + + context 'when an ActiveRecord::QueryCanceled is not raised' do + it 'does not set :search_timeout_occurred' do + get :merge_requests, params: { author_id: user.id } + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:search_timeout_occurred)).to eq(nil) + end + + context 'rendering views' do + render_views + + it 'displays MR counts in nav' do + get :merge_requests, params: { author_id: user.id } + + expect(response.body).to have_content('Open 0 Merged 0 Closed 0 All 0') + expect(response.body).not_to have_content('Open Merged Closed All') + end + end + end + end + describe 'GET merge requests search' do it_behaves_like 'issuables requiring filter', :search_merge_requests diff --git a/spec/controllers/root_controller_spec.rb b/spec/controllers/root_controller_spec.rb index 91dea503a904b3f8e3e2909f8cd04a4caf592a58..789e2bf8a88db468dff332994e899b2d5ea7dce5 100644 --- a/spec/controllers/root_controller_spec.rb +++ b/spec/controllers/root_controller_spec.rb @@ -204,20 +204,48 @@ expect { get :index }.not_to trigger_internal_events('user_views_homepage') end - it 'passes the correct data to the view' do - get :index + context 'when the `merge_request_dashboard` feature flag is enabled' do + before do + stub_feature_flags(merge_request_dashboard: true) + end + + it 'passes the correct data to the view' do + get :index + + expect(assigns[:homepage_app_data]).to eq({ + review_requested_path: "/dashboard/merge_requests", + activity_path: "/dashboard/activity", + assigned_merge_requests_path: "/dashboard/merge_requests", + assigned_work_items_path: "/dashboard/issues?assignee_username=#{user.username}", + authored_work_items_path: "/dashboard/issues?author_username=#{user.username}", + duo_code_review_bot_username: duo_code_review_bot.username, + merge_requests_review_requested_title: "Review requested", + merge_requests_your_merge_requests_title: "Your merge requests", + last_push_event: nil + }) + end + end - expect(assigns[:homepage_app_data]).to eq({ - review_requested_path: "/dashboard/merge_requests", - activity_path: "/dashboard/activity", - assigned_merge_requests_path: "/dashboard/merge_requests", - assigned_work_items_path: "/dashboard/issues?assignee_username=#{user.username}", - authored_work_items_path: "/dashboard/issues?author_username=#{user.username}", - duo_code_review_bot_username: duo_code_review_bot.username, - merge_requests_review_requested_title: "Review requested", - merge_requests_your_merge_requests_title: "Your merge requests", - last_push_event: nil - }) + context 'when the `merge_request_dashboard` feature flag is disabled' do + before do + stub_feature_flags(merge_request_dashboard: false) + end + + it 'passes the correct data to the view' do + get :index + + expect(assigns[:homepage_app_data]).to eq({ + review_requested_path: "/dashboard/merge_requests?reviewer_username=#{user.username}", + activity_path: "/dashboard/activity", + assigned_merge_requests_path: "/dashboard/merge_requests?assignee_username=#{user.username}", + assigned_work_items_path: "/dashboard/issues?assignee_username=#{user.username}", + authored_work_items_path: "/dashboard/issues?author_username=#{user.username}", + duo_code_review_bot_username: duo_code_review_bot.username, + merge_requests_review_requested_title: "Review requested", + merge_requests_your_merge_requests_title: "Your merge requests", + last_push_event: nil + }) + end end end diff --git a/spec/features/dashboard/action_focused_merge_request_dashboard_spec.rb b/spec/features/dashboard/action_focused_merge_request_dashboard_spec.rb index f4217e19d8c172d233efb15cf0878d04129e6d6c..bc8bdc99776a6de82ad2415738d9978660418901 100644 --- a/spec/features/dashboard/action_focused_merge_request_dashboard_spec.rb +++ b/spec/features/dashboard/action_focused_merge_request_dashboard_spec.rb @@ -35,6 +35,8 @@ end before do + stub_feature_flags(merge_request_dashboard: true) + sign_in(current_user) visit merge_requests_dashboard_path diff --git a/spec/features/dashboard/issuables_counter_spec.rb b/spec/features/dashboard/issuables_counter_spec.rb index d1afaa473399b6c85eb44470b631eb95df858b31..a8e50181ac3310bc40bc8d40bb3cc73a52f1b58c 100644 --- a/spec/features/dashboard/issuables_counter_spec.rb +++ b/spec/features/dashboard/issuables_counter_spec.rb @@ -32,6 +32,14 @@ visit merge_requests_path expect_merge_request_count(1) + + merge_request.update!(assignees: []) + + user.invalidate_cache_counts + + visit merge_requests_path + + expect_merge_request_count(0) end def issues_path @@ -52,11 +60,11 @@ def expect_issue_count(count) end def expect_merge_request_count(count) - dashboard_count = find('.top-area .gl-tabs-nav li a.active') + dashboard_count = find('.gl-tabs-nav li a.active') expect(dashboard_count).to have_content(count) within_testid('super-sidebar') do - expect(page).to have_link("#{count} merge request") + expect(page).to have_button("#{count} merge request") end end end diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb index 1522864076c3d1a4dad8dbc0a802a5fd81cb73dd..8d17c9693222db7f1dfbd0e49ed7e07fe5728713 100644 --- a/spec/features/dashboard/merge_requests_spec.rb +++ b/spec/features/dashboard/merge_requests_spec.rb @@ -30,10 +30,16 @@ describe 'sidebar' do it 'has nav items for assigned MRs and review requests' do - visit merge_requests_dashboard_path + visit merge_requests_dashboard_path(assignee_username: user) + + within('#super-sidebar') do + expect(page).to have_css("a[data-track-label='merge_requests_assigned'][aria-current='page']") + end + + click_link 'Review requests' within('#super-sidebar') do - expect(page).to have_css("a[data-track-label='merge_requests_menu'][aria-current='page']") + expect(page).to have_css("a[data-track-label='merge_requests_to_review'][aria-current='page']") end end end @@ -183,8 +189,11 @@ visit merge_requests_dashboard_path(assignee_username: current_user.username) end - it 'includes all merge requests count in badge' do - expect(page).to have_link('Merge requests 8') + it 'includes assigned and reviewers in badge' do + within('#merge-requests') do + expect(page).to have_css("a", text: 'Assigned 3') + expect(page).to have_css("a", text: 'Review requests 2') + end end it 'shows assigned merge requests' do diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 9dfcb6da35f42e8be1f903d012483968a0d6d730..94e09d95c23ace06f197161504fbb63b63fbafc2 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -771,11 +771,23 @@ def find(label_name) 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 @@ -786,6 +798,8 @@ def find(label_name) 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 @@ -799,6 +813,8 @@ def find(label_name) 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 @@ -812,6 +828,8 @@ def find(label_name) 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 diff --git a/spec/frontend/super_sidebar/components/global_search/components/global_search_default_issuables_spec.js b/spec/frontend/super_sidebar/components/global_search/components/global_search_default_issuables_spec.js index b752288f5e82ecaf078710e2556249f185ad5568..2e9dae9619803e33e7c1bd5c84ecc954581f35d8 100644 --- a/spec/frontend/super_sidebar/components/global_search/components/global_search_default_issuables_spec.js +++ b/spec/frontend/super_sidebar/components/global_search/components/global_search_default_issuables_spec.js @@ -9,6 +9,9 @@ import { useMockInternalEventsTracking } from 'helpers/tracking_internal_events_ import { EVENT_CLICK_ISSUES_ASSIGNED_TO_ME_IN_COMMAND_PALETTE, EVENT_CLICK_ISSUES_I_CREATED_IN_COMMAND_PALETTE, + EVENT_CLICK_MERGE_REQUESTS_ASSIGNED_TO_ME_IN_COMMAND_PALETTE, + EVENT_CLICK_MERGE_REQUESTS_THAT_IM_A_REVIEWER_IN_COMMAND_PALETTE, + EVENT_CLICK_MERGE_REQUESTS_I_CREATED_IN_COMMAND_PALETTE, } from '~/super_sidebar/components/global_search/tracking_constants'; import { MOCK_SEARCH_CONTEXT, @@ -145,9 +148,12 @@ describe('GlobalSearchDefaultPlaces', () => { const { bindInternalEventDocument } = useMockInternalEventsTracking(); it.each` - eventTrigger | event - ${'Issues assigned to me'} | ${EVENT_CLICK_ISSUES_ASSIGNED_TO_ME_IN_COMMAND_PALETTE} - ${"Issues I've created"} | ${EVENT_CLICK_ISSUES_I_CREATED_IN_COMMAND_PALETTE} + eventTrigger | event + ${'Issues assigned to me'} | ${EVENT_CLICK_ISSUES_ASSIGNED_TO_ME_IN_COMMAND_PALETTE} + ${"Issues I've created"} | ${EVENT_CLICK_ISSUES_I_CREATED_IN_COMMAND_PALETTE} + ${'Merge requests assigned to me'} | ${EVENT_CLICK_MERGE_REQUESTS_ASSIGNED_TO_ME_IN_COMMAND_PALETTE} + ${"Merge requests that I'm a reviewer"} | ${EVENT_CLICK_MERGE_REQUESTS_THAT_IM_A_REVIEWER_IN_COMMAND_PALETTE} + ${"Merge requests I've created"} | ${EVENT_CLICK_MERGE_REQUESTS_I_CREATED_IN_COMMAND_PALETTE} `('triggers and tracks command dropdown $event', ({ eventTrigger, event }) => { const { trackEventSpy } = bindInternalEventDocument(wrapper.element); // Update to emit the action event from each dropdown item rather than the group diff --git a/spec/frontend/super_sidebar/components/global_search/mock_data.js b/spec/frontend/super_sidebar/components/global_search/mock_data.js index d7c063f3c1d9ff73eea86eea9ac2dc95bb286a70..3903a995adf7ea1482742c54e93e604c796c1fa0 100644 --- a/spec/frontend/super_sidebar/components/global_search/mock_data.js +++ b/spec/frontend/super_sidebar/components/global_search/mock_data.js @@ -9,6 +9,9 @@ import { GROUPS_CATEGORY, MSG_ISSUES_ASSIGNED_TO_ME, MSG_ISSUES_IVE_CREATED, + MSG_MR_ASSIGNED_TO_ME, + MSG_MR_IM_REVIEWER, + MSG_MR_IVE_CREATED, MSG_IN_ALL_GITLAB, MSG_MR_IM_WORKING_ON, } from '~/vue_shared/global_search/constants'; @@ -85,6 +88,28 @@ export const MOCK_PROJECT_SEARCH_CONTEXT = { }; export const MOCK_DEFAULT_SEARCH_OPTIONS = [ + { + text: MSG_ISSUES_ASSIGNED_TO_ME, + href: `${MOCK_ISSUE_PATH}/?assignee_username=${MOCK_USERNAME}`, + }, + { + text: MSG_ISSUES_IVE_CREATED, + href: `${MOCK_ISSUE_PATH}/?author_username=${MOCK_USERNAME}`, + }, + { + text: MSG_MR_ASSIGNED_TO_ME, + href: `${MOCK_MR_PATH}/?assignee_username=${MOCK_USERNAME}`, + }, + { + text: MSG_MR_IM_REVIEWER, + href: `${MOCK_MR_PATH}/?reviewer_username=${MOCK_USERNAME}`, + }, + { + text: MSG_MR_IVE_CREATED, + href: `${MOCK_MR_PATH}/?author_username=${MOCK_USERNAME}`, + }, +]; +export const MOCK_DASHBOARD_FLAG_ENABLED_SEARCH_OPTIONS = [ { text: MSG_ISSUES_ASSIGNED_TO_ME, href: `${MOCK_ISSUE_PATH}/?assignee_username=${MOCK_USERNAME}`, diff --git a/spec/frontend/super_sidebar/components/global_search/store/getters_spec.js b/spec/frontend/super_sidebar/components/global_search/store/getters_spec.js index 63d859112f119a627646582695b94b387887064c..b69bad0c51cbe32d4832bd3227c35e37c410b91f 100644 --- a/spec/frontend/super_sidebar/components/global_search/store/getters_spec.js +++ b/spec/frontend/super_sidebar/components/global_search/store/getters_spec.js @@ -21,6 +21,7 @@ import { MOCK_GROUPED_AUTOCOMPLETE_OPTIONS, MOCK_SORTED_AUTOCOMPLETE_OPTIONS, MOCK_SCOPED_SEARCH_OPTIONS_DEF, + MOCK_DASHBOARD_FLAG_ENABLED_SEARCH_OPTIONS, } from '../mock_data'; describe('Global Search Store Getters', () => { @@ -267,6 +268,22 @@ describe('Global Search Store Getters', () => { MOCK_DEFAULT_SEARCH_OPTIONS.slice(2, MOCK_DEFAULT_SEARCH_OPTIONS.length), ); }); + + describe('when feature flag mergeRequestDashboard is enabled', () => { + beforeEach(() => { + window.gon.features = { mergeRequestDashboard: true }; + }); + + afterEach(() => { + window.gon.features = {}; + }); + + it('returns the correct array', () => { + expect(getters.defaultSearchOptions(state, mockGetters)).toStrictEqual( + MOCK_DASHBOARD_FLAG_ENABLED_SEARCH_OPTIONS, + ); + }); + }); }); describe('without a user', () => { diff --git a/spec/frontend/super_sidebar/components/merge_request_menu_spec.js b/spec/frontend/super_sidebar/components/merge_request_menu_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..53d47397eb35085847aa56fe8e12f2b184cad4b6 --- /dev/null +++ b/spec/frontend/super_sidebar/components/merge_request_menu_spec.js @@ -0,0 +1,70 @@ +import { GlBadge, GlDisclosureDropdown } from '@gitlab/ui'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import MergeRequestMenu from '~/super_sidebar/components/merge_request_menu.vue'; +import { userCounts } from '~/super_sidebar/user_counts_manager'; +import { mergeRequestMenuGroup } from '../mock_data'; + +describe('MergeRequestMenu component', () => { + let wrapper; + + const findGlBadge = (at) => wrapper.findAllComponents(GlBadge).at(at); + const findGlDisclosureDropdown = () => wrapper.findComponent(GlDisclosureDropdown); + const findLink = (name) => wrapper.findByRole('link', { name }); + + const createWrapper = (items) => { + wrapper = mountExtended(MergeRequestMenu, { + propsData: { + items, + }, + }); + }; + + describe('default', () => { + beforeEach(() => { + createWrapper(mergeRequestMenuGroup); + }); + + it('passes the items to the disclosure dropdown', () => { + expect(findGlDisclosureDropdown().props('items')).toBe(mergeRequestMenuGroup); + }); + + it.each(mergeRequestMenuGroup[0].items)('renders item text and count in link', (item) => { + const index = mergeRequestMenuGroup[0].items.indexOf(item); + const { text, href, count, extraAttrs } = mergeRequestMenuGroup[0].items[index]; + const link = findLink(new RegExp(text)); + + expect(link.text()).toContain(text); + expect(link.text()).toContain(String(count)); + expect(link.attributes('href')).toBe(href); + expect(link.attributes('data-track-action')).toBe(extraAttrs['data-track-action']); + expect(link.attributes('data-track-label')).toBe(extraAttrs['data-track-label']); + expect(link.attributes('data-track-property')).toBe(extraAttrs['data-track-property']); + expect(link.attributes('class')).toContain(extraAttrs.class); + }); + + it('renders item count string in badge', () => { + const { count } = mergeRequestMenuGroup[0].items[0]; + expect(findGlBadge(0).text()).toBe(String(count)); + }); + + it('renders 0 string when count is empty', () => { + expect(findGlBadge(1).text()).toBe(String(0)); + }); + + it('renders value from userCounts if `userCount` prop is defined', () => { + userCounts.assigned_merge_requests = 5; + mergeRequestMenuGroup[0].items[0].userCount = 'assigned_merge_requests'; + createWrapper(mergeRequestMenuGroup); + + expect(findGlBadge(0).text()).toBe(String(userCounts.assigned_merge_requests)); + }); + + it('renders item count if unknown `userCount` prop is defined', () => { + const { count } = mergeRequestMenuGroup[0].items[0]; + mergeRequestMenuGroup[0].items[0].userCount = 'foobar'; + createWrapper(mergeRequestMenuGroup); + + expect(findGlBadge(0).text()).toBe(String(count)); + }); + }); +}); diff --git a/spec/frontend/super_sidebar/components/user_counts_spec.js b/spec/frontend/super_sidebar/components/user_counts_spec.js index a07836a01ace1808c1adfdb0ed18d198818da74d..33e3f0519a9d233e686298c17b6a0782f4b599ce 100644 --- a/spec/frontend/super_sidebar/components/user_counts_spec.js +++ b/spec/frontend/super_sidebar/components/user_counts_spec.js @@ -1,6 +1,7 @@ import { nextTick } from 'vue'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import Counter from '~/super_sidebar/components/counter.vue'; +import MergeRequestMenu from '~/super_sidebar/components/merge_request_menu.vue'; import UserCounts from '~/super_sidebar/components/user_counts.vue'; import { userCounts, useCachedUserCounts } from '~/super_sidebar/user_counts_manager'; import { fetchUserCounts } from '~/super_sidebar/user_counts_fetch'; @@ -18,6 +19,7 @@ describe('UserCounts component', () => { const findIssuesCounter = () => wrapper.findByTestId('issues-shortcut-button'); const findMRsCounter = () => wrapper.findByTestId('merge-requests-shortcut-button'); const findTodosCounter = () => wrapper.findByTestId('todos-shortcut-button'); + const findMergeRequestMenu = () => wrapper.findComponent(MergeRequestMenu); const createWrapper = (props = {}) => { wrapper = shallowMountExtended(UserCounts, { @@ -94,6 +96,16 @@ describe('UserCounts component', () => { expect(findTodosCounter().props('count')).toBe(1); }); }); + + it('passes the "Merge request" menu groups to the merge_request_menu component', () => { + expect(findMergeRequestMenu().props('items')).toBe(mockSidebarData.merge_request_menu); + }); + }); + + it('does not render merge request menu when merge_request_menu is null', () => { + createWrapper({ sidebarData: { ...mockSidebarData, merge_request_menu: null } }); + + expect(findMergeRequestMenu().exists()).toBe(false); }); describe('merge request counts', () => { diff --git a/spec/graphql/graphql_triggers_spec.rb b/spec/graphql/graphql_triggers_spec.rb index 5875516c883a14a34911ad324a3f149c68f47230..666b880e81a6952f1e6868fd2c147310e1905a55 100644 --- a/spec/graphql/graphql_triggers_spec.rb +++ b/spec/graphql/graphql_triggers_spec.rb @@ -191,6 +191,10 @@ let_it_be(:user) { create(:user) } let_it_be(:merge_request) { create(:merge_request) } + before do + stub_feature_flags(merge_request_dashboard: true) + end + it 'triggers the user_merge_request_updated subscription' do expect(GitlabSchema.subscriptions).to receive(:trigger).with( :user_merge_request_updated, @@ -200,6 +204,22 @@ described_class.user_merge_request_updated(user, merge_request) end + + describe 'when merge_request_dashboard is disabled' do + before do + stub_feature_flags(merge_request_dashboard: false) + end + + it 'does not trigger the user_merge_request_updated subscription' do + expect(GitlabSchema.subscriptions).not_to receive(:trigger).with( + :user_merge_request_updated, + { user_id: user.id }, + merge_request + ).and_call_original + + described_class.user_merge_request_updated(user, merge_request) + end + end end describe '.ci_pipeline_status_updated' do diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb index a6e2d2e2acf18b4c76366dcca6eb433024849dcd..dfcd5a1cf57a948bc79e5e37c60cbc496440b0f4 100644 --- a/spec/helpers/merge_requests_helper_spec.rb +++ b/spec/helpers/merge_requests_helper_spec.rb @@ -383,14 +383,17 @@ using RSpec::Parameterized::TableSyntax - where(:query_string, :search_page, :user_dismissed, :should_show) do - { assignee_user: 'test' } | true | false | true - { assignee_user: 'test' } | false | true | false - nil | false | false | false + where(:query_string, :feature_flag_enabled, :search_page, :user_dismissed, :should_show) do + { assignee_user: 'test' } | true | true | false | true + { assignee_user: 'test' } | false | true | false | false + { assignee_user: 'test' } | false | false | false | false + { assignee_user: 'test' } | false | false | true | false + nil | false | false | false | false end with_them do before do + stub_feature_flags(merge_request_dashboard: feature_flag_enabled) allow(helper).to receive(:current_user).and_return(current_user) allow(helper).to receive(:user_dismissed?) .with(Users::CalloutsHelper::NEW_MR_DASHBOARD_BANNER).and_return(user_dismissed) diff --git a/spec/helpers/sidebars_helper_spec.rb b/spec/helpers/sidebars_helper_spec.rb index aca29fb8af622d22d0433eff4a2cfc75096e214e..81ec1a2ce00790f47ba2f9ab4a329cb12a1e634c 100644 --- a/spec/helpers/sidebars_helper_spec.rb +++ b/spec/helpers/sidebars_helper_spec.rb @@ -287,6 +287,50 @@ end end + it 'returns "Merge requests" menu', :use_clean_rails_memory_store_caching do + expect(subject[:merge_request_menu]).to eq([ + { + name: _('Merge requests'), + items: [ + { + text: _('Assigned'), + href: merge_requests_dashboard_path(assignee_username: user.username), + count: 4, + userCount: 'assigned_merge_requests', + extraAttrs: { + 'data-track-action': 'click_link', + 'data-track-label': 'merge_requests_assigned', + 'data-track-property': 'nav_core_menu', + class: 'dashboard-shortcuts-merge_requests' + } + }, + { + text: _('Review requests'), + href: merge_requests_dashboard_path(reviewer_username: user.username), + count: 0, + userCount: 'review_requested_merge_requests', + extraAttrs: { + 'data-track-action': 'click_link', + 'data-track-label': 'merge_requests_to_review', + 'data-track-property': 'nav_core_menu', + class: 'dashboard-shortcuts-review_requests' + } + } + ] + } + ]) + end + + context 'when merge_request_dashboard feature flag is enabled' do + before do + stub_feature_flags(merge_request_dashboard: true) + end + + it 'returns nil for merge_request_menu' do + expect(subject[:merge_request_menu]).to be_nil + end + end + describe '"Create new" menu groups', :use_clean_rails_memory_store_caching do def menu_item(href:, text:, id:, component: nil) { diff --git a/spec/lib/sidebars/your_work/menus/merge_requests_menu_spec.rb b/spec/lib/sidebars/your_work/menus/merge_requests_menu_spec.rb index d0b62c24dab7d51e46bb5c6165c408c260262c79..6d824ba1b4a54cd227de75c7b7b85b1de4147ff9 100644 --- a/spec/lib/sidebars/your_work/menus/merge_requests_menu_spec.rb +++ b/spec/lib/sidebars/your_work/menus/merge_requests_menu_spec.rb @@ -13,4 +13,26 @@ expect(menu.has_pill?).to be true expect(menu.pill_count_field).to eq("total_merge_requests") end + + describe 'submenu items' do + using RSpec::Parameterized::TableSyntax + + where(:order, :title, :key, :count_field) do + 0 | 'Assigned' | :assigned | 'assigned_merge_requests' + 1 | 'Review requests' | :review_requested | 'review_requested_merge_requests' + end + + with_them do + let(:item) { menu.renderable_items[order] } + + it 'renders items in the right order' do + expect(item.title).to eq title + end + + it 'has correct pill settings' do + expect(item.has_pill?).to be true + expect(item.pill_count_field).to eq(count_field) + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cbb3d133eebfcb4916ef3d5da66323fac19d0b92..d7adc18127f4aa1697f7caf3f408d4ca7057dabb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -6769,7 +6769,7 @@ def add_user cache_mock = double expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_merge_requests_count', false]) - expect(cache_mock).to receive(:delete).with(['users', user.id, 'review_requested_open_merge_requests_count']) + expect(cache_mock).to receive(:delete).with(['users', user.id, 'review_requested_open_merge_requests_count', false]) allow(Rails).to receive(:cache).and_return(cache_mock) @@ -6859,38 +6859,40 @@ def add_user let_it_be_with_refind(:archived_project) { create(:project, :public, :archived) } let(:cached_only) { false } - before do - create(:merge_request, source_project: project, author: user, assignees: [user], reviewers: [user]) - create(:merge_request, source_project: project, source_branch: 'feature_conflict', author: user, assignees: [user]) + it 'returns number of open merge requests from non-archived projects' do + create(:merge_request, source_project: project, author: user, assignees: [user]) create(:merge_request, :closed, source_project: project, author: user, assignees: [user]) create(:merge_request, source_project: archived_project, author: user, assignees: [user]) - mr = create(:merge_request, :unique_branches, source_project: project, author: user, assignees: [user], reviewers: [user]) - mr2 = create(:merge_request, :unique_branches, source_project: project, author: user, assignees: [user], reviewers: [user]) - - mr.merge_request_reviewers.update_all(state: :reviewed) - mr2.merge_request_reviewers.update_all(state: :requested_changes) + is_expected.to eq 1 end - it 'returns number of open merge requests from non-archived projects where there are no reviewers' do - is_expected.to eq 4 + context 'when fetching only cached' do + let(:cached_only) { true } + + it 'returns nil' do + is_expected.to be_nil + end end - context 'when merge_request_dashboard_list_type is role_based' do + context 'when merge_request_dashboard_author_or_assignee feature flag is enabled' do before do - user.user_preference.update!(merge_request_dashboard_list_type: 'role_based') + stub_feature_flags(merge_request_dashboard: true) end - it 'returns number of open merge requests from non-archived projects' do - is_expected.to eq 4 - end - end + it 'returns number of open merge requests from non-archived projects where there are no reviewers' do + create(:merge_request, source_project: project, author: user, assignees: [user], reviewers: [user]) + create(:merge_request, source_project: project, source_branch: 'feature_conflict', author: user, assignees: [user]) + create(:merge_request, :closed, source_project: project, author: user, assignees: [user]) + create(:merge_request, source_project: archived_project, author: user, assignees: [user]) - context 'when fetching only cached' do - let(:cached_only) { true } + mr = create(:merge_request, :unique_branches, source_project: project, author: user, assignees: [user], reviewers: [user]) + mr2 = create(:merge_request, :unique_branches, source_project: project, author: user, assignees: [user], reviewers: [user]) - it 'returns nil' do - is_expected.to be_nil + mr.merge_request_reviewers.update_all(state: :reviewed) + mr2.merge_request_reviewers.update_all(state: :requested_changes) + + is_expected.to eq 3 end end @@ -6962,12 +6964,7 @@ def add_user create(:merge_request, source_project: archived_project, author: user, reviewers: [user]) end - it 'returns number of open merge requests from non-archived projects where a reviewer has not reviewed' do - mr2 = create(:merge_request, :unique_branches, source_project: project, author: user, reviewers: [user]) - - mr.merge_request_reviewers.update_all(state: :unreviewed) - mr2.merge_request_reviewers.update_all(state: :requested_changes) - + it 'returns number of open merge requests from non-archived projects' do is_expected.to eq 1 end @@ -6978,6 +6975,21 @@ def add_user is_expected.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 'returns number of open merge requests from non-archived projects where a reviewer has not reviewed' do + mr2 = create(:merge_request, :unique_branches, source_project: project, author: user, reviewers: [user]) + + mr.merge_request_reviewers.update_all(state: :unreviewed) + mr2.merge_request_reviewers.update_all(state: :requested_changes) + + is_expected.to eq 1 + end + end end describe '#assigned_open_issues_count' do diff --git a/spec/requests/api/graphql/merge_requests/assignee_or_reviewer_merge_requests_spec.rb b/spec/requests/api/graphql/merge_requests/assignee_or_reviewer_merge_requests_spec.rb index d5ea63c96acc6137bcb0e0f92224fbc0e2b94ecd..709a7439f2629d2cb272024279608860e568fece 100644 --- a/spec/requests/api/graphql/merge_requests/assignee_or_reviewer_merge_requests_spec.rb +++ b/spec/requests/api/graphql/merge_requests/assignee_or_reviewer_merge_requests_spec.rb @@ -39,46 +39,64 @@ def query(params = {}) reviewer_change_requested.merge_request_reviewers[0].update!(state: :requested_changes) 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 merge_request_dashboard feature flag is disabled' do + before do + stub_feature_flags(merge_request_dashboard: false) + end - context 'when assigned_review_states argument is sent' do it do - post_graphql(query({ assigned_review_states: [:REVIEWED] }), current_user: current_user) + 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) - ) + expect(merge_requests).to be_nil end end - context 'when reviewer_review_states argument is sent' do + 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({ reviewer_review_states: [:REQUESTED_CHANGES] }), current_user: current_user) + 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 - 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) + 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), - a_hash_including('id' => reviewer_change_requested.to_global_id.to_s) - ) + 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 diff --git a/spec/requests/dashboard_controller_spec.rb b/spec/requests/dashboard_controller_spec.rb index 2d98e6e455cb14396e8ef246427f26c059b5e640..3ac19b5da7e2614ff02f3b4fc3c3c9ae25ceb81c 100644 --- a/spec/requests/dashboard_controller_spec.rb +++ b/spec/requests/dashboard_controller_spec.rb @@ -30,20 +30,28 @@ def request context 'merge requests dashboard' do let_it_be(:current_user) { create(:user) } - before do - sign_in current_user - end - it_behaves_like 'rate limited endpoint', rate_limit_key: :search_rate_limit do + before do + sign_in current_user + end + def request get merge_requests_dashboard_path, params: { scope: 'all', search: 'test' } end end - it 'redirects to search page with the current query string' do - get merge_requests_dashboard_path, params: { assignee_username: current_user.username } + context 'when merge_request_dashboard feature flag is enabled' do + before do + stub_feature_flags(merge_request_dashboard: true) - expect(response).to redirect_to(merge_requests_search_dashboard_path(params: { assignee_username: current_user.username })) + sign_in current_user + end + + it 'redirects to search page with the current query string' do + get merge_requests_dashboard_path, params: { assignee_username: current_user.username } + + expect(response).to redirect_to(merge_requests_search_dashboard_path(params: { assignee_username: current_user.username })) + end end end diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb index 8d1dc64397de105822013c3005250b254712fca3..1be4e6ce1923fbc6d9c641f665b0e1e2c0395195 100644 --- a/spec/services/draft_notes/publish_service_spec.rb +++ b/spec/services/draft_notes/publish_service_spec.rb @@ -164,6 +164,8 @@ def publish(draft: nil) it 'invalidates cache counts' do expect(merge_request.assignees).to all(receive(:invalidate_merge_request_cache_counts)) + stub_feature_flags(merge_request_dashboard: true) + publish end diff --git a/spec/services/merge_requests/handle_assignees_change_service_spec.rb b/spec/services/merge_requests/handle_assignees_change_service_spec.rb index 9c130b6362c931fb8b92bcc6694c920c27c600a0..c82e3dab15000e8a2798895a36218aef4ec34fb5 100644 --- a/spec/services/merge_requests/handle_assignees_change_service_spec.rb +++ b/spec/services/merge_requests/handle_assignees_change_service_spec.rb @@ -101,10 +101,16 @@ def execute execute end - it 'invalidates cache counts' do - expect(merge_request.assignees).to all(receive(:invalidate_merge_request_cache_counts)) + context 'when merge_request_dashboard feature flag is enabled' do + before do + stub_feature_flags(merge_request_dashboard: true) + end - execute + it 'invalidates cache counts' do + expect(merge_request.assignees).to all(receive(:invalidate_merge_request_cache_counts)) + + execute + end end it 'invalidates cache counts for old assignees' do diff --git a/spec/services/merge_requests/request_review_service_spec.rb b/spec/services/merge_requests/request_review_service_spec.rb index b039431777c1715d848ea47741c884cdec9826c3..c06eeabcbdab21ba7eebe07a5b309fa0b2b5afc9 100644 --- a/spec/services/merge_requests/request_review_service_spec.rb +++ b/spec/services/merge_requests/request_review_service_spec.rb @@ -94,11 +94,17 @@ service.execute(merge_request, user) end - it 'invalidates cache counts' do - expect(user).to receive(:invalidate_merge_request_cache_counts) - expect(current_user).to receive(:invalidate_merge_request_cache_counts) + context 'when merge_request_dashboard feature flag is enabled' do + before do + stub_feature_flags(merge_request_dashboard: true) + end - service.execute(merge_request, user) + it 'invalidates cache counts' do + expect(user).to receive(:invalidate_merge_request_cache_counts) + expect(current_user).to receive(:invalidate_merge_request_cache_counts) + + service.execute(merge_request, user) + end end end end diff --git a/spec/services/merge_requests/update_reviewer_state_service_spec.rb b/spec/services/merge_requests/update_reviewer_state_service_spec.rb index 4d4c004c79ccaaf1cb4176426b2abc359f4a3fa4..920c2fb952185e9a6184f4ac556e5cd3a2072536 100644 --- a/spec/services/merge_requests/update_reviewer_state_service_spec.rb +++ b/spec/services/merge_requests/update_reviewer_state_service_spec.rb @@ -85,16 +85,22 @@ result end - it 'invalidates cache counts for all assignees' do - expect(merge_request.assignees).to all(receive(:invalidate_merge_request_cache_counts)) + context 'when merge_request_dashboard feature flag is enabled' do + before do + stub_feature_flags(merge_request_dashboard: true) + end - expect(result[:status]).to eq :success - end + it 'invalidates cache counts for all assignees' do + expect(merge_request.assignees).to all(receive(:invalidate_merge_request_cache_counts)) + + expect(result[:status]).to eq :success + end - it 'invalidates cache counts for current user' do - expect(current_user).to receive(:invalidate_merge_request_cache_counts) + it 'invalidates cache counts for current user' do + expect(current_user).to receive(:invalidate_merge_request_cache_counts) - expect(result[:status]).to eq :success + expect(result[:status]).to eq :success + end end context 'when reviewer has approved' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 536a5d6dee12d8f4f83f51a4d817e507fec74495..1269f80440c29ceadec8cb80435dbe1fa256b482 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -328,6 +328,9 @@ # Disable suspending ClickHouse data ingestion workers stub_feature_flags(suspend_click_house_data_ingestion: false) + # Experimental merge request dashboard + stub_feature_flags(merge_request_dashboard: false) + # This feature flag allows enabling self-hosted features on Staging Ref: https://gitlab.com/gitlab-org/gitlab/-/issues/497784 stub_feature_flags(allow_self_hosted_features_for_com: false) diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index 649978be7dcc0279e09a3dc203a36d67db440f2c..a989c4644f363d0f0990108c5de82f04f5338b71 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -242,7 +242,10 @@ }, { nav_item: _("Merge requests"), - nav_sub_items: [] + nav_sub_items: [ + _('Assigned'), + _('Review requests') + ] }, { nav_item: _("To-Do List"),