Merge branch 'ps-fix-multiple-modals-on-note-cancel' into 'master'

Fix multiple modals showing when canceling note

See merge request gitlab-org/gitlab!81310

app/assets/javascripts/lib/utils/ignore_while_pending.js

+/**
+ * This will wrap the given function to make sure that it is only triggered once
+ * while executing asynchronously
+ *
+ * @param {Function} fn some function that returns a promise
+ * @returns A function that will only be triggered *once* while the promise is executing
+ */
+export const ignoreWhilePending = (fn) => {
+  const isPendingMap = new WeakMap();
+  const defaultContext = {};
+
+  // We need this to be a function so we get the `this`
+  return function ignoreWhilePendingInner(...args) {
+    const context = this || defaultContext;
+
+    if (isPendingMap.get(context)) {
+      return Promise.resolve();
+    }
+
+    isPendingMap.set(context, true);
+
+    return fn.apply(this, args).finally(() => {
+      isPendingMap.delete(context);
+    });
+  };
+};

app/assets/javascripts/notes/components/noteable_discussion.vue

@@ -6,6 +6,7 @@ import createFlash from '~/flash';
 import { clearDraft, getDiscussionReplyKey } from '~/lib/utils/autosave';
 import { isLoggedIn } from '~/lib/utils/common_utils';
 import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal';
+import { ignoreWhilePending } from '~/lib/utils/ignore_while_pending';
 import { s__, __ } from '~/locale';
 import diffLineNoteFormMixin from '~/notes/mixins/diff_line_note_form';
 import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
@@ -171,7 +172,7 @@ export default {
         this.expandDiscussion({ discussionId: this.discussion.id });
       }
     },
