Small improvements thanks to Robert's feedback

Signed-off-by: Rémy Coutable <remy@rymai.me>

6 files changed, 71 insertions, 106 deletions

diff --git a/spec/features/dashboard_issues_spec.rb b/spec/features/dashboard_issues_spec.rb
index 4dccf645454..9b54b5301e5 100644
--- a/spec/features/dashboard_issues_spec.rb
+++ b/spec/features/dashboard_issues_spec.rb
@@ -21,11 +21,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do
 
       click_link 'No Milestone'
 
-      page.within '.issues-state-filters' do
-        expect(page).to have_content('Open 1')
-        expect(page).to have_content('Closed 0')
-        expect(page).to have_content('All 1')
-      end
+      expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
       expect(page).to have_selector('.issue', count: 1)
     end
 
@@ -34,11 +30,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do
 
       click_link 'Any Milestone'
 
-      page.within '.issues-state-filters' do
-        expect(page).to have_content('Open 2')
-        expect(page).to have_content('Closed 0')
-        expect(page).to have_content('All 2')
-      end
+      expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2)
       expect(page).to have_selector('.issue', count: 2)
     end
 
@@ -49,11 +41,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do
         click_link milestone.title
       end
 
-      page.within '.issues-state-filters' do
-        expect(page).to have_content('Open 1')
-        expect(page).to have_content('Closed 0')
-        expect(page).to have_content('All 1')
-      end
+      expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
       expect(page).to have_selector('.issue', count: 1)
     end
   end
diff --git a/spec/features/issues/filter_by_labels_spec.rb b/spec/features/issues/filter_by_labels_spec.rb
index a615174c00e..0253629f753 100644
--- a/spec/features/issues/filter_by_labels_spec.rb
+++ b/spec/features/issues/filter_by_labels_spec.rb
@@ -83,11 +83,7 @@ feature 'Issue filtering by Labels', feature: true, js: true do
     end
 
     it 'applies the filters' do
-      page.within '.issues-state-filters' do
-        expect(page).to have_content('Open 1')
-        expect(page).to have_content('Closed 0')
-        expect(page).to have_content('All 1')
-      end
+      expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
       expect(page).to have_content "Bugfix2"
       expect(page).not_to have_content "Feature1"
       expect(find('.filtered-labels')).to have_content "bug"
diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb
index 8fd67acff69..8d19198efd3 100644
--- a/spec/features/issues/filter_issues_spec.rb
+++ b/spec/features/issues/filter_issues_spec.rb
@@ -227,11 +227,7 @@ describe 'Filter issues', feature: true do
       it 'filters by text and label' do
         fill_in 'issuable_search', with: 'Bug'
 
-        page.within '.issues-state-filters' do
-          expect(page).to have_content('Open 2')
-          expect(page).to have_content('Closed 0')
-          expect(page).to have_content('All 2')
-        end
+        expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2)
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 2)
         end
@@ -242,11 +238,7 @@ describe 'Filter issues', feature: true do
         end
         find('.dropdown-menu-close-icon').click
 
-        page.within '.issues-state-filters' do
-          expect(page).to have_content('Open 1')
-          expect(page).to have_content('Closed 0')
-          expect(page).to have_content('All 1')
-        end
+        expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 1)
         end
@@ -255,11 +247,7 @@ describe 'Filter issues', feature: true do
       it 'filters by text and milestone' do
         fill_in 'issuable_search', with: 'Bug'
 
-        page.within '.issues-state-filters' do
-          expect(page).to have_content('Open 2')
-          expect(page).to have_content('Closed 0')
-          expect(page).to have_content('All 2')
-        end
+        expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2)
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 2)
         end
@@ -269,11 +257,7 @@ describe 'Filter issues', feature: true do
           click_link '8'
         end
 
-        page.within '.issues-state-filters' do
-          expect(page).to have_content('Open 1')
-          expect(page).to have_content('Closed 0')
-          expect(page).to have_content('All 1')
-        end
+        expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 1)
         end
