From 56f25c3e615752817446d5aedb7d66d25d815641 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Mon, 7 Dec 2015 13:37:01 +0100 Subject: [PATCH] Improve text indication visibility on snippets --- CHANGELOG | 1 + app/helpers/visibility_level_helper.rb | 26 +++--- .../application_settings/_form.html.haml | 4 +- app/views/shared/snippets/_header.html.haml | 2 +- spec/helpers/visibility_level_helper_spec.rb | 89 ++++++++----------- 5 files changed, 55 insertions(+), 67 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 67f70e676c2..c291d4b6dec 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -24,6 +24,7 @@ v 8.3.0 (unreleased) - Fix bug when simultaneously accepting multiple MRs results in MRs that are of "merged" status, but not merged to the target branch - Add languages page to graphs - Block LDAP user when they are no longer found in the LDAP server + - Improve wording on project visibility levels (Zeger-Jan van de Weg) v 8.2.3 - Fix application settings cache not expiring after changes (Stan Hu) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 72c65030f94..2e69ce923a2 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -12,22 +12,22 @@ module VisibilityLevelHelper # Return the description for the +level+ argument. # - # +level+ One of the Gitlab::VisibilityLevel constants - # +form_model+ Either a model object (Project, Snippet, etc.) or the name of - # a Project or Snippet class. + # +level+ One of the Gitlab::VisibilityLevel constants + # +form_model+ Either a model object (Project, Snippet, etc.) or the name of + # a Project or Snippet class. def visibility_level_description(level, form_model) - case form_model.is_a?(String) ? form_model : form_model.class.name - when 'PersonalSnippet', 'ProjectSnippet', 'Snippet' - snippet_visibility_level_description(level) - when 'Project' + case form_model + when Project project_visibility_level_description(level) + when Snippet + snippet_visibility_level_description(level, form_model) end end def project_visibility_level_description(level) case level when Gitlab::VisibilityLevel::PRIVATE - "Project access must be granted explicitly for each user." + "Project access must be granted explicitly to each user." when Gitlab::VisibilityLevel::INTERNAL "The project can be cloned by any logged in user." when Gitlab::VisibilityLevel::PUBLIC @@ -35,12 +35,16 @@ module VisibilityLevelHelper end end - def snippet_visibility_level_description(level) + def snippet_visibility_level_description(level, snippet = nil) case level when Gitlab::VisibilityLevel::PRIVATE - "The snippet is visible only for me." + if snippet.is_a? ProjectSnippet + "The snippet is visible only to project members." + else + "The snippet is visible only to me." + end when Gitlab::VisibilityLevel::INTERNAL - "The snippet is visible for any logged in user." + "The snippet is visible to any logged in user." when Gitlab::VisibilityLevel::PUBLIC "The snippet can be accessed without any authentication." end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index ddaf0e0e8ff..6c355366948 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -14,11 +14,11 @@ .form-group.project-visibility-level-holder = f.label :default_project_visibility, class: 'control-label col-sm-2' .col-sm-10 - = render('shared/visibility_radios', model_method: :default_project_visibility, form: f, selected_level: @application_setting.default_project_visibility, form_model: 'Project') + = render('shared/visibility_radios', model_method: :default_project_visibility, form: f, selected_level: @application_setting.default_project_visibility, form_model: Project) .form-group.project-visibility-level-holder = f.label :default_snippet_visibility, class: 'control-label col-sm-2' .col-sm-10 - = render('shared/visibility_radios', model_method: :default_snippet_visibility, form: f, selected_level: @application_setting.default_snippet_visibility, form_model: 'Snippet') + = render('shared/visibility_radios', model_method: :default_snippet_visibility, form: f, selected_level: @application_setting.default_snippet_visibility, form_model: PersonalSnippet) .form-group = f.label :restricted_visibility_levels, class: 'control-label col-sm-2' .col-sm-10 diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index 89c1d7122b0..eb0fd21c2d4 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -1,6 +1,6 @@ .issuable-details .page-title - .snippet-box.has_tooltip{class: visibility_level_color(@snippet.visibility_level), title: snippet_visibility_level_description(@snippet.visibility_level), data: { container: 'body' }} + .snippet-box.has_tooltip{class: visibility_level_color(@snippet.visibility_level), title: snippet_visibility_level_description(@snippet.visibility_level, @snippet), data: { container: 'body' }} = visibility_level_icon(@snippet.visibility_level, fw: false) = visibility_level_label(@snippet.visibility_level) Snippet ##{@snippet.id} diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index c4f7693329c..aafc24397a9 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -7,69 +7,52 @@ describe VisibilityLevelHelper do init_haml_helpers end - let(:project) { create(:project) } + let(:project) { build(:project) } + let(:personal_snippet) { build(:personal_snippet) } + let(:project_snippet) { build(:project_snippet) } describe 'visibility_level_description' do - shared_examples 'a visibility level description' do - let(:desc) do - visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, - form_model) - end - - let(:expected_class) do - class_name = case form_model.class.name - when 'String' - form_model - else - form_model.class.name - end - - class_name.match(/(project|snippet)$/i)[0] - end - - it 'should refer to the correct class' do - expect(desc).to match(/#{expected_class}/i) + context 'used with a Project' do + it 'delegates projects to #project_visibility_level_description' do + expect(visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, project)) + .to match /project/i end end - context 'form_model argument is a String' do - context 'model object is a personal snippet' do - it_behaves_like 'a visibility level description' do - let(:form_model) { 'PersonalSnippet' } - end - end - - context 'model object is a project snippet' do - it_behaves_like 'a visibility level description' do - let(:form_model) { 'ProjectSnippet' } - end - end - - context 'model object is a project' do - it_behaves_like 'a visibility level description' do - let(:form_model) { 'Project' } - end + context 'called with a Snippet' do + it 'delegates snippets to #snippet_visibility_level_description' do + expect(visibility_level_description(Gitlab::VisibilityLevel::INTERNAL, project_snippet)) + .to match /snippet/i end end + end - context 'form_model argument is a model object' do - context 'model object is a personal snippet' do - it_behaves_like 'a visibility level description' do - let(:form_model) { create(:personal_snippet) } - end - end + describe "#project_visibility_level_description" do + it "describes private projects" do + expect(project_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE)) + .to eq "Project access must be granted explicitly to each user." + end - context 'model object is a project snippet' do - it_behaves_like 'a visibility level description' do - let(:form_model) { create(:project_snippet, project: project) } - end - end + it "describes public projects" do + expect(project_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC)) + .to eq "The project can be cloned without any authentication." + end + end - context 'model object is a project' do - it_behaves_like 'a visibility level description' do - let(:form_model) { project } - end - end + describe "#snippet_visibility_level_description" do + it 'describes visibility only for me' do + expect(snippet_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, personal_snippet)) + .to eq "The snippet is visible only to me." + end + + it 'describes visibility for project members' do + expect(snippet_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, project_snippet)) + .to eq "The snippet is visible only to project members." + end + + it 'defaults to personal snippet' do + expect(snippet_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE)) + .to eq "The snippet is visible only to me." end end