Apply reviewer suggestions

- Use let_it_be over let for performance
- Remove repeated, unnecessary code

ee/spec/features/boards/sidebar_deprecated_spec.rb

@@ -8,19 +8,20 @@ require 'spec_helper'
 RSpec.describe 'Issue Boards', :js do
   include BoardHelpers
 
-  let(:user)         { create(:user) }
-  let(:user2)        { create(:user) }
-  let(:group)        { create(:group) }
-  let(:project)      { create(:project, :public, group: group) }
-  let!(:milestone)   { create(:milestone, project: project) }
-  let!(:development) { create(:label, project: project, name: 'Development') }
-  let!(:stretch)     { create(:label, project: project, name: 'Stretch') }
-  let!(:issue1)      { create(:labeled_issue, project: project, assignees: [user], milestone: milestone, labels: [development], weight: 3, relative_position: 2) }
-  let!(:issue2)      { create(:labeled_issue, project: project, labels: [development, stretch], relative_position: 1) }
-  let!(:scoped_label_1) { create(:label, project: project, name: 'Scoped1::Label1') }
-  let!(:scoped_label_2) { create(:label, project: project, name: 'Scoped2::Label2') }
-  let(:board)        { create(:board, project: project) }
-  let!(:list)        { create(:list, board: board, label: development, position: 0) }
+  let_it_be(:user)         { create(:user) }
+  let_it_be(:user2)        { create(:user) }
+  let_it_be(:group)        { create(:group) }
+  let_it_be(:project)      { create(:project, :public, group: group) }
+  let_it_be(:milestone)    { create(:milestone, project: project) }
+  let_it_be(:development)  { create(:label, project: project, name: 'Development') }
+  let_it_be(:stretch)      { create(:label, project: project, name: 'Stretch') }
+  let_it_be(:issue1)       { create(:labeled_issue, project: project, assignees: [user], milestone: milestone, labels: [development], weight: 3, relative_position: 2) }
+  let_it_be(:issue2)       { create(:labeled_issue, project: project, labels: [development, stretch], relative_position: 1) }
+  let_it_be(:board)        { create(:board, project: project) }
+  let_it_be(:list)         { create(:list, board: board, label: development, position: 0) }
+  let_it_be(:scoped_label_1) { create(:label, project: project, name: 'Scoped1::Label1') }
+  let_it_be(:scoped_label_2) { create(:label, project: project, name: 'Scoped2::Label2') }
+
   let(:card1) { find('.board:nth-child(2)').find('.board-card:nth-child(2)') }
   let(:card2) { find('.board:nth-child(2)').find('.board-card:nth-child(1)') }
 
@@ -79,8 +80,10 @@ RSpec.describe 'Issue Boards', :js do
         click_button('Apply')
         wait_for_requests
 
-        expect(page).to have_link(nil, title: user.name)
-        expect(page).to have_link(nil, title: assignee)
+        aggregate_failures do
+          expect(page).to have_link(nil, title: user.name)
+          expect(page).to have_link(nil, title: assignee)
+        end
       end
 
       expect(card1.all('.avatar').length).to eq(2)
@@ -91,14 +94,12 @@ RSpec.describe 'Issue Boards', :js do
 
       page.within('.assignee') do
         click_button('Edit')
-        wait_for_requests
 
         page.within('.dropdown-menu-user') do
           find('[data-testid="unassign"]').click
         end
 
         click_button('Apply')
-        wait_for_requests
 
         expect(page).to have_content('None')
       end
@@ -109,9 +110,7 @@ RSpec.describe 'Issue Boards', :js do
     it 'assignees to current user' do
       click_card(card2)
 
-      wait_for_requests
-
-      page.within(find('.assignee')) do
+      page.within('.assignee') do
         expect(page).to have_content('None')
 
         click_button 'assign yourself'
@@ -143,9 +142,7 @@ RSpec.describe 'Issue Boards', :js do
         expect(page).to have_content(assignee)
       end
 
