From 2c9bb135057f4fea43aa0be5b94354f288d5070f Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Tue, 22 Nov 2016 17:16:30 +0000 Subject: [PATCH] Fixed tests Grab permissions description from backend Review changes Added unit tests --- app/assets/javascripts/dispatcher.js.es6 | 1 - app/assets/javascripts/project_new.js | 10 ++ .../javascripts/visibility_select.js.es6 | 32 +++--- app/assets/stylesheets/pages/projects.scss | 2 + app/helpers/projects_helper.rb | 11 ++ .../projects/_visibility_select.html.haml | 7 ++ app/views/projects/edit.html.haml | 4 +- app/views/shared/_visibility_select.html.haml | 11 -- features/steps/shared/project.rb | 2 +- .../settings/visibility_settings_spec.rb | 4 +- .../javascripts/visibility_select_spec.js.es6 | 100 ++++++++++++++++++ 11 files changed, 151 insertions(+), 33 deletions(-) create mode 100644 app/views/projects/_visibility_select.html.haml delete mode 100644 app/views/shared/_visibility_select.html.haml create mode 100644 spec/javascripts/visibility_select_spec.js.es6 diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 237a7dab5e9..413117c2226 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -262,7 +262,6 @@ case 'edit': shortcut_handler = new ShortcutsNavigation(); new ProjectNew(); - new gl.VisibilitySelect(); break; case 'new': new ProjectNew(); diff --git a/app/assets/javascripts/project_new.js b/app/assets/javascripts/project_new.js index 7fc611d0dad..86d57d97eae 100644 --- a/app/assets/javascripts/project_new.js +++ b/app/assets/javascripts/project_new.js @@ -14,11 +14,21 @@ return $('.save-project-loader').show(); }; })(this)); + + this.initVisibilitySelect(); + this.toggleSettings(); this.toggleSettingsOnclick(); this.toggleRepoVisibility(); } + ProjectNew.prototype.initVisibilitySelect = function() { + const visibilityContainer = document.querySelector('.js-visibility-select'); + if (!visibilityContainer) return; + const visibilitySelect = new gl.VisibilitySelect(visibilityContainer); + visibilitySelect.init(); + }; + ProjectNew.prototype.toggleSettings = function() { var self = this; diff --git a/app/assets/javascripts/visibility_select.js.es6 b/app/assets/javascripts/visibility_select.js.es6 index e846e7ead77..f712d7ba930 100644 --- a/app/assets/javascripts/visibility_select.js.es6 +++ b/app/assets/javascripts/visibility_select.js.es6 @@ -1,29 +1,27 @@ (() => { - const global = window.gl || (window.gl = {}); - - const VISIBILITY_DESCRIPTIONS = { - 0: 'Project access must be granted explicitly to each user.', - 10: 'This project can be cloned by any logged in user.', - 20: 'The project can be cloned without any authentication.', - }; + const gl = window.gl || (window.gl = {}); class VisibilitySelect { - constructor() { - this.visibilitySelect = document.querySelector('.js-visibility-select'); - this.helpBlock = this.visibilitySelect.querySelector('.help-block'); - this.select = this.visibilitySelect.querySelector('select'); + constructor(container) { + if (!container) throw new Error('VisibilitySelect requires a container element as argument 1'); + this.container = container; + this.helpBlock = this.container.querySelector('.help-block'); + this.select = this.container.querySelector('select'); + } + + init() { if (this.select) { - this.visibilityChanged(); - this.select.addEventListener('change', this.visibilityChanged.bind(this)); + this.updateHelpText(); + this.select.addEventListener('change', this.updateHelpText.bind(this)); } else { - this.helpBlock.textContent = this.visibilitySelect.querySelector('.js-locked').dataset.helpBlock; + this.helpBlock.textContent = this.container.querySelector('.js-locked').dataset.helpBlock; } } - visibilityChanged() { - this.helpBlock.innerText = VISIBILITY_DESCRIPTIONS[this.select.value]; + updateHelpText() { + this.helpBlock.textContent = this.select.querySelector('option:checked').dataset.description; } } - global.VisibilitySelect = VisibilitySelect; + gl.VisibilitySelect = VisibilitySelect; })(); diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 74b058ac94a..726e5c115b8 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -865,6 +865,8 @@ pre.light-well { } .project-feature { + padding-top: 10px; + @media (min-width: $screen-sm-min) { padding-left: 45px; } diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 9cda3b78761..e5438f4775b 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -445,4 +445,15 @@ module ProjectsHelper def project_issues(project) IssuesFinder.new(current_user, project_id: project.id).execute end + + def visibility_select_options(project, selected_level) + levels_options_array = Gitlab::VisibilityLevel.values.map do |level| + [ + visibility_level_label(level), + { data: { description: visibility_level_description(level, project) } }, + level + ] + end + options_for_select(levels_options_array, selected_level) + end end diff --git a/app/views/projects/_visibility_select.html.haml b/app/views/projects/_visibility_select.html.haml new file mode 100644 index 00000000000..65fc0a36ca9 --- /dev/null +++ b/app/views/projects/_visibility_select.html.haml @@ -0,0 +1,7 @@ +- if can_change_visibility_level?(@project, current_user) + = form.select(model_method, visibility_select_options(@project, selected_level), {}, class: 'form-control visibility-select') +- else + .info.js-locked{ data: { help_block: visibility_level_description(@project.visibility_level, @project) } } + = visibility_level_icon(@project.visibility_level) + %strong + = visibility_level_label(@project.visibility_level) diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index ff0da46a31d..1d7fdf68cb3 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -37,13 +37,13 @@ = link_to "(?)", help_page_path("public_access/public_access") %span.help-block .col-md-3.visibility-select-container - = render('shared/visibility_select', model_method: :visibility_level, form: f, selected_level: @project.visibility_level) + = render('projects/visibility_select', model_method: :visibility_level, form: f, selected_level: @project.visibility_level) = f.fields_for :project_feature do |feature_fields| %fieldset.features .row .col-md-9.project-feature = feature_fields.label :repository_access_level, "Repository", class: 'label-light' - %span.help-block Push files to be stored in this project + %span.help-block View and edit files in this project .col-md-3.js-repo-access-level = project_feature_access_select(:repository_access_level) diff --git a/app/views/shared/_visibility_select.html.haml b/app/views/shared/_visibility_select.html.haml deleted file mode 100644 index 42210f565f6..00000000000 --- a/app/views/shared/_visibility_select.html.haml +++ /dev/null @@ -1,11 +0,0 @@ -- if can_change_visibility_level?(@project, current_user) - - levels_options_hash = {} - - Gitlab::VisibilityLevel.values.each do |level| - - levels_options_hash[visibility_level_label(level)] = level - - options = options_for_select(levels_options_hash, selected_level) - = form.select(model_method, options, {}, class: 'form-control visibility-select') -- else - .info.js-locked{ data: { help_block: visibility_level_description(@project.visibility_level, @project)}} - = visibility_level_icon(@project.visibility_level) - %strong - = visibility_level_label(@project.visibility_level) diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index b51152c79c6..5e2c5c6232e 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -97,7 +97,7 @@ module SharedProject step 'I should see project settings' do expect(current_path).to eq edit_namespace_project_path(@project.namespace, @project) expect(page).to have_content("Project name") - expect(page).to have_content("Feature Visibility") + expect(page).to have_content("Sharing & Permissions") end def current_project diff --git a/spec/features/projects/settings/visibility_settings_spec.rb b/spec/features/projects/settings/visibility_settings_spec.rb index 86cc3fb886e..cef315ac9cd 100644 --- a/spec/features/projects/settings/visibility_settings_spec.rb +++ b/spec/features/projects/settings/visibility_settings_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'byebug' feature 'Visibility settings', feature: true, js: true do let(:user) { create(:user) } @@ -13,6 +12,7 @@ feature 'Visibility settings', feature: true, js: true do scenario 'project visibility select is available' do visibility_select_container = find('.js-visibility-select') + expect(visibility_select_container.find('.visibility-select').value).to eq project.visibility_level.to_s expect(visibility_select_container).to have_content 'The project can be cloned without any authentication.' end @@ -21,6 +21,7 @@ feature 'Visibility settings', feature: true, js: true do visibility_select_container = find('.js-visibility-select') visibility_select = visibility_select_container.find('.visibility-select') visibility_select.select('Private') + expect(visibility_select.value).to eq '0' expect(visibility_select_container).to have_content 'Project access must be granted explicitly to each user.' end @@ -37,6 +38,7 @@ feature 'Visibility settings', feature: true, js: true do scenario 'project visibility is locked' do visibility_select_container = find('.js-visibility-select') + expect(visibility_select_container).not_to have_select '.visibility-select' expect(visibility_select_container).to have_content 'Public' expect(visibility_select_container).to have_content 'The project can be cloned without any authentication.' diff --git a/spec/javascripts/visibility_select_spec.js.es6 b/spec/javascripts/visibility_select_spec.js.es6 new file mode 100644 index 00000000000..b21f6912e06 --- /dev/null +++ b/spec/javascripts/visibility_select_spec.js.es6 @@ -0,0 +1,100 @@ +/*= require visibility_select */ + +(() => { + const VisibilitySelect = gl.VisibilitySelect; + + describe('VisibilitySelect', function () { + const lockedElement = document.createElement('div'); + lockedElement.dataset.helpBlock = 'lockedHelpBlock'; + + const checkedElement = document.createElement('div'); + checkedElement.dataset.description = 'checkedDescription'; + + const mockElements = { + container: document.createElement('div'), + select: document.createElement('div'), + '.help-block': document.createElement('div'), + '.js-locked': lockedElement, + 'option:checked': checkedElement, + }; + + beforeEach(function () { + spyOn(Element.prototype, 'querySelector').and.callFake(selector => mockElements[selector]); + }); + + describe('#constructor', function () { + beforeEach(function () { + this.visibilitySelect = new VisibilitySelect(mockElements.container); + }); + + it('sets the container member', function () { + expect(this.visibilitySelect.container).toEqual(mockElements.container); + }); + + it('queries and sets the helpBlock member', function () { + expect(Element.prototype.querySelector).toHaveBeenCalledWith('.help-block'); + expect(this.visibilitySelect.helpBlock).toEqual(mockElements['.help-block']); + }); + + it('queries and sets the select member', function () { + expect(Element.prototype.querySelector).toHaveBeenCalledWith('select'); + expect(this.visibilitySelect.select).toEqual(mockElements.select); + }); + + describe('if there is no container element provided', function () { + it('throws an error', function () { + expect(() => new VisibilitySelect()).toThrowError('VisibilitySelect requires a container element as argument 1'); + }); + }); + }); + + describe('#init', function () { + describe('if there is a select', function () { + beforeEach(function () { + this.visibilitySelect = new VisibilitySelect(mockElements.container); + }); + + it('calls updateHelpText', function () { + spyOn(VisibilitySelect.prototype, 'updateHelpText'); + this.visibilitySelect.init(); + expect(this.visibilitySelect.updateHelpText).toHaveBeenCalled(); + }); + + it('adds a change event listener', function () { + spyOn(this.visibilitySelect.select, 'addEventListener'); + this.visibilitySelect.init(); + expect(this.visibilitySelect.select.addEventListener.calls.argsFor(0)).toContain('change'); + }); + }); + + describe('if there is no select', function () { + beforeEach(function () { + mockElements.select = undefined; + this.visibilitySelect = new VisibilitySelect(mockElements.container); + this.visibilitySelect.init(); + }); + + it('updates the helpBlock text to the locked `data-help-block` messaged', function () { + expect(this.visibilitySelect.helpBlock.textContent) + .toEqual(lockedElement.dataset.helpBlock); + }); + + afterEach(function () { + mockElements.select = document.createElement('div'); + }); + }); + }); + + describe('#updateHelpText', function () { + beforeEach(function () { + this.visibilitySelect = new VisibilitySelect(mockElements.container); + this.visibilitySelect.init(); + }); + + it('updates the helpBlock text to the selected options `data-description`', function () { + expect(this.visibilitySelect.helpBlock.textContent) + .toEqual(checkedElement.dataset.description); + }); + }); + }); +})();