@@ -282,11 +266,7 @@ describe 'Filter issues', feature: true do
       it 'filters by text and assignee' do
         fill_in 'issuable_search', with: 'Bug'
 
-        page.within '.issues-state-filters' do
-          expect(page).to have_content('Open 2')
-          expect(page).to have_content('Closed 0')
-          expect(page).to have_content('All 2')
-        end
+        expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2)
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 2)
         end
@@ -296,11 +276,7 @@ describe 'Filter issues', feature: true do
           click_link user.name
         end
 
-        page.within '.issues-state-filters' do
-          expect(page).to have_content('Open 1')
-          expect(page).to have_content('Closed 0')
-          expect(page).to have_content('All 1')
-        end
+        expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 1)
         end
@@ -309,11 +285,7 @@ describe 'Filter issues', feature: true do
       it 'filters by text and author' do
         fill_in 'issuable_search', with: 'Bug'
 
-        page.within '.issues-state-filters' do
-          expect(page).to have_content('Open 2')
-          expect(page).to have_content('Closed 0')
-          expect(page).to have_content('All 2')
-        end
+        expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2)
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 2)
         end
@@ -323,11 +295,7 @@ describe 'Filter issues', feature: true do
           click_link user.name
         end
 
-        page.within '.issues-state-filters' do
-          expect(page).to have_content('Open 1')
-          expect(page).to have_content('Closed 0')
-          expect(page).to have_content('All 1')
-        end
+        expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 1)
         end
@@ -356,11 +324,7 @@ describe 'Filter issues', feature: true do
       find('.dropdown-menu-close-icon').click
       wait_for_ajax
 
-      page.within '.issues-state-filters' do
-        expect(page).to have_content('Open 2')
-        expect(page).to have_content('Closed 0')
-        expect(page).to have_content('All 2')
-      end
+      expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2)
       page.within '.issues-list' do
         expect(page).to have_selector('.issue', count: 2)
       end
diff --git a/spec/features/merge_requests/filter_by_milestone_spec.rb b/spec/features/merge_requests/filter_by_milestone_spec.rb
index 3d5d9614fb6..d917d5950ec 100644
--- a/spec/features/merge_requests/filter_by_milestone_spec.rb
+++ b/spec/features/merge_requests/filter_by_milestone_spec.rb
@@ -17,11 +17,7 @@ feature 'Merge Request filtering by Milestone', feature: true do
     visit_merge_requests(project)
     filter_by_milestone(Milestone::None.title)
 
-    page.within '.issues-state-filters' do
-      expect(page).to have_content('Open 1')
-      expect(page).to have_content('Closed 0')
-      expect(page).to have_content('All 1')
-    end
+    expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
     expect(page).to have_css('.merge-request', count: 1)
   end
 
@@ -44,11 +40,7 @@ feature 'Merge Request filtering by Milestone', feature: true do
       visit_merge_requests(project)
       filter_by_milestone(Milestone::Upcoming.title)
 
-      page.within '.issues-state-filters' do
-        expect(page).to have_content('Open 1')
-        expect(page).to have_content('Closed 0')
-        expect(page).to have_content('All 1')
-      end
+      expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
       expect(page).to have_css('.merge-request', count: 1)
     end
 
@@ -71,11 +63,7 @@ feature 'Merge Request filtering by Milestone', feature: true do
     visit_merge_requests(project)
     filter_by_milestone(milestone.title)
 
-    page.within '.issues-state-filters' do
-      expect(page).to have_content('Open 1')
-      expect(page).to have_content('Closed 0')
-      expect(page).to have_content('All 1')
-    end
+    expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1)
     expect(page).to have_css('.merge-request', count: 1)
   end
 
diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb
index 5effb7ea36c..62cc10f579a 100644
--- a/spec/helpers/issuables_helper_spec.rb
+++ b/spec/helpers/issuables_helper_spec.rb
@@ -46,18 +46,18 @@ describe IssuablesHelper do
     describe 'counter caching based on issuable type and params', :caching do
       let(:params) do
         {
-          'scope' => 'created-by-me',
-          'state' => 'opened',
-          'utf8' => '✓',
-          'author_id' => '11',
-          'assignee_id' => '18',
-          'label_name' => ['bug', 'discussion', 'documentation'],
-          'milestone_title' => 'v4.0',
-          'sort' => 'due_date_asc',
-          'namespace_id' => 'gitlab-org',
-          'project_id' => 'gitlab-ce',
-          'page' => 2
-        }
+          scope: 'created-by-me',
+          state: 'opened',
+          utf8: '✓',
+          author_id: '11',
+          assignee_id: '18',
+          label_name: ['bug', 'discussion', 'documentation'],
+          milestone_title: 'v4.0',
+          sort: 'due_date_asc',
+          namespace_id: 'gitlab-org',
+          project_id: 'gitlab-ce',
+          page: 2
+        }.with_indifferent_access
       end
 
       it 'returns the cached value when called for the same issuable type & with the same params' do
@@ -73,25 +73,33 @@ describe IssuablesHelper do
           to eq('<span>Open</span> <span class="badge">42</span>')
       end
 
-      describe 'keys not taken in account in the cache key' do
-        %w[state sort utf8 page].each do |param|
-          it "does not take in account params['#{param}'] in the cache key" do
-            expect(helper).to receive(:params).and_return('author_id' => '11', param => 'foo')
-            expect(helper).to receive(:issuables_count_for_state).with(:issues, :opened).and_return(42)
+      it 'does not take some keys into account in the cache key' do
+        expect(helper).to receive(:params).and_return({
+          author_id: '11',
+          state: 'foo',
+          sort: 'foo',
+          utf8: 'foo',
+          page: 'foo'
+        }.with_indifferent_access)
+        expect(helper).to receive(:issuables_count_for_state).with(:issues, :opened).and_return(42)
 
-            expect(helper.issuables_state_counter_text(:issues, :opened)).
-              to eq('<span>Open</span> <span class="badge">42</span>')
+        expect(helper.issuables_state_counter_text(:issues, :opened)).
+          to eq('<span>Open</span> <span class="badge">42</span>')
 
-            expect(helper).to receive(:params).and_return('author_id' => '11', param => 'bar')
-            expect(helper).not_to receive(:issuables_count_for_state)
+        expect(helper).to receive(:params).and_return({
+          author_id: '11',
+          state: 'bar',
+          sort: 'bar',
+          utf8: 'bar',
+          page: 'bar'
+        }.with_indifferent_access)
+        expect(helper).not_to receive(:issuables_count_for_state)
 
-            expect(helper.issuables_state_counter_text(:issues, :opened)).
-              to eq('<span>Open</span> <span class="badge">42</span>')
-          end
-        end
+        expect(helper.issuables_state_counter_text(:issues, :opened)).
+          to eq('<span>Open</span> <span class="badge">42</span>')
       end
 
-      it 'does not take params order in acount in the cache key' do
+      it 'does not take params order into account in the cache key' do
         expect(helper).to receive(:params).and_return('author_id' => '11', 'state' => 'opened')
         expect(helper).to receive(:issuables_count_for_state).with(:issues, :opened).and_return(42)
 
diff --git a/spec/support/matchers/have_issuable_counts.rb b/spec/support/matchers/have_issuable_counts.rb
new file mode 100644
index 00000000000..02605d6b70e
--- /dev/null
+++ b/spec/support/matchers/have_issuable_counts.rb
@@ -0,0 +1,21 @@
+RSpec::Matchers.define :have_issuable_counts do |opts|
+  match do |actual|
+    expected_counts = opts.map do |state, count|
+      "#{state.to_s.humanize} #{count}"
+    end
+
+    actual.within '.issues-state-filters' do
+      expected_counts.each do |expected_count|
+        expect(actual).to have_content(expected_count)
+      end
+    end
+  end
+
+  description do
+    "displays the following issuable counts: #{expected_counts.inspect}"
+  end
+
+  failure_message do
+    "expected the following issuable counts: #{expected_counts.inspect} to be displayed"
+  end
+end