Commit 056d1c6f authored by Simon Knox's avatar Simon Knox

add can_override_approvers to project, add view spec

remove spec for help text that was removed
parent 23c22db6
...@@ -1211,6 +1211,12 @@ class Project < ActiveRecord::Base ...@@ -1211,6 +1211,12 @@ class Project < ActiveRecord::Base
end end
end end
def can_override_approvers?
!disable_overriding_approvers_per_merge_request
rescue NameError
true
end
def allowed_to_share_with_group? def allowed_to_share_with_group?
!namespace.share_with_group_lock !namespace.share_with_group_lock
end end
......
- form = local_assigns.fetch(:form) - form = local_assigns.fetch(:form)
- project = local_assigns.fetch(:project) - project = local_assigns.fetch(:project)
- can_override_approvals = !project.try(:disable_overriding_approvers_per_merge_request) - can_override_approvers = project.can_override_approvers?
.form-group .form-group
= label_tag :merge_method_merge, class: 'label-light' do = label_tag :merge_method_merge, class: 'label-light' do
...@@ -104,12 +104,12 @@ ...@@ -104,12 +104,12 @@
Approvals required Approvals required
= form.number_field :approvals_before_merge, class: "form-control", min: 0 = form.number_field :approvals_before_merge, class: "form-control", min: 0
.help-block .help-block
Set number of approvers required before any open merge request can be merged Set number of approvers required before open merge requests can be merged
.form-group.mr-overwrite-approvals .form-group.mr-overwrite-approvals
.checkbox .checkbox
= form.label :disable_overriding_approvers_per_merge_request do = form.label :disable_overriding_approvers_per_merge_request do
= form.check_box(:disable_overriding_approvers_per_merge_request, { checked: can_override_approvals }, false, true) = form.check_box(:disable_overriding_approvers_per_merge_request, { checked: can_override_approvers }, false, true)
%strong Can overwrite approvers and approvals required per merge request %strong Can overwrite approvers and approvals required per merge request
.form-group.reset-approvals-on-push .form-group.reset-approvals-on-push
......
...@@ -4,13 +4,13 @@ ...@@ -4,13 +4,13 @@
- return unless issuable.is_a?(MergeRequest) - return unless issuable.is_a?(MergeRequest)
- return unless issuable.requires_approve? - return unless issuable.requires_approve?
- overridable = !issuable.target_project.try(:disable_overriding_approvers_per_merge_request) - can_override_approvers = issuable.target_project.can_override_approvers?
.form-group .form-group
= form.label :approver_ids, class: 'control-label' do = form.label :approver_ids, class: 'control-label' do
Approvers Approvers
.col-sm-10 .col-sm-10
- if overridable - if can_override_approvers
- author = issuable.author || current_user - author = issuable.author || current_user
- skip_users = issuable.all_approvers_including_groups + [author] - skip_users = issuable.all_approvers_including_groups + [author]
= users_select_tag("merge_request[approver_ids]", multiple: true, class: 'input-large', email_user: true, skip_users: skip_users) = users_select_tag("merge_request[approver_ids]", multiple: true, class: 'input-large', email_user: true, skip_users: skip_users)
...@@ -36,7 +36,7 @@ ...@@ -36,7 +36,7 @@
- issuable.overall_approvers.each do |approver| - issuable.overall_approvers.each do |approver|
%li{ id: dom_id(approver.user), class: item_classes + ['approver'] } %li{ id: dom_id(approver.user), class: item_classes + ['approver'] }
= link_to approver.user.name, approver.user = link_to approver.user.name, approver.user
- if overridable - if can_override_approvers
.pull-right .pull-right
- if unsaved_approvers - if unsaved_approvers
= link_to "#", data: { confirm: "Are you sure you want to remove approver #{approver.user.name}"}, class: "btn-xs btn btn-remove", title: 'Remove approver' do = link_to "#", data: { confirm: "Are you sure you want to remove approver #{approver.user.name}"}, class: "btn-xs btn btn-remove", title: 'Remove approver' do
...@@ -50,7 +50,7 @@ ...@@ -50,7 +50,7 @@
%li{ id: dom_id(approver_group.group), class: item_classes + ['approver-group'] } %li{ id: dom_id(approver_group.group), class: item_classes + ['approver-group'] }
Group: Group:
= link_to approver_group.group.name, approver_group.group = link_to approver_group.group.name, approver_group.group
- if overridable - if can_override_approvers
.pull-right .pull-right
- if unsaved_approvers - if unsaved_approvers
= link_to "#", data: { confirm: "Are you sure you want to remove group #{approver_group.group.name}"}, class: "btn-xs btn btn-remove", title: 'Remove group' do = link_to "#", data: { confirm: "Are you sure you want to remove group #{approver_group.group.name}"}, class: "btn-xs btn btn-remove", title: 'Remove group' do
...@@ -65,9 +65,9 @@ ...@@ -65,9 +65,9 @@
.form-group .form-group
= form.label :approvals_before_merge, class: 'label-light' do = form.label :approvals_before_merge, class: 'label-light' do
Approvals required Approvals required
= form.number_field :approvals_before_merge, class: 'form-control', value: issuable.approvals_required, readonly: !overridable = form.number_field :approvals_before_merge, class: 'form-control', value: issuable.approvals_required, readonly: !can_override_approvers
- if overridable - if can_override_approvers
.help-block.suggested-approvers .help-block.suggested-approvers
- if @suggested_approvers.any? - if @suggested_approvers.any?
Suggested approvers: Suggested approvers:
......
...@@ -190,7 +190,6 @@ feature 'Merge request approvals', js: true, feature: true do ...@@ -190,7 +190,6 @@ feature 'Merge request approvals', js: true, feature: true do
# new MR setting on the edit MR page # new MR setting on the edit MR page
expect(find('#merge_request_approvals_before_merge').value).to eq('3') expect(find('#merge_request_approvals_before_merge').value).to eq('3')
expect(find('#merge_request_approvals_before_merge ~ .help-block')).to have_content('1 user')
end end
end end
end end
......
require 'spec_helper'
describe 'shared/issuable/_approvals.html.haml' do
let(:project) { build(:empty_project) }
let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
let(:form) { double('form') }
before do
allow(form).to receive(:label)
allow(form).to receive(:number_field)
allow(merge_request).to receive(:requires_approve?).and_return(true)
assign(:project, project)
assign(:suggested_approvers, [])
end
context 'has no approvers' do
it 'shows empty approvers list' do
render 'shared/issuable/approvals', form: form, issuable: merge_request
expect(rendered).to have_text('There are no approvers')
end
context 'can override approvers' do
before do
allow(project).to receive(:can_override_approvers?).and_return(true)
render 'shared/issuable/approvals', form: form, issuable: merge_request
end
it 'shows suggested approvers' do
expect(rendered).to have_css('.suggested-approvers')
end
it 'shows select approvers field' do
expect(rendered).to have_css('#merge_request_approver_ids')
end
it 'shows select approver groups field' do
expect(rendered).to have_css('#merge_request_approver_group_ids')
end
end
context 'can not override approvers' do
before do
allow(project).to receive(:can_override_approvers?).and_return(false)
render 'shared/issuable/approvals', form: form, issuable: merge_request
end
it 'hides suggested approvers' do
expect(rendered).not_to have_css('.suggested-approvers')
end
it 'hides select approvers field' do
expect(rendered).not_to have_css('#merge_request_approver_ids')
end
it 'hides select approver groups field' do
expect(rendered).not_to have_css('#merge_request_approver_group_ids')
end
end
end
context 'has approvers' do
let(:user) { create(:user) }
let(:approver) { create(:approver, user: user, target: merge_request) }
let(:approver_group) { create(:approver_group, target: merge_request) }
before do
assign(:approver, approver)
assign(:approver_group, approver_group)
end
it 'shows approver in table' do
render 'shared/issuable/approvals', form: form, issuable: merge_request, project: project
expect(rendered).to have_text(approver[:name])
expect(rendered).to have_text(approver_group[:name])
end
context 'can override approvers' do
it 'shows remove button for approver' do
render 'shared/issuable/approvals', form: form, issuable: merge_request
expect(rendered).to have_css('.btn-remove')
end
end
context 'can not override approvers' do
it 'hides remove button' do
allow(project).to receive(:can_override_approvers?).and_return(false)
render 'shared/issuable/approvals', form: form, issuable: merge_request
expect(rendered).not_to have_css('.btn-remove')
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment