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 4d3647cdf5cdd8ed6327c348ef7ae52b7c10d23b..74d91734630e7d9fca21603c3c009f049414312f 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/assets/javascripts/filtered_search/available_dropdown_mappings.js b/app/assets/javascripts/filtered_search/available_dropdown_mappings.js index 892e9130fe8d4e972a4bf1b97a32fad50be30991..a1782c549d62fb15313d64f0d886c0bf77119543 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,10 +207,17 @@ export default class AvailableDropdownMappings { } getMergeRequestTargetBranchesEndpoint() { - const endpoint = `${ - gon.relative_url_root || '' - }/-/autocomplete/merge_request_target_branches.json`; + const targetBranchEndpointPath = '/-/autocomplete/merge_request_target_branches.json'; + return this.getMergeRequestBranchesEndpoint(targetBranchEndpointPath); + } + + getMergeRequestSourceBranchesEndpoint() { + const sourceBranchEndpointPath = '/-/autocomplete/merge_request_source_branches.json'; + return this.getMergeRequestBranchesEndpoint(sourceBranchEndpointPath); + } + getMergeRequestBranchesEndpoint(endpointPath = '') { + const endpoint = `${gon.relative_url_root || ''}${endpointPath}`; const params = { group_id: this.getGroupId(), project_id: this.getProjectId(), diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index c9cb1ca14e2813dc7e57f7633138d78dcb56a908..1c2bd10bc8143dd979ed9a997aef4b429f405d7c 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/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index f7ee90ab8705c0a839f42dd970d74e12b99f8dcc..b7de1c08f8628486ad7852a181ac8065900a9921 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 ] @@ -81,7 +82,8 @@ 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 private @@ -132,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) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a751ed0c94d634e5daf85c463f31a47b2384cdd6..524a9b8074b100ed3d81bf8e8e7926ac3a7bfbfa 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 86aaa5128a8f52c2a8a4ef1ee56790ac75aae500..52c8a4d41237d02ef04fed9f69c8af6df163f069 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 2c0ae3d0763b29d758435997a03fe4c9abf8ef0d..925ef0c62e8515bbee8a271156092e0dddf581fc 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/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 d2e5e3fcaa82ad383948b580d6b8bef7b1865ff2..615ae6e448b4e14d08d99cb610bc4cd2290e9dd5 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]); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2185dc9b9410df5586d937bef6106b5ea7887ac5..cac0c232a907b3e053efa2b1956c14304a5dba5b 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 "" diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index a66cb4364d7c50587ef1946d04bb6b9a2c99d853..cbe0319a78d427d145eaf68d6f9699db716c542a 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -454,77 +454,85 @@ 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') } + 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 - context 'anonymous user' do - it 'returns empty json' do - get :merge_request_target_branches, params: { project_id: project.id } + 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 + 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 :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 + 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: ' ' } + ] + ) + + with_them do + 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' }) + 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 :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(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 :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(response).to have_gitlab_http_status(:ok) + expect(json_response).to contain_exactly(expected_result) + end end end + + it_behaves_like 'Get merge_request_{}_branches', :merge_request_target_branches, { 'title' => 'test_target_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_source_branch_spec.rb b/spec/features/merge_requests/user_filters_by_source_branch_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7eade94de2ad5eb65f09ede16a4a3b971486cf33 --- /dev/null +++ b/spec/features/merge_requests/user_filters_by_source_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 + + 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) { 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) + 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, merged: 1, closed: 1, all: 3) + 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, merged: 0, 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, merged: 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, merged: 0, 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 04d0226ac22d5a265b1f21fc625e84b45e2531ba..d3c32da284232f6a430771a10926865db986b878 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -725,22 +725,35 @@ 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, 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(: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', '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 target branches sort by updated at desc' do - expect(described_class.recent_target_branches).to match_array(%w[feature merge-test fix]) + it 'returns branches sort by updated at desc' do + 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