Merge branch 'mrincon-remove-createAction-flash-tests' into 'master'

Remove createAction from createFlash's interface and add tests

See merge request gitlab-org/gitlab!75899

app/assets/javascripts/flash.js

@@ -62,7 +62,7 @@ const createFlashEl = (message, type) => `
   </div>
 `;
 
-const removeFlashClickListener = (flashEl, fadeTransition) => {
+const addDismissFlashClickListener = (flashEl, fadeTransition) => {
   // There are some flash elements which do not have a closeEl.
   // https://gitlab.com/gitlab-org/gitlab/blob/763426ef344488972eb63ea5be8744e0f8459e6b/ee/app/views/layouts/header/_read_only_banner.html.haml
   getCloseEl(flashEl)?.addEventListener('click', () => hideFlash(flashEl, fadeTransition));
@@ -113,7 +113,7 @@ const createFlash = function createFlash({
     }
   }
 
-  removeFlashClickListener(flashEl, fadeTransition);
+  addDismissFlashClickListener(flashEl, fadeTransition);
 
   flashContainer.classList.add('gl-display-block');
 
@@ -130,9 +130,8 @@ const createFlash = function createFlash({
 
 export {
   createFlash as default,
-  createAction,
   hideFlash,
-  removeFlashClickListener,
+  addDismissFlashClickListener,
   FLASH_TYPES,
   FLASH_CLOSED_EVENT,
 };

app/assets/javascripts/main.js

@@ -16,7 +16,7 @@ import * as popovers from '~/popovers';
 import * as tooltips from '~/tooltips';
 import { initHeaderSearchApp } from '~/header_search';
 import initAlertHandler from './alert_handler';
-import { removeFlashClickListener } from './flash';
+import { addDismissFlashClickListener } from './flash';
 import initTodoToggle from './header';
 import initLayoutNav from './layout_nav';
 import { logHelloDeferred } from './lib/logger/hello_deferred';
@@ -259,7 +259,7 @@ if (flashContainer && flashContainer.children.length) {
   flashContainer
     .querySelectorAll('.flash-alert, .flash-notice, .flash-success')
     .forEach((flashEl) => {
-      removeFlashClickListener(flashEl);
+      addDismissFlashClickListener(flashEl);
     });
 }
 

spec/frontend/flash_spec.js

+import * as Sentry from '@sentry/browser';
 import createFlash, {
-  createAction,
   hideFlash,
-  removeFlashClickListener,
+  addDismissFlashClickListener,
   FLASH_TYPES,
   FLASH_CLOSED_EVENT,
 } from '~/flash';
 
+jest.mock('@sentry/browser');
+
 describe('Flash', () => {
   describe('hideFlash', () => {
     let el;
@@ -66,49 +68,6 @@ describe('Flash', () => {
     });
   });
 
-  describe('createAction', () => {
-    let el;
-
-    beforeEach(() => {
-      el = document.createElement('div');
-    });
-
-    it('creates link with href', () => {
-      el.innerHTML = createAction({
-        href: 'testing',
-        title: 'test',
-      });
-
-      expect(el.querySelector('.flash-action').href).toContain('testing');
-    });
-
-    it('uses hash as href when no href is present', () => {
-      el.innerHTML = createAction({
-        title: 'test',
-      });
-
-      expect(el.querySelector('.flash-action').href).toContain('#');
-    });
-
-    it('adds role when no href is present', () => {
-      el.innerHTML = createAction({
-        title: 'test',
-      });
-
-      expect(el.querySelector('.flash-action').getAttribute('role')).toBe('button');
-    });
-
-    it('escapes the title text', () => {
-      el.innerHTML = createAction({
-        title: '<script>alert("a")</script>',
-      });
-
-      expect(el.querySelector('.flash-action').textContent.trim()).toBe(
-        '<script>alert("a")</script>',
-      );
-    });
-  });
-
   describe('createFlash', () => {
     const message = 'test';
     const fadeTransition = false;
@@ -194,7 +153,26 @@ describe('Flash', () => {
         expect(document.body.className).not.toContain('flash-shown');
       });
 
+      it('does not capture error using Sentry', () => {
+        createFlash({ ...defaultParams, captureError: false, error: new Error('Error!') });
+
+        expect(Sentry.captureException).not.toHaveBeenCalled();
+      });
+
+      it('captures error using Sentry', () => {
+        createFlash({ ...defaultParams, captureError: true, error: new Error('Error!') });
+
+        expect(Sentry.captureException).toHaveBeenCalledWith(expect.any(Error));
+        expect(Sentry.captureException).toHaveBeenCalledWith(
+          expect.objectContaining({
+            message: 'Error!',
+          }),
+        );
+      });
+
       describe('with actionConfig', () => {
+        const findFlashAction = () => document.querySelector('.flash-container .flash-action');
+
         it('adds action link', () => {
           createFlash({
             ...defaultParams,
@@ -203,20 +181,69 @@ describe('Flash', () => {
             },
           });
 
-          expect(document.querySelector('.flash-action')).not.toBeNull();
+          expect(findFlashAction()).not.toBeNull();
+        });
+
+        it('creates link with href', () => {
+          createFlash({
+            ...defaultParams,
+            actionConfig: {
+              href: 'testing',
+              title: 'test',
+            },
+          });
+
+          expect(findFlashAction().href).toBe(`${window.location}testing`);
+          expect(findFlashAction().textContent.trim()).toBe('test');
+        });
+
+        it('uses hash as href when no href is present', () => {
+          createFlash({
+            ...defaultParams,
+            actionConfig: {
+              title: 'test',
+            },
+          });
+
+          expect(findFlashAction().href).toBe(`${window.location}#`);
+        });
+
+        it('adds role when no href is present', () => {
+          createFlash({
+            ...defaultParams,
+            actionConfig: {
+              title: 'test',
+            },
+          });
+
+          expect(findFlashAction().getAttribute('role')).toBe('button');
+        });
+
+        it('escapes the title text', () => {
+          createFlash({
+            ...defaultParams,
+            actionConfig: {
+              title: '<script>alert("a")</script>',
+            },
+          });
+
+          expect(findFlashAction().textContent.trim()).toBe('<script>alert("a")</script>');
         });
 
         it('calls actionConfig clickHandler on click', () => {
-          const actionConfig = {
-            title: 'test',
-            clickHandler: jest.fn(),
-          };
+          const clickHandler = jest.fn();
 
-          createFlash({ ...defaultParams, actionConfig });
+          createFlash({
+            ...defaultParams,
+            actionConfig: {
+              title: 'test',
+              clickHandler,
+            },
+          });
 
-          document.querySelector('.flash-action').click();
+          findFlashAction().click();
 
-          expect(actionConfig.clickHandler).toHaveBeenCalled();
+          expect(clickHandler).toHaveBeenCalled();
         });
       });
 
@@ -236,7 +263,7 @@ describe('Flash', () => {
     });
   });
 
-  describe('removeFlashClickListener', () => {
+  describe('addDismissFlashClickListener', () => {
     let el;
 
     describe('with close icon', () => {
@@ -252,7 +279,7 @@ describe('Flash', () => {
       });
 
       it('removes global flash on click', (done) => {
-        removeFlashClickListener(el, false);
+        addDismissFlashClickListener(el, false);
 
         el.querySelector('.js-close-icon').click();
 
@@ -276,7 +303,7 @@ describe('Flash', () => {
       });
 
       it('does not throw', () => {
-        expect(() => removeFlashClickListener(el, false)).not.toThrow();
+        expect(() => addDismissFlashClickListener(el, false)).not.toThrow();
       });
     });
   });