-    async cancelReplyForm(shouldConfirm, isDirty) {
+    cancelReplyForm: ignoreWhilePending(async function cancelReplyForm(shouldConfirm, isDirty) {
       if (shouldConfirm && isDirty) {
         const msg = s__('Notes|Are you sure you want to cancel creating this comment?');
 
@@ -188,7 +189,7 @@ export default {
 
       this.isReplying = false;
       clearDraft(this.autosaveKey);
-    },
+    }),
     saveReply(noteText, form, callback) {
       if (!noteText) {
         this.cancelReplyForm();

app/assets/javascripts/notes/components/noteable_note.vue

@@ -7,6 +7,7 @@ import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_m
 import { INLINE_DIFF_LINES_KEY } from '~/diffs/constants';
 import createFlash from '~/flash';
 import httpStatusCodes from '~/lib/utils/http_status';
+import { ignoreWhilePending } from '~/lib/utils/ignore_while_pending';
 import { truncateSha } from '~/lib/utils/text_utility';
 import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
 import { __, s__, sprintf } from '../../locale';
@@ -350,7 +351,10 @@ export default {
         parent: this.$el,
       });
     },
-    async formCancelHandler({ shouldConfirm, isDirty }) {
+    formCancelHandler: ignoreWhilePending(async function formCancelHandler({
+      shouldConfirm,
+      isDirty,
+    }) {
       if (shouldConfirm && isDirty) {
         const msg = __('Are you sure you want to cancel editing this comment?');
         const confirmed = await confirmAction(msg);
@@ -364,7 +368,7 @@ export default {
       }
       this.isEditing = false;
       this.$emit('cancelForm');
-    },
+    }),
     recoverNoteContent(noteText) {
       // we need to do this to prevent noteForm inconsistent content warning
       // this is something we intentionally do so we need to recover the content

spec/frontend/lib/utils/ignore_while_pending_spec.js

+import waitForPromises from 'helpers/wait_for_promises';
+import { ignoreWhilePending } from '~/lib/utils/ignore_while_pending';
+
+const TEST_ARGS = [123, { foo: 'bar' }];
+
+describe('~/lib/utils/ignore_while_pending', () => {
+  let spyResolve;
+  let spyReject;
+  let spy;
+  let subject;
+
+  beforeEach(() => {
+    spy = jest.fn().mockImplementation(
+      // NOTE: We can't pass an arrow function here...
+      function foo() {
+        return new Promise((resolve, reject) => {
+          spyResolve = resolve;
+          spyReject = reject;
+        });
+      },
+    );
+  });
+
+  describe('with non-instance method', () => {
+    beforeEach(() => {
+      subject = ignoreWhilePending(spy);
+    });
+
+    it('while pending, will ignore subsequent calls', () => {
+      subject(...TEST_ARGS);
+      subject();
+      subject();
+      subject();
+
+      expect(spy).toHaveBeenCalledTimes(1);
+      expect(spy).toHaveBeenCalledWith(...TEST_ARGS);
+    });
+
+    it.each`
+      desc               | act
+      ${'when resolved'} | ${() => spyResolve()}
+      ${'when rejected'} | ${() => spyReject(new Error('foo'))}
+    `('$desc, can be triggered again', async ({ act }) => {
+      // We need the empty catch(), since we are testing rejecting the promise,
+      // which would otherwise cause the test to fail.
+      subject(...TEST_ARGS).catch(() => {});
+      subject();
+      subject();
+      subject();
+
+      act();
+      // We need waitForPromises, so that the underlying finally() runs.
+      await waitForPromises();
+
+      subject({ again: 'foo' });
+
+      expect(spy).toHaveBeenCalledTimes(2);
+      expect(spy).toHaveBeenCalledWith(...TEST_ARGS);
+      expect(spy).toHaveBeenCalledWith({ again: 'foo' });
+    });
+
+    it('while pending, returns empty resolutions for ignored calls', async () => {
+      subject(...TEST_ARGS);
+
+      await expect(subject(...TEST_ARGS)).resolves.toBeUndefined();
+      await expect(subject(...TEST_ARGS)).resolves.toBeUndefined();
+    });
+
+    it('when resolved, returns resolution for origin call', async () => {
+      const resolveValue = { original: 1 };
+      const result = subject(...TEST_ARGS);
+
+      spyResolve(resolveValue);
+
+      await expect(result).resolves.toEqual(resolveValue);
+    });
+
+    it('when rejected, returns rejection for original call', async () => {
+      const rejectedErr = new Error('original');
+      const result = subject(...TEST_ARGS);
+
+      spyReject(rejectedErr);
+
+      await expect(result).rejects.toEqual(rejectedErr);
+    });
+  });
+
+  describe('with instance method', () => {
+    let instance1;
+    let instance2;
+
+    beforeEach(() => {
+      // Let's capture the "this" for tests
+      subject = ignoreWhilePending(function instanceMethod(...args) {
+        return spy(this, ...args);
+      });
+
+      instance1 = {};
+      instance2 = {};
+    });
+
+    it('will not ignore calls across instances', () => {
+      subject.call(instance1, { context: 1 });
+      subject.call(instance1, {});
+      subject.call(instance1, {});
+      subject.call(instance2, { context: 2 });
+      subject.call(instance2, {});
+
+      expect(spy.mock.calls).toEqual([
+        [instance1, { context: 1 }],
+        [instance2, { context: 2 }],
+      ]);
+    });
+
+    it('resolving one instance does not resolve other instances', async () => {
+      subject.call(instance1, { context: 1 });
+
+      // We need to save off spyResolve so it's not overwritten by next call
+      const instance1Resolve = spyResolve;
+
+      subject.call(instance2, { context: 2 });
+
+      instance1Resolve();
+      await waitForPromises();
+
+      subject.call(instance1, { context: 1 });
+      subject.call(instance2, { context: 2 });
+
+      expect(spy.mock.calls).toEqual([
+        [instance1, { context: 1 }],
+        [instance2, { context: 2 }],
+        [instance1, { context: 1 }],
+      ]);
+    });
+  });
+});