-      page.within(find('.board:nth-child(2)')) do
-        find('.board-card:nth-child(2)').click
-      end
+      click_card(card1)
 
       page.within('.assignee') do
         click_button('Edit')
@@ -167,7 +164,6 @@ RSpec.describe 'Issue Boards', :js do
     context 'when the issue is not associated with an epic' do
       it 'displays `None` for value of epic' do
         click_card(card1)
-        wait_for_requests
 
         expect(find('.js-epic-label').text).to have_content('None')
       end
@@ -180,16 +176,14 @@ RSpec.describe 'Issue Boards', :js do
 
       it 'displays name of epic and links to it' do
         click_card(card1)
-        wait_for_requests
 
         expect(find('.js-epic-label')).to have_link(epic1.title, href: epic_path(epic1))
       end
 
       it 'updates the epic associated with the issue' do
         click_card(card1)
-        wait_for_requests
 
-        page.within(find('.js-epic-block')) do
+        page.within('.js-epic-block') do
           page.find('.sidebar-dropdown-toggle').click
           wait_for_requests
 
@@ -201,10 +195,8 @@ RSpec.describe 'Issue Boards', :js do
 
         # Ensure that boards_store is also updated the epic associated with the issue.
         click_card(card1)
-        wait_for_requests
 
         click_card(card1)
-        wait_for_requests
 
         expect(find('.js-epic-label')).to have_content(epic2.title)
       end
@@ -214,14 +206,12 @@ RSpec.describe 'Issue Boards', :js do
   context 'weight' do
     it 'displays weight async' do
       click_card(card1)
-      wait_for_requests
 
       expect(find('.js-weight-weight-label').text).to have_content(issue1.weight)
     end
 
     it 'updates weight in sidebar to 1' do
       click_card(card1)
-      wait_for_requests
 
       page.within '.weight' do
         click_link 'Edit'
@@ -237,18 +227,14 @@ RSpec.describe 'Issue Boards', :js do
       wait_for_requests
 
       click_card(card1)
-      wait_for_requests
 
-      page.within '.weight' do
-        page.within '.value' do
-          expect(page).to have_content '1'
-        end
+      page.within '.weight .value' do
+        expect(page).to have_content '1'
       end
     end
 
     it 'updates weight in sidebar to no weight' do
       click_card(card1)
-      wait_for_requests
 
       page.within '.weight' do
         click_link 'remove weight'
@@ -263,12 +249,9 @@ RSpec.describe 'Issue Boards', :js do
       wait_for_requests
 
       click_card(card1)
-      wait_for_requests
 
-      page.within '.weight' do
-        page.within '.value' do
-          expect(page).to have_content 'None'
-        end
+      page.within '.weight .value' do
+        expect(page).to have_content 'None'
       end
     end
 
@@ -281,7 +264,6 @@ RSpec.describe 'Issue Boards', :js do
 
       it 'hides weight' do
         click_card(card1)
-        wait_for_requests
 
         expect(page).not_to have_selector('.js-weight-weight-label')
       end
@@ -315,18 +297,19 @@ RSpec.describe 'Issue Boards', :js do
         find('.dropdown-menu-close-icon').click
 
         page.within('.value') do
-          expect(page).to have_selector('.gl-label-scoped', count: 2)
-          expect(page).to have_content(scoped_label_1.scoped_label_key)
-          expect(page).to have_content(scoped_label_1.scoped_label_value)
-          expect(page).to have_content(scoped_label_2.scoped_label_key)
-          expect(page).to have_content(scoped_label_2.scoped_label_value)
+          aggregate_failures do
+            expect(page).to have_selector('.gl-label-scoped', count: 2)
+            expect(page).to have_content(scoped_label_1.scoped_label_key)
+            expect(page).to have_content(scoped_label_1.scoped_label_value)
+            expect(page).to have_content(scoped_label_2.scoped_label_key)
+            expect(page).to have_content(scoped_label_2.scoped_label_value)
+          end
         end
       end
     end
 
     context 'with scoped label assigned' do
       let!(:issue3) { create(:labeled_issue, project: project, labels: [development, scoped_label_1, scoped_label_2], relative_position: 3) }
