diff options
author | Michael Kozono <mkozono@gmail.com> | 2017-09-05 17:02:11 -0700 |
---|---|---|
committer | Michael Kozono <mkozono@gmail.com> | 2017-09-06 12:07:21 -0700 |
commit | f25b5b7f8db05ec441574429e024c71893fa7c11 (patch) | |
tree | 8270555cadd697963bc7585b582e0e9c9d2ac2cc | |
parent | edf8dd44f7676d14fda5ce305c2c3e31b8c14c17 (diff) | |
download | gitlab-ce-f25b5b7f8db05ec441574429e024c71893fa7c11.tar.gz |
Fix “Share lock” help text
-rw-r--r-- | app/helpers/groups_helper.rb | 41 | ||||
-rw-r--r-- | app/views/groups/edit.html.haml | 2 | ||||
-rw-r--r-- | spec/helpers/groups_helper_spec.rb | 83 | ||||
-rw-r--r-- | spec/views/groups/edit.html.haml_spec.rb | 34 |
4 files changed, 116 insertions, 44 deletions
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 62ad16314bf..f865cd133ad 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -69,12 +69,18 @@ module GroupsHelper { group_name: group.name } end - def share_with_group_lock_help_text - return default_help unless @group.has_parent? - return default_help unless @group.parent.share_with_group_lock? - return parent_locked_and_has_been_overridden unless @group.share_with_group_lock? - return parent_locked_but_you_can_override if @group.has_owner?(current_user) - return parent_locked_so_ask_the_owner + def share_with_group_lock_help_text(group) + return default_help unless group.parent&.share_with_group_lock? + + if group.share_with_group_lock? + if can?(current_user, :change_share_with_group_lock, group.parent) + ancestor_locked_but_you_can_override(group) + else + ancestor_locked_so_ask_the_owner(group) + end + else + ancestor_locked_and_has_been_overridden(group) + end end private @@ -93,23 +99,30 @@ module GroupsHelper end end - def parent_group_link - link_to @group.parent.name, group_path(@group.parent) + def ancestor_group_link(group) + ancestor = oldest_consecutively_locked_ancestor(group) + link_to ancestor.name, group_path(ancestor) + end + + def oldest_consecutively_locked_ancestor(group) + group.ancestors.find do |group| + !group.has_parent? || !group.parent.share_with_group_lock? + end end def default_help s_("GroupSettings|This setting will be applied to all subgroups unless overridden by a group owner.") end - def parent_locked_but_you_can_override - s_("GroupSettings|This setting is applied on %{parent_group}. You can override the setting or remove the share lock from the parent group.") % { parent_group: parent_group_link } + def ancestor_locked_but_you_can_override(group) + s_("GroupSettings|This setting is applied on %{ancestor_group}. You can override the setting or remove the share lock from %{ancestor_group}.") % { ancestor_group: ancestor_group_link(group) } end - def parent_locked_so_ask_the_owner - s_("GroupSettings|This setting is applied on %{parent_group}. To share this group with another group, ask the owner to override the setting or remove the share lock from the parent group.") % { parent_group: parent_group_link } + def ancestor_locked_so_ask_the_owner(group) + s_("GroupSettings|This setting is applied on %{ancestor_group}. To share projects in this group with another group, ask the owner to override the setting or remove the share lock from %{ancestor_group}.") % { ancestor_group: ancestor_group_link(group) } end - def parent_locked_and_has_been_overridden - s_("GroupSettings|This setting is applied on %{parent_group} and has been overridden on this subgroup.") % { parent_group: parent_group_link } + def ancestor_locked_and_has_been_overridden(group) + s_("GroupSettings|This setting is applied on %{ancestor_group} and has been overridden on this subgroup.") % { ancestor_group: ancestor_group_link(group) } end end diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index bd8c4b5954a..31961691bdf 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -39,7 +39,7 @@ - group_link = link_to @group.name, group_path(@group) = s_("GroupSettings|Prevent sharing a project within %{group} with other groups").html_safe % { group: group_link } %br - %span.descr= share_with_group_lock_help_text + %span.descr= share_with_group_lock_help_text(@group) = render 'group_admin_settings', f: f diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 05f969904f5..baa4584f61d 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -95,4 +95,87 @@ describe GroupsHelper do .to match(/<li style="text-indent: 16px;"><a.*>#{deep_nested_group.name}.*<\/li>.*<a.*>#{very_deep_nested_group.name}<\/a>/m) end end + + # rubocop:disable Layout/SpaceBeforeComma + describe '#share_with_group_lock_help_text', :nested_groups do + let!(:root_group) { create(:group) } + let!(:subgroup) { create(:group, parent: root_group) } + let!(:sub_subgroup) { create(:group, parent: subgroup) } + let(:root_owner) { create(:user) } + let(:sub_owner) { create(:user) } + let(:sub_sub_owner) { create(:user) } + let(:possible_help_texts) do + { + default_help: "This setting will be applied to all subgroups unless overridden by a group owner", + ancestor_locked_but_you_can_override: /This setting is applied on [\w\s<>="\/]+\. You can override the setting or remove the share lock from [\w\s<>="\/]+/, + ancestor_locked_so_ask_the_owner: /This setting is applied on [\w\s<>="\/]+\. To share projects in this group with another group, ask the owner to override the setting or remove the share lock from [\w\s<>="\/]+/, + ancestor_locked_and_has_been_overridden: /This setting is applied on [\w\s<>="\/]+ and has been overridden on this subgroup/ + } + end + let(:possible_linked_ancestors) do + { + root_group: root_group, + subgroup: subgroup + } + end + let(:users) do + { + root_owner: root_owner, + sub_owner: sub_owner, + sub_sub_owner: sub_sub_owner + } + end + subject { helper.share_with_group_lock_help_text(sub_subgroup) } + + where(:root_share_locked, :subgroup_share_locked, :sub_subgroup_share_locked, :current_user, :help_text, :linked_ancestor) do + [ + [false , false , false , :root_owner , :default_help , nil], + [false , false , false , :sub_owner , :default_help , nil], + [false , false , false , :sub_sub_owner , :default_help , nil], + [false , false , true , :root_owner , :default_help , nil], + [false , false , true , :sub_owner , :default_help , nil], + [false , false , true , :sub_sub_owner , :default_help , nil], + [false , true , false , :root_owner , :ancestor_locked_and_has_been_overridden , :subgroup], + [false , true , false , :sub_owner , :ancestor_locked_and_has_been_overridden , :subgroup], + [false , true , false , :sub_sub_owner , :ancestor_locked_and_has_been_overridden , :subgroup], + [false , true , true , :root_owner , :ancestor_locked_but_you_can_override , :subgroup], + [false , true , true , :sub_owner , :ancestor_locked_but_you_can_override , :subgroup], + [false , true , true , :sub_sub_owner , :ancestor_locked_so_ask_the_owner , :subgroup], + [true , false , false , :root_owner , :default_help , nil], + [true , false , false , :sub_owner , :default_help , nil], + [true , false , false , :sub_sub_owner , :default_help , nil], + [true , false , true , :root_owner , :default_help , nil], + [true , false , true , :sub_owner , :default_help , nil], + [true , false , true , :sub_sub_owner , :default_help , nil], + [true , true , false , :root_owner , :ancestor_locked_and_has_been_overridden , :root_group], + [true , true , false , :sub_owner , :ancestor_locked_and_has_been_overridden , :root_group], + [true , true , false , :sub_sub_owner , :ancestor_locked_and_has_been_overridden , :root_group], + [true , true , true , :root_owner , :ancestor_locked_but_you_can_override , :root_group], + [true , true , true , :sub_owner , :ancestor_locked_so_ask_the_owner , :root_group], + [true , true , true , :sub_sub_owner , :ancestor_locked_so_ask_the_owner , :root_group] + ] + end + + with_them do + before do + root_group.add_owner(root_owner) + subgroup.add_owner(sub_owner) + sub_subgroup.add_owner(sub_sub_owner) + + root_group.update_column(:share_with_group_lock, true) if root_share_locked + subgroup.update_column(:share_with_group_lock, true) if subgroup_share_locked + sub_subgroup.update_column(:share_with_group_lock, true) if sub_subgroup_share_locked + + allow(helper).to receive(:current_user).and_return(users[current_user]) + allow(helper).to receive(:can?) + .with(users[current_user], :change_share_with_group_lock, subgroup) + .and_return(Ability.allowed?(users[current_user], :change_share_with_group_lock, subgroup)) + end + + it 'has the correct help text with correct ancestor links' do + expect(subject).to match(possible_help_texts[help_text]) + expect(subject).to match(possible_linked_ancestors[linked_ancestor].name) unless help_text == :default_help + end + end + end end diff --git a/spec/views/groups/edit.html.haml_spec.rb b/spec/views/groups/edit.html.haml_spec.rb index 613368049e1..86bb28a43c1 100644 --- a/spec/views/groups/edit.html.haml_spec.rb +++ b/spec/views/groups/edit.html.haml_spec.rb @@ -6,8 +6,6 @@ describe 'groups/edit.html.haml' do describe 'Share lock option' do let(:root_owner) { create(:user) } let(:root_group) { create(:group) } - let(:expected_label) { default_label } - let(:expected_help) { default_help } before do root_group.add_owner(root_owner) @@ -16,14 +14,16 @@ describe 'groups/edit.html.haml' do shared_examples_for 'share lock option' do |checkbox_options| it 'should have the correct label, help text, and checkbox options' do assign(:group, test_group) - allow(view).to receive(:can?).and_return(true) + allow(view).to receive(:can?).with(test_user, :admin_group, test_group).and_return(true) allow(view).to receive(:can_change_group_visibility_level?).and_return(false) allow(view).to receive(:current_user).and_return(test_user) + expect(view).to receive(:can_change_share_with_group_lock?).and_return(!checkbox_options[:disabled]) + expect(view).to receive(:share_with_group_lock_help_text).and_return('help text here') render - expect(rendered).to have_content(expected_label) - expect(rendered).to have_css('.descr', text: expected_help) + expect(rendered).to have_content("Prevent sharing a project within #{test_group.name} with other groups") + expect(rendered).to have_css('.descr', text: 'help text here') expect(rendered).to have_field('group_share_with_group_lock', checkbox_options) end end @@ -82,14 +82,12 @@ describe 'groups/edit.html.haml' do context 'when the subgroup has "Share lock" disabled (parent overridden)' do context 'as the root_owner' do let(:test_user) { root_owner } - let(:expected_help) { parent_locked_and_has_been_overridden } it_behaves_like 'share lock option', { disabled: false, checked: false } end context 'as the sub_owner' do let(:test_user) { sub_owner } - let(:expected_help) { parent_locked_and_has_been_overridden } it_behaves_like 'share lock option', { disabled: false, checked: false } end @@ -102,39 +100,17 @@ describe 'groups/edit.html.haml' do context 'as the root_owner' do let(:test_user) { root_owner } - let(:expected_help) { parent_locked_but_you_can_override } it_behaves_like 'share lock option', { disabled: false, checked: true } end context 'as the sub_owner' do let(:test_user) { sub_owner } - let(:expected_help) { parent_locked_so_ask_the_owner } it_behaves_like 'share lock option', { disabled: true, checked: true } end end end end - - def default_label - "Prevent sharing a project within #{test_group.name} with other groups" - end - - def default_help - "This setting will be applied to all subgroups unless overridden by a group owner." - end - - def parent_locked_but_you_can_override - "This setting is applied on #{test_group.parent.name}. You can override the setting or remove the share lock from the parent group." - end - - def parent_locked_so_ask_the_owner - "This setting is applied on #{test_group.parent.name}. To share this group with another group, ask the owner to override the setting or remove the share lock from the parent group." - end - - def parent_locked_and_has_been_overridden - "This setting is applied on #{test_group.parent.name} and has been overridden on this subgroup." - end end end |