From 4da474ca928fff916a1e8ac3bb42d81df2364431 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 2 Sep 2016 12:03:49 +0100 Subject: [PATCH 1/2] Hides merge request section in edit project when disabled --- app/assets/javascripts/project_new.js | 8 +-- .../_merge_request_settings.html.haml | 25 ++++---- spec/features/projects/edit_spec.rb | 57 +++++++++++++++++++ 3 files changed, 75 insertions(+), 15 deletions(-) create mode 100644 spec/features/projects/edit_spec.rb diff --git a/app/assets/javascripts/project_new.js b/app/assets/javascripts/project_new.js index 798f15e40a0..06034574c8a 100644 --- a/app/assets/javascripts/project_new.js +++ b/app/assets/javascripts/project_new.js @@ -15,18 +15,18 @@ } ProjectNew.prototype.toggleSettings = function() { - this._showOrHide('#project_builds_enabled', '.builds-feature'); - return this._showOrHide('#project_merge_requests_enabled', '.merge-requests-feature'); + this._showOrHide('#project_project_feature_attributes_builds_access_level', '.builds-feature'); + this._showOrHide('#project_project_feature_attributes_merge_requests_access_level', '.merge-requests-feature'); }; ProjectNew.prototype.toggleSettingsOnclick = function() { - return $('#project_builds_enabled, #project_merge_requests_enabled').on('click', this.toggleSettings); + $('#project_project_feature_attributes_builds_access_level, #project_project_feature_attributes_merge_requests_access_level').on('change', this.toggleSettings); }; ProjectNew.prototype._showOrHide = function(checkElement, container) { var $container; $container = $(container); - if ($(checkElement).prop('checked')) { + if ($(checkElement).val() !== '0') { return $container.show(); } else { return $container.hide(); diff --git a/app/views/projects/_merge_request_settings.html.haml b/app/views/projects/_merge_request_settings.html.haml index 19b4249374b..14eb47089b1 100644 --- a/app/views/projects/_merge_request_settings.html.haml +++ b/app/views/projects/_merge_request_settings.html.haml @@ -1,11 +1,14 @@ -%fieldset.builds-feature - %h5.prepend-top-0 - Merge Requests - .form-group - .checkbox - = f.label :only_allow_merge_if_build_succeeds do - = f.check_box :only_allow_merge_if_build_succeeds - %strong Only allow merge requests to be merged if the build succeeds - .help-block - Builds need to be configured to enable this feature. - = link_to icon('question-circle'), help_page_path('workflow/merge_requests', anchor: 'only-allow-merge-requests-to-be-merged-if-the-build-succeeds') +.merge-requests-feature + %fieldset.builds-feature + %hr + %h5.prepend-top-0 + Merge Requests + .form-group + .checkbox + = f.label :only_allow_merge_if_build_succeeds do + = f.check_box :only_allow_merge_if_build_succeeds + %strong Only allow merge requests to be merged if the build succeeds + %br + %span.descr + Builds need to be configured to enable this feature. + = link_to icon('question-circle'), help_page_path('workflow/merge_requests', anchor: 'only-allow-merge-requests-to-be-merged-if-the-build-succeeds') diff --git a/spec/features/projects/edit_spec.rb b/spec/features/projects/edit_spec.rb new file mode 100644 index 00000000000..a1643fd1f43 --- /dev/null +++ b/spec/features/projects/edit_spec.rb @@ -0,0 +1,57 @@ +require 'rails_helper' + +feature 'Project edit', feature: true, js: true do + include WaitForAjax + + let(:user) { create(:user) } + let(:project) { create(:project) } + + before do + project.team << [user, :master] + login_as(user) + + visit edit_namespace_project_path(project.namespace, project) + end + + context 'feature visibility' do + context 'merge requests select' do + it 'hides merge requests section' do + select('Disabled', from: 'project_project_feature_attributes_merge_requests_access_level') + + expect(page).to have_selector('.merge-requests-feature', visible: false) + end + + it 'hides merge requests section after save' do + select('Disabled', from: 'project_project_feature_attributes_merge_requests_access_level') + + expect(page).to have_selector('.merge-requests-feature', visible: false) + + click_button 'Save changes' + + wait_for_ajax + + expect(page).to have_selector('.merge-requests-feature', visible: false) + end + end + + context 'builds select' do + it 'hides merge requests section' do + select('Disabled', from: 'project_project_feature_attributes_builds_access_level') + + expect(page).to have_selector('.builds-feature', visible: false) + end + + it 'hides merge requests section after save' do + select('Disabled', from: 'project_project_feature_attributes_builds_access_level') + + expect(page).to have_selector('.builds-feature', visible: false) + + click_button 'Save changes' + + wait_for_ajax + + expect(page).to have_selector('.builds-feature', visible: false) + end + end + end +end From f8513d768242dffbd0fd0230e0bbeca972bcf0ec Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 2 Sep 2016 14:51:36 +0100 Subject: [PATCH 2/2] Refactored code to rely less on IDs that could change --- app/assets/javascripts/project_new.js | 18 +++++++++++++----- app/helpers/projects_helper.rb | 2 +- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/project_new.js b/app/assets/javascripts/project_new.js index 06034574c8a..a787b11f2a9 100644 --- a/app/assets/javascripts/project_new.js +++ b/app/assets/javascripts/project_new.js @@ -4,6 +4,8 @@ this.ProjectNew = (function() { function ProjectNew() { this.toggleSettings = bind(this.toggleSettings, this); + this.$selects = $('.features select'); + $('.project-edit-container').on('ajax:before', (function(_this) { return function() { $('.project-edit-container').hide(); @@ -15,17 +17,23 @@ } ProjectNew.prototype.toggleSettings = function() { - this._showOrHide('#project_project_feature_attributes_builds_access_level', '.builds-feature'); - this._showOrHide('#project_project_feature_attributes_merge_requests_access_level', '.merge-requests-feature'); + var self = this; + + this.$selects.each(function () { + var $select = $(this), + className = $select.data('field').replace(/_/g, '-') + .replace('access-level', 'feature'); + self._showOrHide($select, '.' + className); + }); }; ProjectNew.prototype.toggleSettingsOnclick = function() { - $('#project_project_feature_attributes_builds_access_level, #project_project_feature_attributes_merge_requests_access_level').on('change', this.toggleSettings); + this.$selects.on('change', this.toggleSettings); }; ProjectNew.prototype._showOrHide = function(checkElement, container) { - var $container; - $container = $(container); + var $container = $(container); + if ($(checkElement).val() !== '0') { return $container.show(); } else { diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index d6efa603223..4c685b97c03 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -431,6 +431,6 @@ module ProjectsHelper options.delete('Everyone with access') if @project.private? && level != ProjectFeature::ENABLED options = options_for_select(options, selected: @project.project_feature.public_send(field) || ProjectFeature::ENABLED) - content_tag(:select, options, name: "project[project_feature_attributes][#{field.to_s}]", id: "project_project_feature_attributes_#{field.to_s}", class: "pull-right form-control").html_safe + content_tag(:select, options, name: "project[project_feature_attributes][#{field.to_s}]", id: "project_project_feature_attributes_#{field.to_s}", class: "pull-right form-control", data: { field: field }).html_safe end end