Fix label `x` button showing for unauthorized users

The `x` button on labels incorrectly shows even for unauthorized
users. When unauthorized users click this the label is not removed
since there are backend authorization checks but this commit
fixes this UX inconsistency.

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