Resolve "Make project and features visibility settings less confusing"

5 files changed, 58 insertions, 52 deletions

diff --git a/spec/features/projects/edit_spec.rb b/spec/features/projects/edit_spec.rb
index d3b1d1f7be3..17f914c9c17 100644
--- a/spec/features/projects/edit_spec.rb
+++ b/spec/features/projects/edit_spec.rb
@@ -1,20 +1,21 @@
 require 'rails_helper'
 
 feature 'Project edit', js: true do
+  let(:admin)   { create(:admin) }
   let(:user)    { create(:user) }
   let(:project) { create(:project) }
 
-  before do
-    project.team << [user, :master]
-    sign_in(user)
+  context 'feature visibility' do
+    before do
+      project.team << [user, :master]
+      sign_in(user)
 
-    visit edit_project_path(project)
-  end
+      visit edit_project_path(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')
+        find('.project-feature-controls[data-for="project[project_feature_attributes][merge_requests_access_level]"] .project-feature-toggle').click
 
         expect(page).to have_selector('.merge-requests-feature', visible: false)
       end
@@ -30,7 +31,7 @@ feature 'Project edit', js: true do
 
     context 'builds select' do
       it 'hides builds select section' do
-        select('Disabled', from: 'project_project_feature_attributes_builds_access_level')
+        find('.project-feature-controls[data-for="project[project_feature_attributes][builds_access_level]"] .project-feature-toggle').click
 
         expect(page).to have_selector('.builds-feature', visible: false)
       end
@@ -44,4 +45,18 @@ feature 'Project edit', js: true do
       end
     end
   end
+
+  context 'LFS enabled setting' do
+    before do
+      sign_in(admin)
+    end
+
+    it 'displays the correct elements' do
+      allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
+      visit edit_project_path(project)
+
+      expect(page).to have_content('Git Large File Storage')
+      expect(page).to have_selector('input[name="project[lfs_enabled]"] + button', visible: true)
+    end
+  end
 end
diff --git a/spec/features/projects/features_visibility_spec.rb b/spec/features/projects/features_visibility_spec.rb
index 24691629063..57722276d79 100644
--- a/spec/features/projects/features_visibility_spec.rb
+++ b/spec/features/projects/features_visibility_spec.rb
@@ -19,21 +19,16 @@ describe 'Edit Project Settings' do
         it 'toggles visibility' do
           visit edit_project_path(project)
 
-          select 'Disabled', from: "project_project_feature_attributes_#{tool_name}_access_level"
+          # disable by clicking toggle
+          toggle_feature_off("project[project_feature_attributes][#{tool_name}_access_level]")
           page.within('.sharing-permissions') do
             click_button 'Save changes'
           end
           wait_for_requests
           expect(page).not_to have_selector(".shortcuts-#{shortcut_name}")
 
-          select 'Everyone with access', from: "project_project_feature_attributes_#{tool_name}_access_level"
-          page.within('.sharing-permissions') do
-            click_button 'Save changes'
-          end
-          wait_for_requests
-          expect(page).to have_selector(".shortcuts-#{shortcut_name}")
-
-          select 'Only team members', from: "project_project_feature_attributes_#{tool_name}_access_level"
+          # re-enable by clicking toggle again
+          toggle_feature_on("project[project_feature_attributes][#{tool_name}_access_level]")
           page.within('.sharing-permissions') do
             click_button 'Save changes'
           end
@@ -176,19 +171,19 @@ describe 'Edit Project Settings' do
     end
 
     it "disables repository related features" do
-      select "Disabled", from: "project_project_feature_attributes_repository_access_level"
+      toggle_feature_off('project[project_feature_attributes][repository_access_level]')
 
       page.within('.sharing-permissions') do
         click_button "Save changes"
       end
 
-      expect(find(".sharing-permissions")).to have_selector("select.disabled", count: 2)
+      expect(find(".sharing-permissions")).to have_selector(".project-feature-toggle.disabled", count: 2)
     end
 
     it "shows empty features project homepage" do
-      select "Disabled", from: "project_project_feature_attributes_repository_access_level"
-      select "Disabled", from: "project_project_feature_attributes_issues_access_level"
-      select "Disabled", from: "project_project_feature_attributes_wiki_access_level"
+      toggle_feature_off('project[project_feature_attributes][repository_access_level]')
+      toggle_feature_off('project[project_feature_attributes][issues_access_level]')
+      toggle_feature_off('project[project_feature_attributes][wiki_access_level]')
 
       page.within('.sharing-permissions') do
         click_button "Save changes"
@@ -201,9 +196,9 @@ describe 'Edit Project Settings' do
     end
 
     it "hides project activity tabs" do
-      select "Disabled", from: "project_project_feature_attributes_repository_access_level"
-      select "Disabled", from: "project_project_feature_attributes_issues_access_level"
-      select "Disabled", from: "project_project_feature_attributes_wiki_access_level"
+      toggle_feature_off('project[project_feature_attributes][repository_access_level]')
+      toggle_feature_off('project[project_feature_attributes][issues_access_level]')
+      toggle_feature_off('project[project_feature_attributes][wiki_access_level]')
 
       page.within('.sharing-permissions') do
         click_button "Save changes"
@@ -222,7 +217,7 @@ describe 'Edit Project Settings' do
 
     # Regression spec for https://gitlab.com/gitlab-org/gitlab-ce/issues/25272
     it "hides comments activity tab only on disabled issues, merge requests and repository" do
-      select "Disabled", from: "project_project_feature_attributes_issues_access_level"
+      toggle_feature_off('project[project_feature_attributes][issues_access_level]')
 
       save_changes_and_check_activity_tab do
         expect(page).to have_content("Comments")
@@ -230,7 +225,7 @@ describe 'Edit Project Settings' do
 
       visit edit_project_path(project)
 
-      select "Disabled", from: "project_project_feature_attributes_merge_requests_access_level"
+      toggle_feature_off('project[project_feature_attributes][merge_requests_access_level]')
 
       save_changes_and_check_activity_tab do
         expect(page).to have_content("Comments")
@@ -238,7 +233,7 @@ describe 'Edit Project Settings' do
 
       visit edit_project_path(project)
 
-      select "Disabled", from: "project_project_feature_attributes_repository_access_level"
+      toggle_feature_off('project[project_feature_attributes][repository_access_level]')
 
       save_changes_and_check_activity_tab do
         expect(page).not_to have_content("Comments")
@@ -275,4 +270,12 @@ describe 'Edit Project Settings' do
       expect(page).not_to have_selector('.project-stats')
     end
   end
+
+  def toggle_feature_off(feature_name)
+    find(".project-feature-controls[data-for=\"#{feature_name}\"] .project-feature-toggle.checked").click
+  end
+
+  def toggle_feature_on(feature_name)
+    find(".project-feature-controls[data-for=\"#{feature_name}\"] .project-feature-toggle:not(.checked)").click
+  end
 end
diff --git a/spec/features/projects/settings/merge_requests_settings_spec.rb b/spec/features/projects/settings/merge_requests_settings_spec.rb
index 104ce08d9f3..b1ec556bf16 100644
--- a/spec/features/projects/settings/merge_requests_settings_spec.rb
+++ b/spec/features/projects/settings/merge_requests_settings_spec.rb
@@ -19,8 +19,8 @@ feature 'Project settings > Merge Requests', :js do
         expect(page).to have_content('Only allow merge requests to be merged if the pipeline succeeds')
         expect(page).to have_content('Only allow merge requests to be merged if all discussions are resolved')
 
-        select 'Disabled', from: "project_project_feature_attributes_merge_requests_access_level"
         within('.sharing-permissions-form') do
+          find('.project-feature-controls[data-for="project[project_feature_attributes][merge_requests_access_level]"] .project-feature-toggle').click
           click_on('Save changes')
         end
 
@@ -39,8 +39,8 @@ feature 'Project settings > Merge Requests', :js do
         expect(page).not_to have_content('Only allow merge requests to be merged if the pipeline succeeds')
         expect(page).to have_content('Only allow merge requests to be merged if all discussions are resolved')
 
-        select 'Everyone with access', from: "project_project_feature_attributes_builds_access_level"
         within('.sharing-permissions-form') do
+          find('.project-feature-controls[data-for="project[project_feature_attributes][builds_access_level]"] .project-feature-toggle').click
           click_on('Save changes')
         end
 
@@ -60,8 +60,8 @@ feature 'Project settings > Merge Requests', :js do
       expect(page).not_to have_content('Only allow merge requests to be merged if the pipeline succeeds')
       expect(page).not_to have_content('Only allow merge requests to be merged if all discussions are resolved')
 
-      select 'Everyone with access', from: "project_project_feature_attributes_merge_requests_access_level"
       within('.sharing-permissions-form') do
+        find('.project-feature-controls[data-for="project[project_feature_attributes][merge_requests_access_level]"] .project-feature-toggle').click
         click_on('Save changes')
       end
 
diff --git a/spec/features/projects/settings/visibility_settings_spec.rb b/spec/features/projects/settings/visibility_settings_spec.rb
index 1756c7d00fe..37ee6255bd1 100644
--- a/spec/features/projects/settings/visibility_settings_spec.rb
+++ b/spec/features/projects/settings/visibility_settings_spec.rb
@@ -11,19 +11,19 @@ feature 'Visibility settings', js: true do
     end
 
     scenario 'project visibility select is available' do
-      visibility_select_container = find('.js-visibility-select')
+      visibility_select_container = find('.project-visibility-setting')
 
-      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 accessed without any authentication.'
+      expect(visibility_select_container.find('select').value).to eq project.visibility_level.to_s
+      expect(visibility_select_container).to have_content 'The project can be accessed by anyone, regardless of authentication.'
     end
 
     scenario 'project visibility description updates on change' do
-      visibility_select_container = find('.js-visibility-select')
-      visibility_select = visibility_select_container.find('.visibility-select')
+      visibility_select_container = find('.project-visibility-setting')
+      visibility_select = visibility_select_container.find('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.'
+      expect(visibility_select_container).to have_content 'Access must be granted explicitly to each user.'
     end
   end
 
@@ -37,11 +37,10 @@ feature 'Visibility settings', js: true do
     end
 
     scenario 'project visibility is locked' do
-      visibility_select_container = find('.js-visibility-select')
+      visibility_select_container = find('.project-visibility-setting')
 
-      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 accessed without any authentication.'
+      expect(visibility_select_container).to have_selector 'select[name="project[visibility_level]"]:disabled'
+      expect(visibility_select_container).to have_content 'The project can be accessed by anyone, regardless of authentication.'
     end
   end
 end
diff --git a/spec/views/projects/edit.html.haml_spec.rb b/spec/views/projects/edit.html.haml_spec.rb
index c1398629749..5c6b2e4b042 100644
--- a/spec/views/projects/edit.html.haml_spec.rb
+++ b/spec/views/projects/edit.html.haml_spec.rb
@@ -15,17 +15,6 @@ describe 'projects/edit' do
                                     current_application_settings: Gitlab::CurrentSettings.current_application_settings)
   end
 
-  context 'LFS enabled setting' do
-    it 'displays the correct elements' do
-      allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
-
-      render
-
-      expect(rendered).to have_select('project_lfs_enabled')
-      expect(rendered).to have_content('Git Large File Storage')
-    end
-  end
-
   context 'project export disabled' do
     it 'does not display the project export option' do
       stub_application_setting(project_export_enabled?: false)