-      let(:board) { create(:board, project: project) }
       let(:card3) { find('.board:nth-child(2)').find('.board-card:nth-child(1)') }
 
       before do
@@ -351,18 +334,22 @@ RSpec.describe 'Issue Boards', :js do
           find('.dropdown-menu-close-icon').click
 
           page.within('.value') do
-            expect(page).to have_selector('.gl-label-scoped', count: 1)
-            expect(page).not_to have_content(scoped_label_1.scoped_label_value)
-            expect(page).to have_content(scoped_label_2.scoped_label_key)
-            expect(page).to have_content(scoped_label_2.scoped_label_value)
+            aggregate_failures do
+              expect(page).to have_selector('.gl-label-scoped', count: 1)
+              expect(page).not_to have_content(scoped_label_1.scoped_label_value)
+              expect(page).to have_content(scoped_label_2.scoped_label_key)
+              expect(page).to have_content(scoped_label_2.scoped_label_value)
+            end
           end
         end
 
-        expect(card3).to have_selector('.gl-label-scoped', count: 1)
-        expect(card3).not_to have_content(scoped_label_1.scoped_label_key)
-        expect(card3).not_to have_content(scoped_label_1.scoped_label_value)
-        expect(card3).to have_content(scoped_label_2.scoped_label_key)
-        expect(card3).to have_content(scoped_label_2.scoped_label_value)
+        aggregate_failures do
+          expect(card3).to have_selector('.gl-label-scoped', count: 1)
+          expect(card3).not_to have_content(scoped_label_1.scoped_label_key)
+          expect(card3).not_to have_content(scoped_label_1.scoped_label_value)
+          expect(card3).to have_content(scoped_label_2.scoped_label_key)
+          expect(card3).to have_content(scoped_label_2.scoped_label_value)
+        end
       end
     end
   end

ee/spec/features/boards/sidebar_spec.rb

@@ -5,24 +5,29 @@ require 'spec_helper'
 RSpec.describe 'Issue Boards', :js do
   include BoardHelpers
 
-  let(:user)         { create(:user) }
-  let(:user2)        { create(:user) }
-  let(:group)        { create(:group) }
-  let(:project)      { create(:project, :public, group: group) }
-  let!(:milestone)   { create(:milestone, project: project) }
-  let!(:development) { create(:label, project: project, name: 'Development') }
-  let!(:stretch)     { create(:label, project: project, name: 'Stretch') }
-  let!(:issue1)      { create(:labeled_issue, project: project, assignees: [user], milestone: milestone, labels: [development], weight: 3, relative_position: 2) }
-  let!(:issue2)      { create(:labeled_issue, project: project, labels: [development, stretch], relative_position: 1) }
-  let!(:scoped_label_1) { create(:label, project: project, name: 'Scoped1::Label1') }
-  let!(:scoped_label_2) { create(:label, project: project, name: 'Scoped2::Label2') }
-  let(:board)        { create(:board, project: project) }
-  let!(:list)        { create(:list, board: board, label: development, position: 0) }
+  let_it_be(:user)         { create(:user) }
+  let_it_be(:user2)        { create(:user) }
+
+  let_it_be(:group)        { create(:group) }
+  let_it_be(:project)      { create(:project, :public, group: group) }
+
+  let_it_be(:milestone)    { create(:milestone, project: project) }
+  let_it_be(:development)  { create(:label, project: project, name: 'Development') }
+  let_it_be(:stretch)      { create(:label, project: project, name: 'Stretch') }
+  let_it_be(:scoped_label_1) { create(:label, project: project, name: 'Scoped1::Label1') }
+  let_it_be(:scoped_label_2) { create(:label, project: project, name: 'Scoped2::Label2') }
+
+  let_it_be(:issue1)       { create(:labeled_issue, project: project, assignees: [user], milestone: milestone, labels: [development], weight: 3, relative_position: 2) }
+  let_it_be(:issue2)       { create(:labeled_issue, project: project, labels: [development, stretch], relative_position: 1) }
+  let_it_be(:epic1) { create(:epic, group: group, title: 'Foo') }
+  let_it_be(:epic2) { create(:epic, group: group, title: 'Bar') }
+  let_it_be(:epic_issue) { create(:epic_issue, issue: issue2, epic: epic1) }
+
+  let_it_be(:board)        { create(:board, project: project) }
+  let_it_be(:list)         { create(:list, board: board, label: development, position: 0) }
+
   let(:card1) { find('.board:nth-child(2)').find('.board-card:nth-child(2)') }
   let(:card2) { find('.board:nth-child(2)').find('.board-card:nth-child(1)') }
