Merge branch '34841-todos' into 'master'

Fix bad type checking to prevent 0 count badge to be shown

Closes #34841

See merge request gitlab-org/gitlab-ce!14767

5 files changed, 69 insertions, 52 deletions

diff --git a/app/assets/javascripts/header.js b/app/assets/javascripts/header.js
index dc170c60456..ea2e2205077 100644
--- a/app/assets/javascripts/header.js
+++ b/app/assets/javascripts/header.js
@@ -1,7 +1,16 @@
-/* eslint-disable func-names, space-before-function-paren, prefer-arrow-callback, no-var */
+import { highCountTrim } from '~/lib/utils/text_utility';
 
-$(document).on('todo:toggle', function(e, count) {
-  var $todoPendingCount = $('.todos-count');
-  $todoPendingCount.text(gl.text.highCountTrim(count));
-  $todoPendingCount.toggleClass('hidden', count === 0);
+/**
+ * Updates todo counter when todos are toggled.
+ * When count is 0, we hide the badge.
+ *
+ * @param {jQuery.Event} e
+ * @param {String} count
+ */
+$(document).on('todo:toggle', (e, count) => {
+  const parsedCount = parseInt(count, 10);
+  const $todoPendingCount = $('.todos-count');
+
+  $todoPendingCount.text(highCountTrim(parsedCount));
+  $todoPendingCount.toggleClass('hidden', parsedCount === 0);
 });
diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js
index 021f936a4fa..f776829f69c 100644
--- a/app/assets/javascripts/lib/utils/text_utility.js
+++ b/app/assets/javascripts/lib/utils/text_utility.js
@@ -1,4 +1,4 @@
-/* eslint-disable func-names, space-before-function-paren, wrap-iife, no-var, no-param-reassign, no-cond-assign, quotes, one-var, one-var-declaration-per-line, operator-assignment, no-else-return, prefer-template, prefer-arrow-callback, no-empty, max-len, consistent-return, no-unused-vars, no-return-assign, max-len, vars-on-top */
+/* eslint-disable import/prefer-default-export, func-names, space-before-function-paren, wrap-iife, no-var, no-param-reassign, no-cond-assign, quotes, one-var, one-var-declaration-per-line, operator-assignment, no-else-return, prefer-template, prefer-arrow-callback, no-empty, max-len, consistent-return, no-unused-vars, no-return-assign, max-len, vars-on-top */
 
 import 'vendor/latinise';
 
@@ -13,9 +13,17 @@ if ((base = w.gl).text == null) {
 gl.text.addDelimiter = function(text) {
   return text ? text.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ",") : text;
 };
-gl.text.highCountTrim = function(count) {
+
+/**
+ * Returns '99+' for numbers bigger than 99.
+ *
+ * @param {Number} count
+ * @return {Number|String}
+ */
+export function highCountTrim(count) {
   return count > 99 ? '99+' : count;
-};
+}
+
 gl.text.randomString = function() {
   return Math.random().toString(36).substring(7);
 };
diff --git a/changelogs/unreleased/34841-todos.yml b/changelogs/unreleased/34841-todos.yml
new file mode 100644
index 00000000000..37180eefbfc
--- /dev/null
+++ b/changelogs/unreleased/34841-todos.yml
@@ -0,0 +1,5 @@
+---
+title: Fix bad type checking to prevent 0 count badge to be shown
+merge_request:
+author:
+type: fixed
diff --git a/spec/javascripts/header_spec.js b/spec/javascripts/header_spec.js
index 0e01934d3a3..4751eb868a4 100644
--- a/spec/javascripts/header_spec.js
+++ b/spec/javascripts/header_spec.js
@@ -1,53 +1,48 @@
-/* eslint-disable space-before-function-paren, no-var */
-
 import '~/header';
-import '~/lib/utils/text_utility';
 
-(function() {
-  describe('Header', function() {
-    var todosPendingCount = '.todos-count';
-    var fixtureTemplate = 'issues/open-issue.html.raw';
+describe('Header', function () {
+  const todosPendingCount = '.todos-count';
+  const fixtureTemplate = 'issues/open-issue.html.raw';
 
-    function isTodosCountHidden() {
-      return $(todosPendingCount).hasClass('hidden');
-    }
+  function isTodosCountHidden() {
+    return $(todosPendingCount).hasClass('hidden');
+  }
 
-    function triggerToggle(newCount) {
-      $(document).trigger('todo:toggle', newCount);
-    }
+  function triggerToggle(newCount) {
+    $(document).trigger('todo:toggle', newCount);
+  }
 
-    preloadFixtures(fixtureTemplate);
-    beforeEach(function() {
-      loadFixtures(fixtureTemplate);
-    });
+  preloadFixtures(fixtureTemplate);
+  beforeEach(() => {
+    loadFixtures(fixtureTemplate);
+  });
 
-    it('should update todos-count after receiving the todo:toggle event', function() {
-      triggerToggle(5);
-      expect($(todosPendingCount).text()).toEqual('5');
-    });
+  it('should update todos-count after receiving the todo:toggle event', () => {
+    triggerToggle('5');
+    expect($(todosPendingCount).text()).toEqual('5');
+  });
 
-    it('should hide todos-count when it is 0', function() {
-      triggerToggle(0);
-      expect(isTodosCountHidden()).toEqual(true);
+  it('should hide todos-count when it is 0', () => {
+    triggerToggle('0');
+    expect(isTodosCountHidden()).toEqual(true);
+  });
+
+  it('should show todos-count when it is more than 0', () => {
+    triggerToggle('10');
+    expect(isTodosCountHidden()).toEqual(false);
+  });
+
+  describe('when todos-count is 1000', () => {
+    beforeEach(() => {
+      triggerToggle('1000');
     });
 
-    it('should show todos-count when it is more than 0', function() {
-      triggerToggle(10);
+    it('should show todos-count', () => {
       expect(isTodosCountHidden()).toEqual(false);
     });
 
-    describe('when todos-count is 1000', function() {
-      beforeEach(function() {
-        triggerToggle(1000);
-      });
-
-      it('should show todos-count', function() {
-        expect(isTodosCountHidden()).toEqual(false);
-      });
-
-      it('should show 99+ for todos-count', function() {
-        expect($(todosPendingCount).text()).toEqual('99+');
-      });
+    it('should show 99+ for todos-count', () => {
+      expect($(todosPendingCount).text()).toEqual('99+');
     });
   });
-}).call(window);
+});
diff --git a/spec/javascripts/lib/utils/text_utility_spec.js b/spec/javascripts/lib/utils/text_utility_spec.js
index f1a975ba962..829b3ef5735 100644
--- a/spec/javascripts/lib/utils/text_utility_spec.js
+++ b/spec/javascripts/lib/utils/text_utility_spec.js
@@ -1,4 +1,4 @@
-import '~/lib/utils/text_utility';
+import { highCountTrim } from '~/lib/utils/text_utility';
 
 describe('text_utility', () => {
   describe('gl.text.getTextWidth', () => {
@@ -35,14 +35,14 @@ describe('text_utility', () => {
     });
   });
 
-  describe('gl.text.highCountTrim', () => {
+  describe('highCountTrim', () => {
     it('returns 99+ for count >= 100', () => {
-      expect(gl.text.highCountTrim(105)).toBe('99+');
-      expect(gl.text.highCountTrim(100)).toBe('99+');
+      expect(highCountTrim(105)).toBe('99+');
+      expect(highCountTrim(100)).toBe('99+');
     });
 
     it('returns exact number for count < 100', () => {
-      expect(gl.text.highCountTrim(45)).toBe(45);
+      expect(highCountTrim(45)).toBe(45);
     });
   });