From 1d862c199eb1edf5c54a1531f682e372cc6705ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com> Date: Wed, 18 Oct 2023 21:23:26 +0800 Subject: [PATCH 1/9] Support source branch param on MR list page Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134555 EE: false --- .../add_extra_tokens_for_merge_requests.js | 19 +++++++++++++++---- app/finders/merge_requests_finder.rb | 1 + .../add_extra_tokens_for_merge_requests.js | 4 ++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js b/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js index 4d3647cdf5cdd8..74d91734630e7d 100644 --- a/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js +++ b/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js @@ -5,9 +5,10 @@ import { TOKEN_TYPE_APPROVED_BY, TOKEN_TYPE_REVIEWER, TOKEN_TYPE_TARGET_BRANCH, + TOKEN_TYPE_SOURCE_BRANCH, } from '~/vue_shared/components/filtered_search_bar/constants'; -export default (IssuableTokenKeys, disableTargetBranchFilter = false) => { +export default (IssuableTokenKeys, disableBranchFilter = false) => { const reviewerToken = { formattedKey: TOKEN_TITLE_REVIEWER, key: TOKEN_TYPE_REVIEWER, @@ -57,7 +58,7 @@ export default (IssuableTokenKeys, disableTargetBranchFilter = false) => { IssuableTokenKeys.tokenKeysWithAlternative.push(draftToken.token); IssuableTokenKeys.conditions.push(...draftToken.conditions); - if (!disableTargetBranchFilter) { + if (!disableBranchFilter) { const targetBranchToken = { formattedKey: __('Target-Branch'), key: TOKEN_TYPE_TARGET_BRANCH, @@ -68,8 +69,18 @@ export default (IssuableTokenKeys, disableTargetBranchFilter = false) => { tag: 'branch', }; - IssuableTokenKeys.tokenKeys.push(targetBranchToken); - IssuableTokenKeys.tokenKeysWithAlternative.push(targetBranchToken); + const sourceBranchToken = { + formattedKey: __('Source-Branch'), + key: TOKEN_TYPE_SOURCE_BRANCH, + type: 'string', + param: '', + symbol: '', + icon: 'branch', + tag: 'branch', + }; + + IssuableTokenKeys.tokenKeys.push(targetBranchToken, sourceBranchToken); + IssuableTokenKeys.tokenKeysWithAlternative.push(targetBranchToken, sourceBranchToken); } const approvedToken = { diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index f7ee90ab8705c0..ab3521b8123897 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -46,6 +46,7 @@ def self.scalar_params :merged_before, :reviewer_id, :reviewer_username, + :source_branch, :target_branch, :wip ] diff --git a/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js b/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js index d2e5e3fcaa82ad..615ae6e448b4e1 100644 --- a/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js +++ b/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js @@ -47,8 +47,8 @@ const approvers = { }, }; -export default (IssuableTokenKeys, disableTargetBranchFilter = false) => { - addExtraTokensForMergeRequests(IssuableTokenKeys, disableTargetBranchFilter); +export default (IssuableTokenKeys, disableBranchFilter = false) => { + addExtraTokensForMergeRequests(IssuableTokenKeys, disableBranchFilter); const tokenPosition = 3; IssuableTokenKeys.tokenKeys.splice(tokenPosition, 0, ...[approvers.token]); -- GitLab From 15af64949d89195bc2d6bc10efbcaf0bcc46bb72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com> Date: Thu, 19 Oct 2023 00:22:04 +0800 Subject: [PATCH 2/9] Add autocomplete/merge_request_source_branches --- .../available_dropdown_mappings.js | 28 +++++++++++++-- app/controllers/autocomplete_controller.rb | 36 +++++++++++++------ app/models/merge_request.rb | 8 +++++ .../shared/issuable/_search_bar.html.haml | 5 +++ config/routes.rb | 1 + locale/gitlab.pot | 3 ++ 6 files changed, 67 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/filtered_search/available_dropdown_mappings.js b/app/assets/javascripts/filtered_search/available_dropdown_mappings.js index 892e9130fe8d4e..1069aa77af62d4 100644 --- a/app/assets/javascripts/filtered_search/available_dropdown_mappings.js +++ b/app/assets/javascripts/filtered_search/available_dropdown_mappings.js @@ -11,6 +11,7 @@ import { TOKEN_TYPE_RELEASE, TOKEN_TYPE_REVIEWER, TOKEN_TYPE_TARGET_BRANCH, + TOKEN_TYPE_SOURCE_BRANCH, } from '~/vue_shared/components/filtered_search_bar/constants'; import DropdownEmoji from './dropdown_emoji'; import DropdownHint from './dropdown_hint'; @@ -157,6 +158,15 @@ export default class AvailableDropdownMappings { }, element: this.container.querySelector('#js-dropdown-target-branch'), }, + [TOKEN_TYPE_SOURCE_BRANCH]: { + reference: null, + gl: DropdownNonUser, + extraArguments: { + endpoint: this.getMergeRequestSourceBranchesEndpoint(), + symbol: '', + }, + element: this.container.querySelector('#js-dropdown-source-branch'), + }, environment: { reference: null, gl: DropdownNonUser, @@ -197,9 +207,21 @@ export default class AvailableDropdownMappings { } getMergeRequestTargetBranchesEndpoint() { - const endpoint = `${ - gon.relative_url_root || '' - }/-/autocomplete/merge_request_target_branches.json`; + return this.getMergeRequestBranchesEndpoint(false, true); + } + + getMergeRequestSourceBranchesEndpoint() { + return this.getMergeRequestBranchesEndpoint(true, false); + } + + getMergeRequestBranchesEndpoint(sourcesBranch = false, targetBranch = false) { + let endpointPath = ''; + if (sourcesBranch) { + endpointPath = '/-/autocomplete/merge_request_source_branches.json'; + } else if (targetBranch) { + endpointPath = '/-/autocomplete/merge_request_target_branches.json'; + } + const endpoint = `${gon.relative_url_root || ''}${endpointPath}`; const params = { group_id: this.getGroupId(), diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index c9cb1ca14e2813..1c2bd10bc8143d 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -3,16 +3,18 @@ class AutocompleteController < ApplicationController include SearchRateLimitable - skip_before_action :authenticate_user!, only: [:users, :award_emojis, :merge_request_target_branches] + skip_before_action :authenticate_user!, only: [ + :users, :award_emojis, :merge_request_target_branches, :merge_request_source_branches + ] before_action :check_search_rate_limit!, only: [:users, :projects] feature_category :user_profile, [:users, :user] feature_category :groups_and_projects, [:projects] feature_category :team_planning, [:award_emojis] - feature_category :code_review_workflow, [:merge_request_target_branches] + feature_category :code_review_workflow, [:merge_request_target_branches, :merge_request_source_branches] feature_category :continuous_delivery, [:deploy_keys_with_owners] - urgency :low, [:merge_request_target_branches, :deploy_keys_with_owners, :users] + urgency :low, [:merge_request_target_branches, :merge_request_source_branches, :deploy_keys_with_owners, :users] urgency :low, [:award_emojis] urgency :medium, [:projects] @@ -62,14 +64,11 @@ def award_emojis end def merge_request_target_branches - if target_branch_params.present? - merge_requests = MergeRequestsFinder.new(current_user, target_branch_params).execute - target_branches = merge_requests.recent_target_branches + merge_request_branches(target: true) + end - render json: target_branches.map { |target_branch| { title: target_branch } } - else - render json: { error: _('At least one of group_id or project_id must be specified') }, status: :bad_request - end + def merge_request_source_branches + merge_request_branches(source: true) end def deploy_keys_with_owners @@ -90,7 +89,7 @@ def project .execute end - def target_branch_params + def branch_params params.permit(:group_id, :project_id).select { |_, v| v.present? } end @@ -98,6 +97,21 @@ def target_branch_params def presented_suggested_users [] end + + def merge_request_branches(source: false, target: false) + if branch_params.present? + merge_requests = MergeRequestsFinder.new(current_user, branch_params).execute + + branches = [] + + branches.concat(merge_requests.recent_source_branches) if source + branches.concat(merge_requests.recent_target_branches) if target + + render json: branches.map { |branch| { title: branch } } + else + render json: { error: _('At least one of group_id or project_id must be specified') }, status: :bad_request + end + end end AutocompleteController.prepend_mod_with('AutocompleteController') diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a751ed0c94d634..524a9b8074b100 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -529,6 +529,14 @@ def self.recent_target_branches(limit: 100) .pluck(:target_branch) end + def self.recent_source_branches(limit: 100) + group(:source_branch) + .select(:source_branch) + .reorder(arel_table[:updated_at].maximum.desc) + .limit(limit) + .pluck(:source_branch) + end + def self.sort_by_attribute(method, excluded_labels: []) case method.to_s when 'merged_at', 'merged_at_asc' then order_merged_at_asc diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index 86aaa5128a8f52..52c8a4d41237d0 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -185,6 +185,11 @@ %li.filter-dropdown-item %button.btn.btn-link.js-data-value.monospace {{title}} + #js-dropdown-source-branch.filtered-search-input-dropdown-menu.dropdown-menu + %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } + %li.filter-dropdown-item + %button.btn.btn-link.js-data-value.monospace + {{title}} #js-dropdown-environment.filtered-search-input-dropdown-menu.dropdown-menu %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } %li.filter-dropdown-item diff --git a/config/routes.rb b/config/routes.rb index 2c0ae3d0763b29..925ef0c62e8515 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -88,6 +88,7 @@ get '/autocomplete/projects' => 'autocomplete#projects' get '/autocomplete/award_emojis' => 'autocomplete#award_emojis' get '/autocomplete/merge_request_target_branches' => 'autocomplete#merge_request_target_branches' + get '/autocomplete/merge_request_source_branches' => 'autocomplete#merge_request_source_branches' get '/autocomplete/deploy_keys_with_owners' => 'autocomplete#deploy_keys_with_owners' Gitlab.ee do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2185dc9b9410df..cac0c232a907b3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -45892,6 +45892,9 @@ msgstr "" msgid "Source project cannot be found." msgstr "" +msgid "Source-Branch" +msgstr "" + msgid "SourceEditor|\"el\" parameter is required for createInstance()" msgstr "" -- GitLab From 416c0911c1c53fc6235316c9782329313f5d4dd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com> Date: Thu, 19 Oct 2023 00:24:43 +0800 Subject: [PATCH 3/9] Add MergeRequestsFinder#by_negated_target_branch --- app/finders/merge_requests_finder.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index ab3521b8123897..e8993a1c115255 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -83,6 +83,7 @@ def filter_negated_items(items) items = by_negated_reviewer(items) items = by_negated_approved_by(items) by_negated_target_branch(items) + by_negated_source_branch(items) end private @@ -133,6 +134,12 @@ def by_negated_target_branch(items) items.where.not(target_branch: not_params[:target_branch]) end + + def by_negated_source_branch(items) + return items unless not_params[:source_branch] + + items.where.not(source_branch: not_params[:source_branch]) + end # rubocop: enable CodeReuse/ActiveRecord def by_negated_approved_by(items) -- GitLab From 2f2bb72624d169ed4ada82bf25799416eca534d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com> Date: Thu, 19 Oct 2023 22:16:37 +0800 Subject: [PATCH 4/9] Add spec for source branch filter --- app/finders/merge_requests_finder.rb | 2 +- .../autocomplete_controller_spec.rb | 30 ++++++--- .../user_filters_by_srouce_branch_spec.rb | 65 +++++++++++++++++++ spec/models/merge_request_spec.rb | 18 +++-- 4 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 spec/features/merge_requests/user_filters_by_srouce_branch_spec.rb diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index e8993a1c115255..b7de1c08f86284 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -82,7 +82,7 @@ def filter_negated_items(items) items = super(items) items = by_negated_reviewer(items) items = by_negated_approved_by(items) - by_negated_target_branch(items) + items = by_negated_target_branch(items) by_negated_source_branch(items) end diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index a66cb4364d7c50..e4ade46bf040c1 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -454,12 +454,10 @@ def request end end - context 'Get merge_request_target_branches', feature_category: :code_review_workflow do - let!(:merge_request) { create(:merge_request, source_project: project, target_branch: 'feature') } - + shared_examples 'Get merge_request_{}_branches' do |path, expected_result| context 'anonymous user' do it 'returns empty json' do - get :merge_request_target_branches, params: { project_id: project.id } + get path, params: { project_id: project.id } expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_empty @@ -470,7 +468,7 @@ def request it 'returns empty json' do sign_in(create(:user)) - get :merge_request_target_branches, params: { project_id: project.id } + get path, params: { project_id: project.id } expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_empty @@ -491,7 +489,7 @@ def request it 'returns an error' do sign_in(user) - get :merge_request_target_branches, params: params + get path, params: params expect(response).to have_gitlab_http_status(:bad_request) expect(json_response).to eq({ 'error' => 'At least one of group_id or project_id must be specified' }) @@ -503,10 +501,10 @@ def request it 'returns json' do sign_in(user) - get :merge_request_target_branches, params: { project_id: project.id } + get path, params: { project_id: project.id } expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to contain_exactly({ 'title' => 'feature' }) + expect(json_response).to contain_exactly(expected_result) end end @@ -520,11 +518,23 @@ def request sign_in(user) - get :merge_request_target_branches, params: { group_id: group.id } + get path, params: { group_id: group.id } expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to contain_exactly({ 'title' => 'feature' }) + expect(json_response).to contain_exactly(expected_result) end end end + + context 'Get merge_request_target_branches', feature_category: :code_review_workflow do + let!(:merge_request) { create(:merge_request, source_project: project, target_branch: 'test_target_branch') } + + it_behaves_like 'Get merge_request_{}_branches', :merge_request_target_branches, { 'title' => 'test_target_branch' } + end + + context 'Get merge_request_source_branches', feature_category: :code_review_workflow do + let!(:merge_request) { create(:merge_request, source_project: project, source_branch: 'test_source_branch') } + + it_behaves_like 'Get merge_request_{}_branches', :merge_request_source_branches, { 'title' => 'test_source_branch' } + end end diff --git a/spec/features/merge_requests/user_filters_by_srouce_branch_spec.rb b/spec/features/merge_requests/user_filters_by_srouce_branch_spec.rb new file mode 100644 index 00000000000000..64dbe18d9311d2 --- /dev/null +++ b/spec/features/merge_requests/user_filters_by_srouce_branch_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Merge Requests > User filters by source branch', :js, feature_category: :code_review_workflow do + include FilteredSearchHelpers + + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:user) { project.creator } + + let_it_be(:mr1) do + create(:merge_request, source_project: project, + target_project: project, source_branch: 'source1', target_branch: 'target1') + end + + let_it_be(:mr2) do + create(:merge_request, source_project: project, + target_project: project, source_branch: 'source2', target_branch: 'target1') + end + + before do + sign_in(user) + visit project_merge_requests_path(project) + end + + context 'when filtering by source-branch:source1' do + it 'applies the filter' do + input_filtered_search('source-branch:=source1') + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).to have_content mr1.title + expect(page).not_to have_content mr2.title + end + end + + context 'when filtering by source-branch:source2' do + it 'applies the filter' do + input_filtered_search('source-branch:=source2') + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).not_to have_content mr1.title + expect(page).to have_content mr2.title + end + end + + context 'when filtering by source-branch:non-exists-branch' do + it 'applies the filter' do + input_filtered_search('source-branch:=non-exists-branch') + + expect(page).to have_issuable_counts(open: 0, closed: 0, all: 0) + expect(page).not_to have_content mr1.title + expect(page).not_to have_content mr2.title + end + end + + context 'when filtering by source-branch:!=source1' do + it 'applies the filter' do + input_filtered_search('source-branch:!=source1') + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).not_to have_content mr1.title + expect(page).to have_content mr2.title + end + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 04d0226ac22d5a..a22cbc0b55cc17 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -725,12 +725,17 @@ def create_merge_request_with_diffs(source_branch, diffs: 2) end end - describe '.recent_target_branches' do + describe '.recent_target_branches and .recent_source_branches' do + def create_mr(source_branch, target_branch) + create(:merge_request, :opened, source_project: project, + target_branch: target_branch, source_branch: source_branch) + end + let(:project) { create(:project) } - let!(:merge_request1) { create(:merge_request, :opened, source_project: project, target_branch: 'feature') } - let!(:merge_request2) { create(:merge_request, :closed, source_project: project, target_branch: 'merge-test') } - let!(:merge_request3) { create(:merge_request, :opened, source_project: project, target_branch: 'fix') } - let!(:merge_request4) { create(:merge_request, :closed, source_project: project, target_branch: 'feature') } + let!(:merge_request1) { create_mr('source1', 'feature') } + let!(:merge_request2) { create_mr('source2', 'merge-test') } + let!(:merge_request3) { create_mr('source3', 'fix') } + let!(:merge_request4) { create_mr('source4', 'feature') } before do merge_request1.update_columns(updated_at: 1.day.since) @@ -739,8 +744,9 @@ def create_merge_request_with_diffs(source_branch, diffs: 2) merge_request4.update_columns(updated_at: 4.days.since) end - it 'returns target branches sort by updated at desc' do + it 'returns branches sort by updated at desc' do expect(described_class.recent_target_branches).to match_array(%w[feature merge-test fix]) + expect(described_class.recent_source_branches).to match_array(%w[source1 source2 source3 source4]) end end -- GitLab From b6a22d6a0f9bc20bae56bff5fc4b0dd150983b99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com> Date: Fri, 20 Oct 2023 10:26:17 +0800 Subject: [PATCH 5/9] Fix typo "srouce" to "source" --- ...rouce_branch_spec.rb => user_filters_by_source_branch_spec.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/features/merge_requests/{user_filters_by_srouce_branch_spec.rb => user_filters_by_source_branch_spec.rb} (100%) diff --git a/spec/features/merge_requests/user_filters_by_srouce_branch_spec.rb b/spec/features/merge_requests/user_filters_by_source_branch_spec.rb similarity index 100% rename from spec/features/merge_requests/user_filters_by_srouce_branch_spec.rb rename to spec/features/merge_requests/user_filters_by_source_branch_spec.rb -- GitLab From f7d87b4857a8a5e8fe81b4ef5fb355b046320775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com> Date: Mon, 23 Oct 2023 23:46:04 +0800 Subject: [PATCH 6/9] Add status for merge_request_spec --- spec/models/merge_request_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a22cbc0b55cc17..8e5d3fddae1f9e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -726,16 +726,16 @@ def create_merge_request_with_diffs(source_branch, diffs: 2) end describe '.recent_target_branches and .recent_source_branches' do - def create_mr(source_branch, target_branch) - create(:merge_request, :opened, source_project: project, + def create_mr(source_branch, target_branch, status) + create(:merge_request, status, source_project: project, target_branch: target_branch, source_branch: source_branch) end let(:project) { create(:project) } - let!(:merge_request1) { create_mr('source1', 'feature') } - let!(:merge_request2) { create_mr('source2', 'merge-test') } - let!(:merge_request3) { create_mr('source3', 'fix') } - let!(:merge_request4) { create_mr('source4', 'feature') } + let!(:merge_request1) { create_mr('source1', 'feature', :opened) } + let!(:merge_request2) { create_mr('source2', 'merge-test', :closed) } + let!(:merge_request3) { create_mr('source3', 'fix', :opened) } + let!(:merge_request4) { create_mr('source4', 'feature', :closed) } before do merge_request1.update_columns(updated_at: 1.day.since) -- GitLab From 14e77ffce90c58abb3581b688fb84e93774ab21d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com> Date: Tue, 24 Oct 2023 10:13:50 +0800 Subject: [PATCH 7/9] Use "let_it_be" instead of "let!" --- .../autocomplete_controller_spec.rb | 112 +++++++++--------- 1 file changed, 55 insertions(+), 57 deletions(-) diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index e4ade46bf040c1..cbe0319a78d427 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -454,87 +454,85 @@ def request end end - shared_examples 'Get merge_request_{}_branches' do |path, expected_result| - context 'anonymous user' do - it 'returns empty json' do - get path, params: { project_id: project.id } + context 'GET branches', feature_category: :code_review_workflow do + let_it_be(:merge_request) do + create(:merge_request, source_project: project, + source_branch: 'test_source_branch', target_branch: 'test_target_branch') + end - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_empty + shared_examples 'Get merge_request_{}_branches' do |path, expected_result| + context 'anonymous user' do + it 'returns empty json' do + get path, params: { project_id: project.id } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + end end - end - context 'user without any accessible merge requests' do - it 'returns empty json' do - sign_in(create(:user)) + context 'user without any accessible merge requests' do + it 'returns empty json' do + sign_in(create(:user)) - get path, params: { project_id: project.id } + get path, params: { project_id: project.id } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_empty + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + end end - end - context 'user with an accessible merge request but no scope' do - where( - params: [ - {}, - { group_id: ' ' }, - { project_id: ' ' }, - { group_id: ' ', project_id: ' ' } - ] - ) - - with_them do - it 'returns an error' do - sign_in(user) + context 'user with an accessible merge request but no scope' do + where( + params: [ + {}, + { group_id: ' ' }, + { project_id: ' ' }, + { group_id: ' ', project_id: ' ' } + ] + ) - get path, params: params + with_them do + it 'returns an error' do + sign_in(user) - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response).to eq({ 'error' => 'At least one of group_id or project_id must be specified' }) + get path, params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ 'error' => 'At least one of group_id or project_id must be specified' }) + end end end - end - context 'user with an accessible merge request by project' do - it 'returns json' do - sign_in(user) + context 'user with an accessible merge request by project' do + it 'returns json' do + sign_in(user) - get path, params: { project_id: project.id } + get path, params: { project_id: project.id } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to contain_exactly(expected_result) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to contain_exactly(expected_result) + end end - end - context 'user with an accessible merge request by group' do - let(:group) { create(:group) } - let(:project) { create(:project, namespace: group) } - let(:user) { create(:user) } + context 'user with an accessible merge request by group' do + let(:group) { create(:group) } + let(:user) { create(:user) } - it 'returns json' do - group.add_owner(user) + it 'returns json' do + project.update!(namespace: group) + group.add_owner(user) - sign_in(user) + sign_in(user) - get path, params: { group_id: group.id } + get path, params: { group_id: group.id } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to contain_exactly(expected_result) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to contain_exactly(expected_result) + end end end - end - - context 'Get merge_request_target_branches', feature_category: :code_review_workflow do - let!(:merge_request) { create(:merge_request, source_project: project, target_branch: 'test_target_branch') } it_behaves_like 'Get merge_request_{}_branches', :merge_request_target_branches, { 'title' => 'test_target_branch' } - end - - context 'Get merge_request_source_branches', feature_category: :code_review_workflow do - let!(:merge_request) { create(:merge_request, source_project: project, source_branch: 'test_source_branch') } - it_behaves_like 'Get merge_request_{}_branches', :merge_request_source_branches, { 'title' => 'test_source_branch' } end end -- GitLab From 757595de22ad0fd416a3e938269a37b0a7ac050d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com> Date: Tue, 24 Oct 2023 16:34:56 +0800 Subject: [PATCH 8/9] Add merged status into MR spec --- .../user_filters_by_source_branch_spec.rb | 26 +++++++++---------- spec/models/merge_request_spec.rb | 25 +++++++++++------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/spec/features/merge_requests/user_filters_by_source_branch_spec.rb b/spec/features/merge_requests/user_filters_by_source_branch_spec.rb index 64dbe18d9311d2..7eade94de2ad5e 100644 --- a/spec/features/merge_requests/user_filters_by_source_branch_spec.rb +++ b/spec/features/merge_requests/user_filters_by_source_branch_spec.rb @@ -5,18 +5,18 @@ RSpec.describe 'Merge Requests > User filters by source branch', :js, feature_category: :code_review_workflow do include FilteredSearchHelpers + def create_mr(source_branch, target_branch, status) + create(:merge_request, status, source_project: project, + target_branch: target_branch, source_branch: source_branch) + end + let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:user) { project.creator } - let_it_be(:mr1) do - create(:merge_request, source_project: project, - target_project: project, source_branch: 'source1', target_branch: 'target1') - end - - let_it_be(:mr2) do - create(:merge_request, source_project: project, - target_project: project, source_branch: 'source2', target_branch: 'target1') - end + let_it_be(:mr1) { create_mr('source1', 'target1', :opened) } + let_it_be(:mr2) { create_mr('source2', 'target1', :opened) } + let_it_be(:mr3) { create_mr('source1', 'target2', :merged) } + let_it_be(:mr4) { create_mr('source1', 'target2', :closed) } before do sign_in(user) @@ -27,7 +27,7 @@ it 'applies the filter' do input_filtered_search('source-branch:=source1') - expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).to have_issuable_counts(open: 1, merged: 1, closed: 1, all: 3) expect(page).to have_content mr1.title expect(page).not_to have_content mr2.title end @@ -37,7 +37,7 @@ it 'applies the filter' do input_filtered_search('source-branch:=source2') - expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).to have_issuable_counts(open: 1, merged: 0, closed: 0, all: 1) expect(page).not_to have_content mr1.title expect(page).to have_content mr2.title end @@ -47,7 +47,7 @@ it 'applies the filter' do input_filtered_search('source-branch:=non-exists-branch') - expect(page).to have_issuable_counts(open: 0, closed: 0, all: 0) + expect(page).to have_issuable_counts(open: 0, merged: 0, closed: 0, all: 0) expect(page).not_to have_content mr1.title expect(page).not_to have_content mr2.title end @@ -57,7 +57,7 @@ it 'applies the filter' do input_filtered_search('source-branch:!=source1') - expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).to have_issuable_counts(open: 1, merged: 0, closed: 0, all: 1) expect(page).not_to have_content mr1.title expect(page).to have_content mr2.title end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 8e5d3fddae1f9e..d3c32da284232f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -726,27 +726,34 @@ def create_merge_request_with_diffs(source_branch, diffs: 2) end describe '.recent_target_branches and .recent_source_branches' do - def create_mr(source_branch, target_branch, status) - create(:merge_request, status, source_project: project, - target_branch: target_branch, source_branch: source_branch) + def create_mr(source_branch, target_branch, status, remove_source_branch = false) + if remove_source_branch + create(:merge_request, status, :remove_source_branch, source_project: project, + target_branch: target_branch, source_branch: source_branch) + else + create(:merge_request, status, source_project: project, + target_branch: target_branch, source_branch: source_branch) + end end let(:project) { create(:project) } - let!(:merge_request1) { create_mr('source1', 'feature', :opened) } - let!(:merge_request2) { create_mr('source2', 'merge-test', :closed) } - let!(:merge_request3) { create_mr('source3', 'fix', :opened) } - let!(:merge_request4) { create_mr('source4', 'feature', :closed) } + let!(:merge_request1) { create_mr('source1', 'target1', :opened) } + let!(:merge_request2) { create_mr('source2', 'target2', :closed) } + let!(:merge_request3) { create_mr('source3', 'target3', :opened) } + let!(:merge_request4) { create_mr('source4', 'target1', :closed) } + let!(:merge_request5) { create_mr('source5', 'target4', :merged, true) } before do merge_request1.update_columns(updated_at: 1.day.since) merge_request2.update_columns(updated_at: 2.days.since) merge_request3.update_columns(updated_at: 3.days.since) merge_request4.update_columns(updated_at: 4.days.since) + merge_request5.update_columns(updated_at: 5.days.since) end it 'returns branches sort by updated at desc' do - expect(described_class.recent_target_branches).to match_array(%w[feature merge-test fix]) - expect(described_class.recent_source_branches).to match_array(%w[source1 source2 source3 source4]) + expect(described_class.recent_target_branches).to match_array(%w[target1 target2 target3 target4]) + expect(described_class.recent_source_branches).to match_array(%w[source1 source2 source3 source4 source5]) end end -- GitLab From e85c02d5fcda674dbe6ffdee22100be99cf4c587 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com> Date: Wed, 25 Oct 2023 09:31:45 +0800 Subject: [PATCH 9/9] Refactor function getMergeRequestBranchesEndpoint --- .../available_dropdown_mappings.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/filtered_search/available_dropdown_mappings.js b/app/assets/javascripts/filtered_search/available_dropdown_mappings.js index 1069aa77af62d4..a1782c549d62fb 100644 --- a/app/assets/javascripts/filtered_search/available_dropdown_mappings.js +++ b/app/assets/javascripts/filtered_search/available_dropdown_mappings.js @@ -207,22 +207,17 @@ export default class AvailableDropdownMappings { } getMergeRequestTargetBranchesEndpoint() { - return this.getMergeRequestBranchesEndpoint(false, true); + const targetBranchEndpointPath = '/-/autocomplete/merge_request_target_branches.json'; + return this.getMergeRequestBranchesEndpoint(targetBranchEndpointPath); } getMergeRequestSourceBranchesEndpoint() { - return this.getMergeRequestBranchesEndpoint(true, false); + const sourceBranchEndpointPath = '/-/autocomplete/merge_request_source_branches.json'; + return this.getMergeRequestBranchesEndpoint(sourceBranchEndpointPath); } - getMergeRequestBranchesEndpoint(sourcesBranch = false, targetBranch = false) { - let endpointPath = ''; - if (sourcesBranch) { - endpointPath = '/-/autocomplete/merge_request_source_branches.json'; - } else if (targetBranch) { - endpointPath = '/-/autocomplete/merge_request_target_branches.json'; - } + getMergeRequestBranchesEndpoint(endpointPath = '') { const endpoint = `${gon.relative_url_root || ''}${endpointPath}`; - const params = { group_id: this.getGroupId(), project_id: this.getProjectId(), -- GitLab