-  let(:epic1) { create(:epic, group: group, title: 'Foo') }
-  let!(:epic2) { create(:epic, group: group, title: 'Bar') }
-  let!(:epic_issue) { create(:epic_issue, issue: issue2, epic: epic1) }
 
   before do
     stub_licensed_features(multiple_issue_assignees: true)
@@ -32,15 +37,16 @@ RSpec.describe 'Issue Boards', :js do
 
     sign_in user
 
-    visit project_board_path(project, board)
-    wait_for_requests
+    visit_project_board
   end
 
   context 'assignee' do
+    let(:assignee_widget) { '[data-testid="issue-boards-sidebar"] [data-testid="assignees-widget"]' }
+
     it 'updates the issues assignee' do
       click_card(card2)
 
-      page.within('.assignee') do
+      page.within(assignee_widget) do
         click_button('Edit')
         wait_for_requests
 
@@ -62,7 +68,7 @@ RSpec.describe 'Issue Boards', :js do
     it 'adds multiple assignees' do
       click_card(card1)
 
-      page.within('.assignee') do
+      page.within(assignee_widget) do
         click_button('Edit')
         wait_for_requests
 
@@ -78,8 +84,10 @@ RSpec.describe 'Issue Boards', :js do
         click_button('Apply')
         wait_for_requests
 
-        expect(page).to have_link(nil, title: user.name)
-        expect(page).to have_link(nil, title: assignee)
+        aggregate_failures do
+          expect(page).to have_link(nil, title: user.name)
+          expect(page).to have_link(nil, title: assignee)
+        end
       end
 
       expect(card1.all('.avatar').length).to eq(2)
@@ -88,7 +96,7 @@ RSpec.describe 'Issue Boards', :js do
     it 'removes the assignee' do
       click_card(card1)
 
-      page.within('.assignee') do
+      page.within(assignee_widget) do
         click_button('Edit')
         wait_for_requests
 
@@ -108,9 +116,7 @@ RSpec.describe 'Issue Boards', :js do
     it 'assignees to current user' do
       click_card(card2)
 
-      wait_for_requests
-
-      page.within(find('.assignee')) do
+      page.within(assignee_widget) do
         expect(page).to have_content('None')
 
         click_button 'assign yourself'
@@ -126,7 +132,7 @@ RSpec.describe 'Issue Boards', :js do
     it 'updates assignee dropdown' do
       click_card(card2)
 
-      page.within('.assignee') do
+      page.within(assignee_widget) do
         click_button('Edit')
         wait_for_requests
 
@@ -142,11 +148,9 @@ RSpec.describe 'Issue Boards', :js do
         expect(page).to have_content(assignee)
       end
 
-      page.within(find('.board:nth-child(2)')) do
-        find('.board-card:nth-child(2)').click
-      end
+      click_card(card1)
 
-      page.within('.assignee') do
+      page.within(assignee_widget) do
         click_button('Edit')
 
         expect(find('.dropdown-menu')).to have_selector('.gl-new-dropdown-item-check-icon')
