diff --git a/ee/app/views/shared/issuable/_approvals.html.haml b/ee/app/views/shared/issuable/_approvals.html.haml index f700162b276fabc27338007c8bab33e40a93e335..d9ff76128042b5dbd0c2520262a77f624fd814d9 100644 --- a/ee/app/views/shared/issuable/_approvals.html.haml +++ b/ee/app/views/shared/issuable/_approvals.html.haml @@ -11,7 +11,7 @@ = form.label :approver_ids, class: 'col-form-label col-sm-2' do Approvers .col-sm-10 - - if Feature.enabled?(:approval_rules, @project) + - if Feature.enabled?(:approval_rules, @target_project) = render 'shared/issuable/approvals_multiple_rule', issuable: issuable - else = render 'shared/issuable/approvals_single_rule', issuable: issuable, presenter: presenter, form: form diff --git a/ee/app/views/shared/issuable/_approvals_multiple_rule.html.haml b/ee/app/views/shared/issuable/_approvals_multiple_rule.html.haml index 0d74f93b3b02b487a0ae9a0b692fbbc53f0f81de..4e481d4f0a179ad7d12169510004b178367e2856 100644 --- a/ee/app/views/shared/issuable/_approvals_multiple_rule.html.haml +++ b/ee/app/views/shared/issuable/_approvals_multiple_rule.html.haml @@ -1,7 +1,7 @@ -#js-mr-approvals-input{ data: { 'project_id': @project.id, +#js-mr-approvals-input{ data: { 'project_id': @target_project.id, 'can_edit': can?(current_user, :update_approvers, issuable).to_s, - 'allow_multi_rule': @project.multiple_approval_rules_available?.to_s, + 'allow_multi_rule': @target_project.multiple_approval_rules_available?.to_s, 'mr_id': issuable.iid, - 'mr_settings_path': issuable.iid && api_v4_projects_merge_requests_approval_settings_path(id: @project.id, merge_request_iid: issuable.iid), - 'project_settings_path': api_v4_projects_approval_settings_path(id: @project.id) } } + 'mr_settings_path': issuable.iid && api_v4_projects_merge_requests_approval_settings_path(id: @target_project.id, merge_request_iid: issuable.iid), + 'project_settings_path': api_v4_projects_approval_settings_path(id: @target_project.id) } } = sprite_icon('spinner', size: 24, css_class: 'gl-spinner') diff --git a/ee/changelogs/unreleased/10601-merge-request-creation-from-fork-fails-due-to-approval-rule-sourcing.yml b/ee/changelogs/unreleased/10601-merge-request-creation-from-fork-fails-due-to-approval-rule-sourcing.yml new file mode 100644 index 0000000000000000000000000000000000000000..2b12c0bc37ed4693e26a6ff0a3491a3ae0fa9db6 --- /dev/null +++ b/ee/changelogs/unreleased/10601-merge-request-creation-from-fork-fails-due-to-approval-rule-sourcing.yml @@ -0,0 +1,5 @@ +--- +title: Fix approval rule sourcing from forked MR +merge_request: 10474 +author: +type: fixed diff --git a/ee/spec/features/merge_request/user_sets_approval_rules_spec.rb b/ee/spec/features/merge_request/user_sets_approval_rules_spec.rb index 9169a5ae41622e82d5f73797bb0b513ddbb6f656..5850cc1032ed0851a72cc1b30ce831ec3d30581a 100644 --- a/ee/spec/features/merge_request/user_sets_approval_rules_spec.rb +++ b/ee/spec/features/merge_request/user_sets_approval_rules_spec.rb @@ -3,10 +3,16 @@ require 'rails_helper' describe 'Merge request > User sets approval rules', :js do + include ProjectForksHelper + let(:approver) { create(:user) } let(:author) { create(:user) } let(:project) { create(:project, :public, :repository) } + def page_rule_names + page.all('.js-approval-rules table .js-name') + end + before do stub_licensed_features(multiple_approval_rules: true) @@ -16,12 +22,36 @@ end context "with project approval rules" do - let!(:regular_rule) { create(:approval_project_rule, project: project, users: [approver], name: 'Regular Rule') } + let!(:regular_rules) do + Array.new(3) do |i| + create(:approval_project_rule, project: project, users: [approver], name: "Regular Rule #{i}") + end + end + + context "from a fork" do + let(:forked_project) { fork_project(project, nil, repository: true) } + + before do + forked_project.add_maintainer(author) + allow(forked_project).to receive(:multiple_approval_rules_available?).and_return(false) + + sign_in(author) + visit project_new_merge_request_path(forked_project, merge_request: { target_branch: 'master', target_project_id: project.id, source_branch: 'feature' }) + wait_for_requests + end + + it "shows approval rules from target project" do + names = page_rule_names + regular_rules.each_with_index do |rule, idx| + expect(names[idx]).to have_text(rule.name) + end + end + end context "with a private group rule" do let!(:private_group) { create(:group, :private) } let!(:private_rule) { create(:approval_project_rule, project: project, groups: [private_group], name: 'Private Rule') } - let!(:rules) { [regular_rule, private_rule] } + let!(:rules) { regular_rules + [private_rule] } before do private_group.add_developer(approver) @@ -32,7 +62,7 @@ end it "shows approval rules" do - names = page.all('.js-approval-rules table .js-name') + names = page_rule_names rules.each.with_index do |rule, idx| expect(names[idx]).to have_text(rule.name) end diff --git a/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb b/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb index 0206928a2113d9052084f53d1735f552f44bfced..88c4b52b3a668b883a23acfb4dfdfb45dfa0856d 100644 --- a/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb @@ -12,6 +12,7 @@ assign(:hidden_commit_count, 0) assign(:total_commit_count, merge_request.commits.count) assign(:project, merge_request.target_project) + assign(:target_project, merge_request.target_project) assign(:mr_presenter, merge_request.present(current_user: merge_request.author)) allow(view).to receive(:can?).and_return(true) diff --git a/spec/views/projects/merge_requests/edit.html.haml_spec.rb b/spec/views/projects/merge_requests/edit.html.haml_spec.rb index c13eab30054d8a32787f210aaeae8b5bf00d015e..fd0cbe52ef75a5711dfe1a59d8a7d7ef90f601d7 100644 --- a/spec/views/projects/merge_requests/edit.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/edit.html.haml_spec.rb @@ -23,6 +23,7 @@ before do assign(:project, project) + assign(:target_project, project) assign(:merge_request, closed_merge_request) assign(:mr_presenter, closed_merge_request.present(current_user: user))