Merge branch '66123-over-a-thousand-todos-displays-count-of-one' into 'master'

Fixes #30967 - Over one thousand todo messages displaying a count of one in the UI.

Closes #30967

See merge request gitlab-org/gitlab!16844

app/assets/javascripts/header.js

@@ -15,11 +15,10 @@ import { parseBoolean } from '~/lib/utils/common_utils';
  */
 export default function initTodoToggle() {
   $(document).on('todo:toggle', (e, count) => {
-    const parsedCount = parseInt(count, 10);
     const $todoPendingCount = $('.todos-count');
 
-    $todoPendingCount.text(highCountTrim(parsedCount));
-    $todoPendingCount.toggleClass('hidden', parsedCount === 0);
+    $todoPendingCount.text(highCountTrim(count));
+    $todoPendingCount.toggleClass('hidden', count === 0);
   });
 }
 

app/assets/javascripts/pages/dashboard/todos/index/todos.js

@@ -4,6 +4,7 @@ import $ from 'jquery';
 import { visitUrl } from '~/lib/utils/url_utility';
 import UsersSelect from '~/users_select';
 import { isMetaClick } from '~/lib/utils/common_utils';
+import { addDelimiter } from '~/lib/utils/text_utility';
 import { __ } from '~/locale';
 import flash from '~/flash';
 import axios from '~/lib/utils/axios_utils';
@@ -145,8 +146,8 @@ export default class Todos {
 
   updateBadges(data) {
     $(document).trigger('todo:toggle', data.count);
-    document.querySelector('.todos-pending .badge').innerHTML = data.count;
-    document.querySelector('.todos-done .badge').innerHTML = data.done_count;
+    document.querySelector('.todos-pending .badge').innerHTML = addDelimiter(data.count);
+    document.querySelector('.todos-done .badge').innerHTML = addDelimiter(data.done_count);
   }
 
   goToTodoUrl(e) {

app/controllers/dashboard/todos_controller.rb

@@ -78,8 +78,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController
 
   def todos_counts
     {
-      count: number_with_delimiter(current_user.todos_pending_count),
-      done_count: number_with_delimiter(current_user.todos_done_count)
+      count: current_user.todos_pending_count,
+      done_count: current_user.todos_done_count
     }
   end
 

changelogs/unreleased/66123-over-a-thousand-todos-displays-count-of-one.yml

+---
+title: Fix for count in todo badge when user has over 1,000 todos. Will now correctly
+  display todo count after user marks some todos as done.
+merge_request: 16844
+author: Jesse Hall @jessehall3
+type: fixed

spec/controllers/dashboard/todos_controller_spec.rb

@@ -131,7 +131,7 @@ describe Dashboard::TodosController do
 
       expect(todo.reload).to be_pending
       expect(response).to have_gitlab_http_status(200)
-      expect(json_response).to eq({ "count" => "1", "done_count" => "0" })
+      expect(json_response).to eq({ "count" => 1, "done_count" => 0 })
     end
   end
 
@@ -145,7 +145,7 @@ describe Dashboard::TodosController do
         expect(todo.reload).to be_pending
       end
       expect(response).to have_gitlab_http_status(200)
-      expect(json_response).to eq({ 'count' => '2', 'done_count' => '0' })
+      expect(json_response).to eq({ 'count' => 2, 'done_count' => 0 })
     end
   end
 end

spec/javascripts/header_spec.js

@@ -20,26 +20,26 @@ describe('Header', function() {
   });
 
   it('should update todos-count after receiving the todo:toggle event', () => {
-    triggerToggle('5');
+    triggerToggle(5);
 
     expect($(todosPendingCount).text()).toEqual('5');
   });
 
   it('should hide todos-count when it is 0', () => {
-    triggerToggle('0');
+    triggerToggle(0);
 
     expect(isTodosCountHidden()).toEqual(true);
   });
 
   it('should show todos-count when it is more than 0', () => {
-    triggerToggle('10');
+    triggerToggle(10);
 
     expect(isTodosCountHidden()).toEqual(false);
   });
 
   describe('when todos-count is 1000', () => {
     beforeEach(() => {
-      triggerToggle('1000');
+      triggerToggle(1000);
     });
 
     it('should show todos-count', () => {

spec/javascripts/todos_spec.js

 import $ from 'jquery';
+import MockAdapter from 'axios-mock-adapter';
 import Todos from '~/pages/dashboard/todos/index/todos';
 import '~/lib/utils/common_utils';
+import '~/gl_dropdown';
+import axios from '~/lib/utils/axios_utils';
+import { addDelimiter } from '~/lib/utils/text_utility';
+
+const TEST_COUNT_BIG = 2000;
+const TEST_DONE_COUNT_BIG = 7300;
 
 describe('Todos', () => {
   preloadFixtures('todos/todos.html');
   let todoItem;
+  let mock;
 
   beforeEach(() => {
     loadFixtures('todos/todos.html');
     todoItem = document.querySelector('.todos-list .todo');
+    mock = new MockAdapter(axios);
 
     return new Todos();
   });
 
+  afterEach(() => {
+    mock.restore();
+  });
+
   describe('goToTodoUrl', () => {
     it('opens the todo url', done => {
       const todoLink = todoItem.dataset.url;
@@ -53,5 +66,43 @@ describe('Todos', () => {
         expect(windowOpenSpy).not.toHaveBeenCalled();
       });
     });
+
+    describe('on done todo click', () => {
+      let onToggleSpy;
+
+      beforeEach(done => {
+        const el = document.querySelector('.js-done-todo');
+        const path = el.dataset.href;
+
+        // Arrange
+        mock
+          .onDelete(path)
+          .replyOnce(200, { count: TEST_COUNT_BIG, done_count: TEST_DONE_COUNT_BIG });
+        onToggleSpy = jasmine.createSpy('onToggle');
+        $(document).on('todo:toggle', onToggleSpy);
+
+        // Act
+        el.click();
+
+        // Wait for axios and HTML to udpate
+        setImmediate(done);
+      });
+
+      it('dispatches todo:toggle', () => {
+        expect(onToggleSpy).toHaveBeenCalledWith(jasmine.anything(), TEST_COUNT_BIG);
+      });
+
+      it('updates pending text', () => {
+        expect(document.querySelector('.todos-pending .badge').innerHTML).toEqual(
+          addDelimiter(TEST_COUNT_BIG),
+        );
+      });
+
+      it('updates done text', () => {
+        expect(document.querySelector('.todos-done .badge').innerHTML).toEqual(
+          addDelimiter(TEST_DONE_COUNT_BIG),
+        );
+      });
+    });
   });
 });