@@ -155,119 +159,100 @@ RSpec.describe 'Issue Boards', :js do
   end
 
   context 'epic' do
+    let(:epic_widget) { find('[data-testid="sidebar-epic"]') }
+
     before do
       stub_licensed_features(epics: true)
       group.add_owner(user)
 
-      visit project_board_path(project, board)
-      wait_for_requests
+      visit_project_board
     end
 
     context 'when the issue is not associated with an epic' do
       it 'displays `None` for value of epic' do
         click_card(card1)
-        wait_for_requests
 
-        expect(find('[data-testid="sidebar-epic"]').text).to have_content('None')
+        expect(epic_widget.text).to have_content('None')
       end
     end
 
     context 'when the issue is associated with an epic' do
       it 'displays name of epic and links to it' do
         click_card(card2)
-        wait_for_requests
 
-        expect(find('[data-testid="sidebar-epic"]')).to have_link(epic1.title, href: epic_path(epic1))
+        expect(epic_widget).to have_link(epic1.title, href: epic_path(epic1))
       end
 
       it 'updates the epic associated with the issue' do
         click_card(card2)
-        wait_for_requests
 
-        page.within(find('[data-testid="sidebar-epic"]')) do
+        within(epic_widget) do
           click_button 'Edit'
           wait_for_requests
 
           find('.gl-new-dropdown-item', text: epic2.title).click
           wait_for_requests
 
-          expect(page.find('.value')).to have_content(epic2.title)
+          expect(page).to have_content(epic2.title)
         end
       end
     end
   end
 
   context 'weight' do
+    let(:weight_widget) { find('[data-testid="sidebar-weight"]') }
+    let(:weight_value) { find('[data-testid="sidebar-weight"] .value') }
+
     it 'displays weight async' do
       click_card(card1)
-      wait_for_requests
 
-      expect(find('[data-testid="sidebar-weight"]').text).to have_content(issue1.weight)
+      expect(weight_value).to have_content(issue1.weight)
     end
 
     it 'updates weight in sidebar to 1' do
       click_card(card1)
-      wait_for_requests
 
-      page.within '[data-testid="sidebar-weight"]' do
+      within weight_widget do
         click_button 'Edit'
         find('.weight input').send_keys 1, :enter
-
-        page.within '.value' do
-          expect(page).to have_content '1'
-        end
       end
 
+      expect(weight_value).to have_content '1'
+
       # Ensure the request was sent and things are persisted
-      visit project_board_path(project, board)
-      wait_for_requests
+      visit_project_board
 
       click_card(card1)
-      wait_for_requests
 
-      page.within '[data-testid="sidebar-weight"]' do
-        page.within '.value' do
-          expect(page).to have_content '1'
-        end
-      end
+      expect(weight_value).to have_content '1'
     end
 
     it 'updates weight in sidebar to no weight' do
       click_card(card1)
-      wait_for_requests
 
-      page.within '[data-testid="sidebar-weight"]' do
+      within weight_widget do
         click_button 'remove weight'
-
-        page.within '.value' do
-          expect(page).to have_content 'None'
-        end
       end
 
+      expect(weight_value).to have_content 'None'
+
       # Ensure the request was sent and things are persisted
-      visit project_board_path(project, board)
-      wait_for_requests
+      visit_project_board
 
       click_card(card1)
-      wait_for_requests
 
-      page.within '.weight' do
-        page.within '.value' do
-          expect(page).to have_content 'None'
-        end
-      end
+      expect(weight_value).to have_content 'None'
     end
 
     context 'unlicensed' do
       before do
         stub_licensed_features(issue_weights: false)
-        visit project_board_path(project, board)
-        wait_for_requests
+
+        visit_project_board
       end
 
       it 'hides weight' do
         click_card(card1)
-        wait_for_requests
 
         expect(page).not_to have_selector('[data-testid="sidebar-weight"]')
       end
