Merge branch '216881-fix-close-button-showing-for-unauthorized-users' into 'master'

Fix label `x` button showing for unauthorized users

See merge request gitlab-org/gitlab!45189

app/assets/javascripts/sidebar/components/labels/sidebar_labels.vue

@@ -77,7 +77,7 @@ export default {
 <template>
   <labels-select
     class="block labels js-labels-block"
-    :allow-label-remove="true"
+    :allow-label-remove="allowLabelEdit"
     :allow-label-create="allowLabelCreate"
     :allow-label-edit="allowLabelEdit"
     :allow-multiselect="true"

ee/app/assets/javascripts/epic/components/sidebar_items/sidebar_labels.vue

@@ -128,7 +128,7 @@ export default {
 
 <template>
   <labels-select-vue
-    :allow-label-remove="true"
+    :allow-label-remove="canUpdate"
     :allow-label-edit="canUpdate"
     :allow-label-create="true"
     :allow-multiselect="true"

spec/features/issues/user_edits_issue_spec.rb

@@ -13,319 +13,349 @@ RSpec.describe "Issues > User edits issue", :js do
   let_it_be(:milestone) { create(:milestone, project: project) }
   let_it_be(:milestones) { create_list(:milestone, 25, project: project_with_milestones) }
 
-  before do
-    project.add_developer(user)
-    project_with_milestones.add_developer(user)
-    sign_in(user)
-  end
-
-  context "from edit page" do
+  context 'with authorized user' do
     before do
-      visit edit_project_issue_path(project, issue)
+      project.add_developer(user)
+      project_with_milestones.add_developer(user)
+      sign_in(user)
     end
 
-    it "previews content" do
-      form = first(".gfm-form")
-
-      page.within(form) do
-        fill_in("Description", with: "Bug fixed :smile:")
-        click_button("Preview")
+    context "from edit page" do
+      before do
+        visit edit_project_issue_path(project, issue)
       end
 
-      expect(form).to have_button("Write")
-    end
+      it "previews content" do
+        form = first(".gfm-form")
 
-    it 'allows user to select unassigned' do
-      visit edit_project_issue_path(project, issue)
+        page.within(form) do
+          fill_in("Description", with: "Bug fixed :smile:")
+          click_button("Preview")
+        end
 
-      expect(page).to have_content "Assignee #{user.name}"
+        expect(form).to have_button("Write")
+      end
 
-      first('.js-user-search').click
-      click_link 'Unassigned'
+      it 'allows user to select unassigned' do
+        visit edit_project_issue_path(project, issue)
 
-      click_button 'Save changes'
+        expect(page).to have_content "Assignee #{user.name}"
 
-      page.within('.assignee') do
-        expect(page).to have_content 'None - assign yourself'
-      end
-    end
+        first('.js-user-search').click
+        click_link 'Unassigned'
 
-    context 'with due date' do
-      before do
-        visit edit_project_issue_path(project, issue)
+        click_button 'Save changes'
+
+        page.within('.assignee') do
+          expect(page).to have_content 'None - assign yourself'
+        end
       end
 
-      it 'saves with due date' do
-        date = Date.today.at_beginning_of_month.tomorrow
+      context 'with due date' do
+        before do
+          visit edit_project_issue_path(project, issue)
+        end
 
-        fill_in 'issue_title', with: 'bug 345'
-        fill_in 'issue_description', with: 'bug description'
-        find('#issuable-due-date').click
+        it 'saves with due date' do
+          date = Date.today.at_beginning_of_month.tomorrow
 
-        page.within '.pika-single' do
-          click_button date.day
-        end
+          fill_in 'issue_title', with: 'bug 345'
+          fill_in 'issue_description', with: 'bug description'
+          find('#issuable-due-date').click
 
-        expect(find('#issuable-due-date').value).to eq date.to_s
+          page.within '.pika-single' do
+            click_button date.day
+          end
 
-        click_button 'Save changes'
+          expect(find('#issuable-due-date').value).to eq date.to_s
 
-        page.within '.issuable-sidebar' do
-          expect(page).to have_content date.to_s(:medium)
+          click_button 'Save changes'
+
+          page.within '.issuable-sidebar' do
+            expect(page).to have_content date.to_s(:medium)
+          end
         end
-      end
 
-      it 'warns about version conflict' do
-        issue.update(title: "New title")
+        it 'warns about version conflict' do
+          issue.update(title: "New title")
 
-        fill_in 'issue_title', with: 'bug 345'
-        fill_in 'issue_description', with: 'bug description'
+          fill_in 'issue_title', with: 'bug 345'
+          fill_in 'issue_description', with: 'bug description'
 
-        click_button 'Save changes'
+          click_button 'Save changes'
 
-        expect(page).to have_content 'Someone edited the issue the same time you did'
+          expect(page).to have_content 'Someone edited the issue the same time you did'
+        end
       end
     end
-  end
 
-  context "from issue#show" do
-    before do
-      visit project_issue_path(project, issue)
-    end
+    context "from issue#show" do
+      before do
+        visit project_issue_path(project, issue)
+      end
 
-    describe 'update labels' do
-      it 'will not send ajax request when no data is changed' do
-        page.within '.labels' do
-          click_on 'Edit'
+      describe 'update labels' do
+        it 'will not send ajax request when no data is changed' do
+          page.within '.labels' do
+            click_on 'Edit'
 
-          find('.dropdown-title button').click
+            find('.dropdown-title button').click
 
-          expect(page).not_to have_selector('.block-loading')
-          expect(page).not_to have_selector('.gl-spinner')
+            expect(page).not_to have_selector('.block-loading')
+            expect(page).not_to have_selector('.gl-spinner')
+          end
         end
-      end
 
-      it 'can add label to issue' do
-        page.within '.block.labels' do
-          expect(page).to have_text('verisimilitude')
-          expect(page).not_to have_text('syzygy')
+        it 'can add label to issue' do
+          page.within '.block.labels' do
+            expect(page).to have_text('verisimilitude')
+            expect(page).not_to have_text('syzygy')
 
-          click_on 'Edit'
+            click_on 'Edit'
 
-          wait_for_requests
+            wait_for_requests
 
-          click_on 'syzygy'
-          find('.dropdown-header-button').click
+            click_on 'syzygy'
+            find('.dropdown-header-button').click
 
-          wait_for_requests
+            wait_for_requests
 
-          expect(page).to have_text('verisimilitude')
-          expect(page).to have_text('syzygy')
+            expect(page).to have_text('verisimilitude')
+            expect(page).to have_text('syzygy')
+          end
         end
-      end
 
-      it 'can remove label from issue by clicking on the label `x` button' do
-        page.within '.block.labels' do
-          expect(page).to have_text('verisimilitude')
+        it 'can remove label from issue by clicking on the label `x` button' do
+          page.within '.block.labels' do
+            expect(page).to have_text('verisimilitude')
 
-          within '.gl-label' do
-            click_button
-          end
+            within '.gl-label' do
+              click_button
+            end
 
-          wait_for_requests
+            wait_for_requests
 
-          expect(page).not_to have_text('verisimilitude')
+            expect(page).not_to have_text('verisimilitude')
+          end
         end
       end
-    end
 
-    describe 'update assignee' do
-      context 'by authorized user' do
-        def close_dropdown_menu_if_visible
-          find('.dropdown-menu-toggle', visible: :all).tap do |toggle|
-            toggle.click if toggle.visible?
+      describe 'update assignee' do
+        context 'by authorized user' do
+          def close_dropdown_menu_if_visible
+            find('.dropdown-menu-toggle', visible: :all).tap do |toggle|
+              toggle.click if toggle.visible?
+            end
           end
-        end
 
-        it 'allows user to select unassigned' do
-          visit project_issue_path(project, issue)
+          it 'allows user to select unassigned' do
+            visit project_issue_path(project, issue)
 
-          page.within('.assignee') do
-            expect(page).to have_content "#{user.name}"
+            page.within('.assignee') do
+              expect(page).to have_content "#{user.name}"
 
-            click_link 'Edit'
-            click_link 'Unassigned'
-            first('.title').click
-            expect(page).to have_content 'None - assign yourself'
+              click_link 'Edit'
+              click_link 'Unassigned'
+              first('.title').click
+              expect(page).to have_content 'None - assign yourself'
+            end
           end
-        end
 
-        it 'allows user to select an assignee' do
-          issue2 = create(:issue, project: project, author: user)
-          visit project_issue_path(project, issue2)
+          it 'allows user to select an assignee' do
+            issue2 = create(:issue, project: project, author: user)
+            visit project_issue_path(project, issue2)
 
-          page.within('.assignee') do
-            expect(page).to have_content "None"
-          end
+            page.within('.assignee') do
+              expect(page).to have_content "None"
+            end
 
-          page.within '.assignee' do
-            click_link 'Edit'
-          end
+            page.within '.assignee' do
+              click_link 'Edit'
+            end
 
-          page.within '.dropdown-menu-user' do
-            click_link user.name
-          end
+            page.within '.dropdown-menu-user' do
+              click_link user.name
+            end
 
-          page.within('.assignee') do
-            expect(page).to have_content user.name
+            page.within('.assignee') do
+              expect(page).to have_content user.name
+            end
           end
-        end
 
-        it 'allows user to unselect themselves' do
-          issue2 = create(:issue, project: project, author: user, assignees: [user])
+          it 'allows user to unselect themselves' do
+            issue2 = create(:issue, project: project, author: user, assignees: [user])
 
-          visit project_issue_path(project, issue2)
+            visit project_issue_path(project, issue2)
 
-          page.within '.assignee' do
-            expect(page).to have_content user.name
+            page.within '.assignee' do
+              expect(page).to have_content user.name
 
-            click_link 'Edit'
-            click_link user.name
+              click_link 'Edit'
+              click_link user.name
 
-            close_dropdown_menu_if_visible
+              close_dropdown_menu_if_visible
 
-            page.within '.value .assign-yourself' do
-              expect(page).to have_content "None"
+              page.within '.value .assign-yourself' do
+                expect(page).to have_content "None"
+              end
             end
           end
         end
-      end
 
-      context 'by unauthorized user' do
-        let(:guest) { create(:user) }
+        context 'by unauthorized user' do
+          let(:guest) { create(:user) }
 
-        before do
-          project.add_guest(guest)
-        end
+          before do
+            project.add_guest(guest)
+          end
 
-        it 'shows assignee text' do
-          sign_out(:user)
-          sign_in(guest)
+          it 'shows assignee text' do
+            sign_out(:user)
+            sign_in(guest)
 
-          visit project_issue_path(project, issue)
-          expect(page).to have_content issue.assignees.first.name
+            visit project_issue_path(project, issue)
+            expect(page).to have_content issue.assignees.first.name
+          end
         end
       end
-    end
 
-    describe 'update milestone' do
-      context 'by authorized user' do
-        it 'allows user to select unassigned' do
-          visit project_issue_path(project, issue)
+      describe 'update milestone' do
+        context 'by authorized user' do
+          it 'allows user to select unassigned' do
+            visit project_issue_path(project, issue)
 
-          page.within('.milestone') do
-            expect(page).to have_content "None"
-          end
+            page.within('.milestone') do
+              expect(page).to have_content "None"
+            end
 
-          find('.block.milestone .edit-link').click
-          sleep 2 # wait for ajax stuff to complete
-          first('.dropdown-content li').click
-          sleep 2
-          page.within('.milestone') do
-            expect(page).to have_content 'None'
+            find('.block.milestone .edit-link').click
+            sleep 2 # wait for ajax stuff to complete
+            first('.dropdown-content li').click
+            sleep 2
+            page.within('.milestone') do
+              expect(page).to have_content 'None'
+            end
           end
-        end
 
-        it 'allows user to de-select milestone' do
-          visit project_issue_path(project, issue)
+          it 'allows user to de-select milestone' do
+            visit project_issue_path(project, issue)
 
-          page.within('.milestone') do
-            click_link 'Edit'
-            click_link milestone.title
+            page.within('.milestone') do
+              click_link 'Edit'
+              click_link milestone.title
 
-            page.within '.value' do
-              expect(page).to have_content milestone.title
-            end
+              page.within '.value' do
+                expect(page).to have_content milestone.title
+              end
 
-            click_link 'Edit'
-            click_link milestone.title
+              click_link 'Edit'
+              click_link milestone.title
 
-            page.within '.value' do
-              expect(page).to have_content 'None'
+              page.within '.value' do
+                expect(page).to have_content 'None'
+              end
             end
           end
-        end
 
-        it 'allows user to search milestone' do
-          visit project_issue_path(project_with_milestones, issue_with_milestones)
+          it 'allows user to search milestone' do
+            visit project_issue_path(project_with_milestones, issue_with_milestones)
 
-          page.within('.milestone') do
-            click_link 'Edit'
-            wait_for_requests
-            # We need to enclose search string in quotes for exact match as all the milestone titles
-            # within tests are prefixed with `My title`.
-            find('.dropdown-input-field', visible: true).send_keys "\"#{milestones[0].title}\""
-            wait_for_requests
+            page.within('.milestone') do
+              click_link 'Edit'
+              wait_for_requests
+              # We need to enclose search string in quotes for exact match as all the milestone titles
+              # within tests are prefixed with `My title`.
+              find('.dropdown-input-field', visible: true).send_keys "\"#{milestones[0].title}\""
+              wait_for_requests
 
-            page.within '.dropdown-content' do
-              expect(page).to have_content milestones[0].title
+              page.within '.dropdown-content' do
+                expect(page).to have_content milestones[0].title
+              end
             end
           end
         end
-      end
 
-      context 'by unauthorized user' do
-        let(:guest) { create(:user) }
+        context 'by unauthorized user' do
+          let(:guest) { create(:user) }
 
-        before do
-          project.add_guest(guest)
-          issue.milestone = milestone
-          issue.save
-        end
+          before do
+            project.add_guest(guest)
+            issue.milestone = milestone
+            issue.save
+          end
 
-        it 'shows milestone text' do
-          sign_out(:user)
-          sign_in(guest)
+          it 'shows milestone text' do
+            sign_out(:user)
+            sign_in(guest)
 
-          visit project_issue_path(project, issue)
-          expect(page).to have_content milestone.title
+            visit project_issue_path(project, issue)
+            expect(page).to have_content milestone.title
+          end
         end
       end
-    end
 
-    context 'update due date' do
-      it 'adds due date to issue' do
-        date = Date.today.at_beginning_of_month + 2.days
+      context 'update due date' do
+        it 'adds due date to issue' do
+          date = Date.today.at_beginning_of_month + 2.days
 
-        page.within '.due_date' do
-          click_link 'Edit'
+          page.within '.due_date' do
+            click_link 'Edit'
 
-          page.within '.pika-single' do
-            click_button date.day
-          end
+            page.within '.pika-single' do
+              click_button date.day
+            end
 
-          wait_for_requests
+            wait_for_requests
 
-          expect(find('.value').text).to have_content date.strftime('%b %-d, %Y')
+            expect(find('.value').text).to have_content date.strftime('%b %-d, %Y')
+          end
         end
-      end
 
-      it 'removes due date from issue' do
-        date = Date.today.at_beginning_of_month + 2.days
+        it 'removes due date from issue' do
+          date = Date.today.at_beginning_of_month + 2.days
 
-        page.within '.due_date' do
-          click_link 'Edit'
+          page.within '.due_date' do
+            click_link 'Edit'
 
-          page.within '.pika-single' do
-            click_button date.day
+            page.within '.pika-single' do
+              click_button date.day
+            end
+
+            wait_for_requests
+
+            expect(page).to have_no_content 'None'
+
+            click_link 'remove due date'
+            expect(page).to have_content 'None'
           end
+        end
+      end
+    end
+  end
+
+  context 'with unauthorized user' do
+    before do
+      sign_in(user)
+    end
 
-          wait_for_requests
+    context "from issue#show" do
+      before do
+        visit project_issue_path(project, issue)
+      end
 
-          expect(page).to have_no_content 'None'
+      describe 'updating labels' do
+        it 'cannot edit labels' do
+          page.within '.block.labels' do
+            expect(page).not_to have_button('Edit')
+          end
+        end
 
-          click_link 'remove due date'
-          expect(page).to have_content 'None'
+        it 'cannot remove label with a click as it has no `x` button' do
+          page.within '.block.labels' do
+            within '.gl-label' do
+              expect(page).not_to have_button
+            end
+          end
         end
       end
     end