From 8c3e6987d931847b72752dfcac4215dbdc47fd88 Mon Sep 17 00:00:00 2001 From: Lucas Deschamps Date: Wed, 9 Nov 2016 07:59:51 +0100 Subject: Navigation bar issuables counters reflects dashboard issuables counters --- app/helpers/issuables_helper.rb | 11 ++++++++ app/views/layouts/nav/_dashboard.html.haml | 4 +-- .../fix_navigation_bar_issuables_counters.yml | 4 +++ spec/features/dashboard/issuables_counter_spec.rb | 33 ++++++++++++++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/fix_navigation_bar_issuables_counters.yml create mode 100644 spec/features/dashboard/issuables_counter_spec.rb diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 8127c3f3ee3..22311267860 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -141,8 +141,19 @@ module IssuablesHelper html.html_safe end + def cached_assigned_issuables_count(assignee, issuable_type, state) + cache_key = "#{assignee.id}_#{issuable_type}_#{state}" + Rails.cache.fetch(cache_key, expires_in: 2.minutes) do + assigned_issuables_count(assignee, issuable_type, state) + end + end + private + def assigned_issuables_count(assignee, issuable_type, state) + assignee.send("assigned_#{issuable_type}").send(state).count + end + def sidebar_gutter_collapsed? cookies[:collapsed_gutter] == 'true' end diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index a0356feef95..2a6d9cda379 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/app/views/layouts/nav/_dashboard.html.haml @@ -26,12 +26,12 @@ = link_to assigned_issues_dashboard_path, title: 'Issues', class: 'dashboard-shortcuts-issues' do %span Issues - %span.count= number_with_delimiter(current_user.assigned_issues.opened.count) + %span.count= number_with_delimiter(cached_assigned_issuables_count(current_user, :issues, :opened)) = nav_link(path: 'dashboard#merge_requests') do = link_to assigned_mrs_dashboard_path, title: 'Merge Requests', class: 'dashboard-shortcuts-merge_requests' do %span Merge Requests - %span.count= number_with_delimiter(current_user.assigned_merge_requests.opened.count) + %span.count= number_with_delimiter(cached_assigned_issuables_count(current_user, :merge_requests, :opened)) = nav_link(controller: 'dashboard/snippets') do = link_to dashboard_snippets_path, title: 'Snippets' do %span diff --git a/changelogs/unreleased/fix_navigation_bar_issuables_counters.yml b/changelogs/unreleased/fix_navigation_bar_issuables_counters.yml new file mode 100644 index 00000000000..c66f191a2d0 --- /dev/null +++ b/changelogs/unreleased/fix_navigation_bar_issuables_counters.yml @@ -0,0 +1,4 @@ +--- +title: Navigation bar issuables counters reflects dashboard issuables counters +merge_request: +author: Lucas Deschamps diff --git a/spec/features/dashboard/issuables_counter_spec.rb b/spec/features/dashboard/issuables_counter_spec.rb new file mode 100644 index 00000000000..699bc102790 --- /dev/null +++ b/spec/features/dashboard/issuables_counter_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe 'Navigation bar counter', feature: true, js: true, caching: true do + let(:user) { create(:user) } + let(:project) { create(:project, namespace: user.namespace) } + + before do + login_as(user) + visit issues_dashboard_path + end + + it 'reflects dashboard issues count' do + create(:issue, project: project, assignee: user) + visit issues_dashboard_path + + dashboard_count = find('li.active span.badge') + nav_count = find('.dashboard-shortcuts-issues span.count') + + expect(dashboard_count).to have_content('0') + expect(nav_count).to have_content('0') + end + + it 'reflects dashboard merge requests count' do + create(:merge_request, assignee: user) + visit merge_requests_dashboard_path + + dashboard_count = find('li.active span.badge') + nav_count = find('.dashboard-shortcuts-merge_requests span.count') + + expect(dashboard_count).to have_content('0') + expect(nav_count).to have_content('0') + end +end -- cgit v1.2.1 From 677af1dd30f9b44a7be9eaa68bddda38331b8bbb Mon Sep 17 00:00:00 2001 From: Lucas Deschamps Date: Wed, 9 Nov 2016 23:15:29 +0100 Subject: Improve changes after MR review. --- app/helpers/issuables_helper.rb | 4 ++-- changelogs/unreleased/fix_navigation_bar_issuables_counters.yml | 2 +- spec/features/dashboard/issuables_counter_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 22311267860..58d46daa51e 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -142,7 +142,7 @@ module IssuablesHelper end def cached_assigned_issuables_count(assignee, issuable_type, state) - cache_key = "#{assignee.id}_#{issuable_type}_#{state}" + cache_key = hexdigest(['assigned_issuables_count', assignee.id, issuable_type, state].join('-')) Rails.cache.fetch(cache_key, expires_in: 2.minutes) do assigned_issuables_count(assignee, issuable_type, state) end @@ -151,7 +151,7 @@ module IssuablesHelper private def assigned_issuables_count(assignee, issuable_type, state) - assignee.send("assigned_#{issuable_type}").send(state).count + assignee.public_send("assigned_#{issuable_type}").public_send(state).count end def sidebar_gutter_collapsed? diff --git a/changelogs/unreleased/fix_navigation_bar_issuables_counters.yml b/changelogs/unreleased/fix_navigation_bar_issuables_counters.yml index c66f191a2d0..0f7f8155f91 100644 --- a/changelogs/unreleased/fix_navigation_bar_issuables_counters.yml +++ b/changelogs/unreleased/fix_navigation_bar_issuables_counters.yml @@ -1,4 +1,4 @@ --- title: Navigation bar issuables counters reflects dashboard issuables counters -merge_request: +merge_request: 7368 author: Lucas Deschamps diff --git a/spec/features/dashboard/issuables_counter_spec.rb b/spec/features/dashboard/issuables_counter_spec.rb index 699bc102790..f134eee19eb 100644 --- a/spec/features/dashboard/issuables_counter_spec.rb +++ b/spec/features/dashboard/issuables_counter_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe 'Navigation bar counter', feature: true, js: true, caching: true do let(:user) { create(:user) } - let(:project) { create(:project, namespace: user.namespace) } + let(:project) { create(:empty_project, namespace: user.namespace) } before do login_as(user) -- cgit v1.2.1 From 7fa366107b30fa0ccac758ff9e6854f86e54b116 Mon Sep 17 00:00:00 2001 From: Lucas Deschamps Date: Thu, 10 Nov 2016 23:41:25 +0100 Subject: Improve issuables_counter_spec relevance. --- spec/features/dashboard/issuables_counter_spec.rb | 33 +++++++++++++++++------ 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/spec/features/dashboard/issuables_counter_spec.rb b/spec/features/dashboard/issuables_counter_spec.rb index f134eee19eb..1960552b411 100644 --- a/spec/features/dashboard/issuables_counter_spec.rb +++ b/spec/features/dashboard/issuables_counter_spec.rb @@ -1,33 +1,50 @@ require 'spec_helper' -describe 'Navigation bar counter', feature: true, js: true, caching: true do +describe 'Navigation bar counter', feature: true, js: true do let(:user) { create(:user) } let(:project) { create(:empty_project, namespace: user.namespace) } before do login_as(user) - visit issues_dashboard_path end it 'reflects dashboard issues count' do - create(:issue, project: project, assignee: user) + issue = create(:issue, project: project, assignee: user) + visit issues_dashboard_path + + dashboard_count = find('li.active span.badge') + nav_count = find('.dashboard-shortcuts-issues span.count') + + expect(nav_count).to have_content('1') + expect(dashboard_count).to have_content('1') + + issue.assignee = nil visit issues_dashboard_path dashboard_count = find('li.active span.badge') nav_count = find('.dashboard-shortcuts-issues span.count') - expect(dashboard_count).to have_content('0') - expect(nav_count).to have_content('0') + expect(nav_count).to have_content('1') + expect(dashboard_count).to have_content('1') end it 'reflects dashboard merge requests count' do - create(:merge_request, assignee: user) + merge_request = create(:merge_request, source_project: project, assignee: user) + visit merge_requests_dashboard_path + + dashboard_count = find('li.active span.badge') + nav_count = find('.dashboard-shortcuts-merge_requests span.count') + + expect(nav_count).to have_content('1') + expect(dashboard_count).to have_content('1') + + merge_request.assignee = nil visit merge_requests_dashboard_path dashboard_count = find('li.active span.badge') nav_count = find('.dashboard-shortcuts-merge_requests span.count') - expect(dashboard_count).to have_content('0') - expect(nav_count).to have_content('0') + expect(nav_count).to have_content('1') + expect(dashboard_count).to have_content('1') end end -- cgit v1.2.1 From 3d13096f7198ed1ec4b7c568977877452f842394 Mon Sep 17 00:00:00 2001 From: Lucas Deschamps Date: Tue, 15 Nov 2016 09:05:19 +0100 Subject: Refactor issuables_counter_spec. --- spec/features/dashboard/issuables_counter_spec.rb | 40 ++++++++++------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/spec/features/dashboard/issuables_counter_spec.rb b/spec/features/dashboard/issuables_counter_spec.rb index 1960552b411..41dcfe439c2 100644 --- a/spec/features/dashboard/issuables_counter_spec.rb +++ b/spec/features/dashboard/issuables_counter_spec.rb @@ -1,50 +1,44 @@ require 'spec_helper' -describe 'Navigation bar counter', feature: true, js: true do +describe 'Navigation bar counter', feature: true, js: true, caching: true do let(:user) { create(:user) } let(:project) { create(:empty_project, namespace: user.namespace) } + let(:issue) { create(:issue, project: project) } + let(:merge_request) { create(:merge_request, source_project: project) } before do + issue.update(assignee: user) + merge_request.update(assignee: user) login_as(user) end it 'reflects dashboard issues count' do - issue = create(:issue, project: project, assignee: user) visit issues_dashboard_path - dashboard_count = find('li.active span.badge') - nav_count = find('.dashboard-shortcuts-issues span.count') - - expect(nav_count).to have_content('1') - expect(dashboard_count).to have_content('1') + expect_counters('issues', '1') - issue.assignee = nil + issue.update(assignee: nil) visit issues_dashboard_path - dashboard_count = find('li.active span.badge') - nav_count = find('.dashboard-shortcuts-issues span.count') - - expect(nav_count).to have_content('1') - expect(dashboard_count).to have_content('1') + expect_counters('issues', '1') end it 'reflects dashboard merge requests count' do - merge_request = create(:merge_request, source_project: project, assignee: user) visit merge_requests_dashboard_path - dashboard_count = find('li.active span.badge') - nav_count = find('.dashboard-shortcuts-merge_requests span.count') - - expect(nav_count).to have_content('1') - expect(dashboard_count).to have_content('1') + expect_counters('merge_requests', '1') - merge_request.assignee = nil + merge_request.update(assignee: nil) visit merge_requests_dashboard_path + expect_counters('merge_requests', '1') + end + + def expect_counters(issuable_type, count) dashboard_count = find('li.active span.badge') - nav_count = find('.dashboard-shortcuts-merge_requests span.count') + nav_count = find(".dashboard-shortcuts-#{issuable_type} span.count") - expect(nav_count).to have_content('1') - expect(dashboard_count).to have_content('1') + expect(nav_count).to have_content(count) + expect(dashboard_count).to have_content(count) end end -- cgit v1.2.1