@@ -278,8 +263,7 @@ RSpec.describe 'Issue Boards', :js do
     before do
       stub_licensed_features(scoped_labels: true)
 
-      visit project_board_path(project, board)
-      wait_for_requests
+      visit_project_board
     end
 
     it 'adds multiple scoped labels' do
@@ -301,25 +285,25 @@ RSpec.describe 'Issue Boards', :js do
         find('[data-testid="close-icon"]').click
 
         page.within('.value') do
-          expect(page).to have_selector('.gl-label-scoped', count: 2)
-          expect(page).to have_content(scoped_label_1.scoped_label_key)
-          expect(page).to have_content(scoped_label_1.scoped_label_value)
-          expect(page).to have_content(scoped_label_2.scoped_label_key)
-          expect(page).to have_content(scoped_label_2.scoped_label_value)
+          aggregate_failures do
+            expect(page).to have_selector('.gl-label-scoped', count: 2)
+            expect(page).to have_content(scoped_label_1.scoped_label_key)
+            expect(page).to have_content(scoped_label_1.scoped_label_value)
+            expect(page).to have_content(scoped_label_2.scoped_label_key)
+            expect(page).to have_content(scoped_label_2.scoped_label_value)
+          end
         end
       end
     end
 
     context 'with scoped label assigned' do
       let!(:issue3) { create(:labeled_issue, project: project, labels: [development, scoped_label_1, scoped_label_2], relative_position: 3) }
-      let(:board) { create(:board, project: project) }
       let(:card3) { find('.board:nth-child(2)').find('.board-card:nth-child(1)') }
 
       before do
         stub_licensed_features(scoped_labels: true)
 
-        visit project_board_path(project, board)
-        wait_for_requests
+        visit_project_board
       end
 
       it 'removes existing scoped label' do
@@ -337,18 +321,22 @@ RSpec.describe 'Issue Boards', :js do
           find('[data-testid="close-icon"]').click
 
           page.within('.value') do
-            expect(page).to have_selector('.gl-label-scoped', count: 1)
-            expect(page).not_to have_content(scoped_label_1.scoped_label_value)
-            expect(page).to have_content(scoped_label_2.scoped_label_key)
-            expect(page).to have_content(scoped_label_2.scoped_label_value)
+            aggregate_failures do
+              expect(page).to have_selector('.gl-label-scoped', count: 1)
+              expect(page).not_to have_content(scoped_label_1.scoped_label_value)
+              expect(page).to have_content(scoped_label_2.scoped_label_key)
+              expect(page).to have_content(scoped_label_2.scoped_label_value)
+            end
           end
         end
 
-        expect(card3).to have_selector('.gl-label-scoped', count: 1)
-        expect(card3).not_to have_content(scoped_label_1.scoped_label_key)
-        expect(card3).not_to have_content(scoped_label_1.scoped_label_value)
-        expect(card3).to have_content(scoped_label_2.scoped_label_key)
-        expect(card3).to have_content(scoped_label_2.scoped_label_value)
+        aggregate_failures do
+          expect(card3).to have_selector('.gl-label-scoped', count: 1)
+          expect(card3).not_to have_content(scoped_label_1.scoped_label_key)
+          expect(card3).not_to have_content(scoped_label_1.scoped_label_value)
+          expect(card3).to have_content(scoped_label_2.scoped_label_key)
+          expect(card3).to have_content(scoped_label_2.scoped_label_value)
+        end
       end
     end
   end
@@ -378,4 +366,9 @@ RSpec.describe 'Issue Boards', :js do
       expect(page).not_to have_selector('.js-board-settings-sidebar')
     end
   end
+
+  def visit_project_board
+    visit project_board_path(project, board)
+    wait_for_requests
+  end
 end

spec/support/helpers/board_helpers.rb

@@ -4,6 +4,7 @@ module BoardHelpers
   def click_card(card)
     within card do
       first('.board-card-number').click
+      wait_for_requests
     end
   end
 end