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

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');
+ * Updates todo counter when todos are toggled.
-  $todoPendingCount.text(gl.text.highCountTrim(count));
+ * When count is 0, we hide the badge.
-  $todoPendingCount.toggleClass('hidden', count === 0);
+ *
+ * @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);
 });

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);
 };

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

spec/javascripts/header_spec.js

@@ -1,12 +1,8 @@
-/* eslint-disable space-before-function-paren, no-var */
-
 import '~/header';
-import '~/lib/utils/text_utility';
 
-(function() {
 describe('Header', function () {
-    var todosPendingCount = '.todos-count';
+  const todosPendingCount = '.todos-count';
-    var fixtureTemplate = 'issues/open-issue.html.raw';
+  const fixtureTemplate = 'issues/open-issue.html.raw';
 
   function isTodosCountHidden() {
     return $(todosPendingCount).hasClass('hidden');
@@ -17,37 +13,36 @@ import '~/lib/utils/text_utility';
   }
 
   preloadFixtures(fixtureTemplate);
-    beforeEach(function() {
+  beforeEach(() => {
     loadFixtures(fixtureTemplate);
   });
 
-    it('should update todos-count after receiving the todo:toggle event', 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', function() {
+  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', function() {
+  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', function() {
+  describe('when todos-count is 1000', () => {
-      beforeEach(function() {
+    beforeEach(() => {
-        triggerToggle(1000);
+      triggerToggle('1000');
     });
 
-      it('should show todos-count', function() {
+    it('should show todos-count', () => {
       expect(isTodosCountHidden()).toEqual(false);
     });
 
-      it('should show 99+ for todos-count', function() {
+    it('should show 99+ for todos-count', () => {
       expect($(todosPendingCount).text()).toEqual('99+');
     });
   });
 });
-}).call(window);

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(highCountTrim(105)).toBe('99+');
-      expect(gl.text.highCountTrim(100)).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);
     });
   });