From 2d39b3337425b69f775fa65592aefc297ccd0a26 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Mon, 22 Nov 2021 20:49:58 -0700
Subject: [PATCH 1/3] Update scroll position to synchronize the Jump To Next
 button

When you scroll a discussion halfway up the page, the
Jump To Next Unresolved Discussion button will go to the
discussion after that one.

When you scroll the page such that a discussion scrolls down
through the middle of the page and below it, the Jump button
considers that discussion the "next" in line to be seen, and
will scroll to it.

Changelog: added
---
 .../javascripts/diffs/components/app.vue      |   8 +
 .../javascripts/diffs/utils/discussions.js    |  91 +++++++++++
 .../notes/components/discussion_notes.vue     |  46 +++++-
 .../notes/components/notes_app.vue            |   8 +
 .../javascripts/notes/stores/getters.js       |  34 ++--
 .../diffs/components/diff_discussions_spec.js |   8 +
 spec/frontend/diffs/utils/discussions_spec.js | 151 ++++++++++++++++++
 .../notes/components/discussion_notes_spec.js |   8 +
 .../components/noteable_discussion_spec.js    |  16 ++
 .../notes/components/notes_app_spec.js        |   8 +
 spec/frontend/notes/stores/getters_spec.js    |   2 +-
 11 files changed, 368 insertions(+), 12 deletions(-)
 create mode 100644 app/assets/javascripts/diffs/utils/discussions.js
 create mode 100644 spec/frontend/diffs/utils/discussions_spec.js

diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 66d06a3a1b6096..ef26f160ad2792 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -44,6 +44,10 @@ import {
   TRACKING_MULTIPLE_FILES_MODE,
 } from '../constants';
 
+import {
+  discussionIntersectionObserver,
+  discussionIntersectionObserverHandlerFactory,
+} from '../utils/discussions';
 import diffsEventHub from '../event_hub';
 import { reviewStatuses } from '../utils/file_reviews';
 import { diffsApp } from '../utils/performance';
@@ -86,6 +90,10 @@ export default {
     ALERT_MERGE_CONFLICT,
     ALERT_COLLAPSED_FILES,
   },
+  provide: {
+    discussionObserver: discussionIntersectionObserver(),
+    discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+  },
   props: {
     endpoint: {
       type: String,
diff --git a/app/assets/javascripts/diffs/utils/discussions.js b/app/assets/javascripts/diffs/utils/discussions.js
new file mode 100644
index 00000000000000..ecb22db7e030ed
--- /dev/null
+++ b/app/assets/javascripts/diffs/utils/discussions.js
@@ -0,0 +1,91 @@
+import { create } from '~/lib/utils/intersection_observer';
+
+function normalize(processable) {
+  const { entry } = processable;
+  const offset = entry.rootBounds.bottom - entry.boundingClientRect.top;
+  const direction =
+    offset < 0 ? 'Up' : 'Down'; /* eslint-disable-line @gitlab/require-i18n-strings */
+
+  return {
+    ...processable,
+    entry: {
+      time: entry.time,
+      type: entry.isIntersecting ? 'intersection' : `scroll${direction}`,
+    },
+  };
+}
+
+function sort(
+  { discussionIds, entry: alpha, currentDiscussion: alphaDiscussion },
+  { entry: beta, currentDiscussion: betaDiscussion },
+) {
+  const diff = alpha.time - beta.time;
+  const bottomUpDiscussions = discussionIds.reverse();
+  let order = 0;
+
+  if (diff < 0) {
+    order = -1;
+  } else if (diff > 0) {
+    order = 1;
+  } else if (alpha.type === 'intersection' && beta.type === 'scrollUp') {
+    order = 2;
+  } else if (alpha.type === 'scrollUp' && beta.type === 'intersection') {
+    order = -2;
+  } else if (alpha.type === 'scrollUp' && beta.type === 'scrollUp') {
+    order =
+      bottomUpDiscussions.indexOf(alphaDiscussion.id) -
+      bottomUpDiscussions.indexOf(betaDiscussion.id);
+  }
+
+  return order;
+}
+
+function filter(entry) {
+  return entry.type !== 'scrollDown';
+}
+
+export function discussionIntersectionObserver() {
+  return create({
+    threshold: 0,
+    rootMargin: '0px 0px -50% 0px',
+  });
+}
+
+export function discussionIntersectionObserverHandlerFactory() {
+  let unprocessed = [];
+  let timer = null;
+
+  return (processable) => {
+    unprocessed.push(processable);
+
+    if (timer) {
+      clearTimeout(timer);
+    }
+
+    timer = setTimeout(() => {
+      unprocessed
+        .map(normalize)
+        .filter(filter)
+        .sort(sort)
+        .forEach((discussionObservationContainer) => {
+          const {
+            entry: { type },
+            currentDiscussion: { id },
+            isFirstUnresolved,
+            isDiffsPage,
+            functions: { setCurrentDiscussionId, getPreviousUnresolvedDiscussionId },
+          } = discussionObservationContainer;
+
+          if (type === 'intersection') {
+            setCurrentDiscussionId(id);
+          } else if (type === 'scrollUp') {
+            setCurrentDiscussionId(
+              isFirstUnresolved ? null : getPreviousUnresolvedDiscussionId(id, isDiffsPage),
+            );
+          }
+        });
+
+      unprocessed = [];
+    }, 0);
+  };
+}
diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue
index 6fcfa66ea49413..c596468b0e6c8a 100644
--- a/app/assets/javascripts/notes/components/discussion_notes.vue
+++ b/app/assets/javascripts/notes/components/discussion_notes.vue
@@ -17,6 +17,7 @@ export default {
     NoteEditedText,
     DiscussionNotesRepliesWrapper,
   },
+  inject: ['discussionObserver', 'discussionObserverHandler'],
   props: {
     discussion: {
       type: Object,
@@ -54,7 +55,12 @@ export default {
     },
   },
   computed: {
-    ...mapGetters(['userCanReply']),
+    ...mapGetters([
+      'userCanReply',
+      'previousUnresolvedDiscussionId',
+      'firstUnresolvedDiscussionId',
+      'unresolvedDiscussionsIdsOrdered',
+    ]),
     hasReplies() {
       return Boolean(this.replies.length);
     },
@@ -77,9 +83,44 @@ export default {
         url: this.discussion.discussion_path,
       };
     },
+    isFirstUnresolved() {
+      return this.firstUnresolvedDiscussionId() === this.discussion.id;
+    },
+    discussionIds() {
+      return this.unresolvedDiscussionsIdsOrdered(!this.isOverviewTab);
+    },
+  },
+  mounted() {
+    const { observer, id } = this.discussionObserver;
+
+    if (this.$refs.firstDiscussionNote) {
+      const note = this.$refs.firstDiscussionNote.$el;
+
+      observer.observe(note);
+
+      note.addEventListener(`${id}IntersectionUpdate`, (event) => {
+        const observerEntry = event.detail;
+
+        this.discussionObserverHandler({
+          entry: observerEntry,
+          isFirstUnresolved: this.isFirstUnresolved,
+          currentDiscussion: { ...this.discussion },
+          discussionIds: this.discussionIds,
+          isDiffsPage: !this.isOverviewTab,
+          functions: {
+            setCurrentDiscussionId: this.setCurrentDiscussionId,
+            getPreviousUnresolvedDiscussionId: this.previousUnresolvedDiscussionId,
+          },
+        });
+      });
+    }
   },
   methods: {
-    ...mapActions(['toggleDiscussion', 'setSelectedCommentPositionHover']),
+    ...mapActions([
+      'toggleDiscussion',
+      'setSelectedCommentPositionHover',
+      'setCurrentDiscussionId',
+    ]),
     componentName(note) {
       if (note.isPlaceholderNote) {
         if (note.placeholderType === SYSTEM_NOTE) {
@@ -124,6 +165,7 @@ export default {
       <template v-if="shouldGroupReplies">
         <component
           :is="componentName(firstNote)"
+          ref="firstDiscussionNote"
           :note="componentData(firstNote)"
           :line="line || diffLine"
           :discussion-file="discussion.diff_file"
diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue
index c4924cd41f5a05..39c0ecfa881eb0 100644
--- a/app/assets/javascripts/notes/components/notes_app.vue
+++ b/app/assets/javascripts/notes/components/notes_app.vue
@@ -8,6 +8,10 @@ import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item
 import OrderedLayout from '~/vue_shared/components/ordered_layout.vue';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import draftNote from '../../batch_comments/components/draft_note.vue';
+import {
+  discussionIntersectionObserver,
+  discussionIntersectionObserverHandlerFactory,
+} from '../../diffs/utils/discussions';
 import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility';
 import placeholderNote from '../../vue_shared/components/notes/placeholder_note.vue';
 import placeholderSystemNote from '../../vue_shared/components/notes/placeholder_system_note.vue';
@@ -38,6 +42,10 @@ export default {
     TimelineEntryItem,
   },
   mixins: [glFeatureFlagsMixin()],
+  provide: {
+    discussionObserver: discussionIntersectionObserver(),
+    discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+  },
   props: {
     noteableData: {
       type: Object,
diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js
index a710ac0ccf5851..9b1f217f9e77ac 100644
--- a/app/assets/javascripts/notes/stores/getters.js
+++ b/app/assets/javascripts/notes/stores/getters.js
@@ -256,15 +256,6 @@ export const findUnresolvedDiscussionIdNeighbor = (state, getters) => ({
   return ids[index];
 };
 
-// Gets the ID of the discussion following the one provided, respecting order (diff or date)
-// @param {Boolean} discussionId - id of the current discussion
-// @param {Boolean} diffOrder - is ordered by diff?
-export const nextUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) =>
-  getters.findUnresolvedDiscussionIdNeighbor({ discussionId, diffOrder, step: 1 });
-
-export const previousUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) =>
-  getters.findUnresolvedDiscussionIdNeighbor({ discussionId, diffOrder, step: -1 });
-
 // @param {Boolean} diffOrder - is ordered by diff?
 export const firstUnresolvedDiscussionId = (state, getters) => (diffOrder) => {
   if (diffOrder) {
@@ -272,6 +263,31 @@ export const firstUnresolvedDiscussionId = (state, getters) => (diffOrder) => {
   }
   return getters.unresolvedDiscussionsIdsByDate[0];
 };
+// @param {Boolean} diffOrder - is ordered by diff?
+export const lastUnresolvedDiscussionId = (state, getters) => (diffOrder) => {
+  const ids = diffOrder
+    ? getters.unresolvedDiscussionsIdsByDiff
+    : getters.unresolvedDiscussionsIdsByDate;
+
+  return ids[ids.length - 1];
+};
+
+// Gets the ID of the discussion following the one provided, respecting order (diff or date)
+// @param {Boolean} discussionId - id of the current discussion
+// @param {Boolean} diffOrder - is ordered by diff?
+export const nextUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) => {
+  // get the last ID so that NEXT is the first one
+  const id = discussionId || getters.lastUnresolvedDiscussionId(diffOrder);
+
+  return getters.findUnresolvedDiscussionIdNeighbor({ discussionId: id, diffOrder, step: 1 });
+};
+
+export const previousUnresolvedDiscussionId = (state, getters) => (discussionId, diffOrder) => {
+  // get the first ID so that PREVIOUS is the last one
+  const id = discussionId || getters.firstUnresolvedDiscussionId(diffOrder);
+
+  return getters.findUnresolvedDiscussionIdNeighbor({ discussionId: id, diffOrder, step: -1 });
+};
 
 export const getDiscussion = (state) => (discussionId) =>
   state.discussions.find((discussion) => discussion.id === discussionId);
diff --git a/spec/frontend/diffs/components/diff_discussions_spec.js b/spec/frontend/diffs/components/diff_discussions_spec.js
index bd6f4cd25454b1..b867b6fff3c7a7 100644
--- a/spec/frontend/diffs/components/diff_discussions_spec.js
+++ b/spec/frontend/diffs/components/diff_discussions_spec.js
@@ -1,6 +1,10 @@
 import { GlIcon } from '@gitlab/ui';
 import { mount, createLocalVue } from '@vue/test-utils';
 import DiffDiscussions from '~/diffs/components/diff_discussions.vue';
+import {
+  discussionIntersectionObserver,
+  discussionIntersectionObserverHandlerFactory,
+} from '~/diffs/utils/discussions';
 import { createStore } from '~/mr_notes/stores';
 import DiscussionNotes from '~/notes/components/discussion_notes.vue';
 import NoteableDiscussion from '~/notes/components/noteable_discussion.vue';
@@ -19,6 +23,10 @@ describe('DiffDiscussions', () => {
     store = createStore();
     wrapper = mount(localVue.extend(DiffDiscussions), {
       store,
+      provide: {
+        discussionObserver: discussionIntersectionObserver(),
+        discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+      },
       propsData: {
         discussions: getDiscussionsMockData(),
         ...props,
diff --git a/spec/frontend/diffs/utils/discussions_spec.js b/spec/frontend/diffs/utils/discussions_spec.js
new file mode 100644
index 00000000000000..a44ca7ff398358
--- /dev/null
+++ b/spec/frontend/diffs/utils/discussions_spec.js
@@ -0,0 +1,151 @@
+import {
+  discussionIntersectionObserver,
+  discussionIntersectionObserverHandlerFactory,
+} from '~/diffs/utils/discussions';
+import { create } from '~/lib/utils/intersection_observer';
+
+jest.mock('~/lib/utils/intersection_observer');
+
+describe('Diff Discussions Utils', () => {
+  describe('discussionIntersectionObserver', () => {
+    it('should call the memoized Intersection Observer creation function with the Diff Discussions-specific options', () => {
+      discussionIntersectionObserver();
+
+      expect(create).toHaveBeenCalledWith({
+        threshold: 0,
+        rootMargin: '0px 0px -50% 0px',
+      });
+    });
+  });
+
+  describe('discussionIntersectionObserverHandlerFactory', () => {
+    it('creates a handler function', () => {
+      expect(discussionIntersectionObserverHandlerFactory()).toBeInstanceOf(Function);
+    });
+
+    describe('intersection observer handler', () => {
+      const functions = {
+        setCurrentDiscussionId: jest.fn(),
+        getPreviousUnresolvedDiscussionId: jest.fn().mockImplementation((id) => {
+          return Number(id) - 1;
+        }),
+      };
+      const defaultProcessableWrapper = {
+        entry: {
+          time: 0,
+          isIntersecting: true,
+          rootBounds: {
+            bottom: 0,
+          },
+          boundingClientRect: {
+            top: 0,
+          },
+        },
+        currentDiscussion: {
+          id: 1,
+        },
+        discussionIds: [1, 30, 20, 200],
+        isFirstUnresolved: false,
+        isDiffsPage: true,
+      };
+      let handler;
+      let getMock;
+      let setMock;
+
+      beforeEach(() => {
+        functions.setCurrentDiscussionId.mockClear();
+        functions.getPreviousUnresolvedDiscussionId.mockClear();
+
+        defaultProcessableWrapper.functions = functions;
+
+        setMock = functions.setCurrentDiscussionId.mock;
+        getMock = functions.getPreviousUnresolvedDiscussionId.mock;
+        handler = discussionIntersectionObserverHandlerFactory();
+      });
+
+      it('debounces multiple simultaneous requests into one queue', () => {
+        handler(defaultProcessableWrapper);
+        handler(defaultProcessableWrapper);
+        handler(defaultProcessableWrapper);
+        handler(defaultProcessableWrapper);
+
+        expect(setTimeout).toHaveBeenCalledTimes(4);
+        expect(clearTimeout).toHaveBeenCalledTimes(3);
+
+        // By only advancing to one timer, we ensure it's all being batched into one queue
+        jest.advanceTimersToNextTimer();
+
+        expect(functions.setCurrentDiscussionId).toHaveBeenCalledTimes(4);
+      });
+
+      it('properly processes, sorts and executes the correct actions for a set of observed intersections', () => {
+        handler(defaultProcessableWrapper);
+        handler({
+          // This observation is here to be filtered out because it's a scrollDown
+          ...defaultProcessableWrapper,
+          entry: {
+            ...defaultProcessableWrapper.entry,
+            isIntersecting: false,
+            boundingClientRect: { top: 10 },
+            rootBounds: { bottom: 100 },
+          },
+        });
+        handler({
+          ...defaultProcessableWrapper,
+          entry: {
+            ...defaultProcessableWrapper.entry,
+            time: 101,
+            isIntersecting: false,
+            rootBounds: { bottom: -100 },
+          },
+          currentDiscussion: { id: 20 },
+        });
+        handler({
+          ...defaultProcessableWrapper,
+          entry: {
+            ...defaultProcessableWrapper.entry,
+            time: 100,
+            isIntersecting: false,
+            boundingClientRect: { top: 100 },
+          },
+          currentDiscussion: { id: 30 },
+          isDiffsPage: false,
+        });
+        handler({
+          ...defaultProcessableWrapper,
+          isFirstUnresolved: true,
+          entry: {
+            ...defaultProcessableWrapper.entry,
+            time: 100,
+            isIntersecting: false,
+            boundingClientRect: { top: 200 },
+          },
+        });
+
+        jest.advanceTimersToNextTimer();
+
+        expect(setMock.calls.length).toBe(4);
+        expect(setMock.calls[0]).toEqual([1]);
+        expect(setMock.calls[1]).toEqual([29]);
+        expect(setMock.calls[2]).toEqual([null]);
+        expect(setMock.calls[3]).toEqual([19]);
+
+        expect(getMock.calls.length).toBe(2);
+        expect(getMock.calls[0]).toEqual([30, false]);
+        expect(getMock.calls[1]).toEqual([20, true]);
+
+        [
+          setMock.invocationCallOrder[0],
+          getMock.invocationCallOrder[0],
+          setMock.invocationCallOrder[1],
+          setMock.invocationCallOrder[2],
+          getMock.invocationCallOrder[1],
+          setMock.invocationCallOrder[3],
+        ].forEach((order, idx, list) => {
+          // Compare each invocation sequence to the one before it (except the first one)
+          expect(list[idx - 1] || -1).toBeLessThan(order);
+        });
+      });
+    });
+  });
+});
diff --git a/spec/frontend/notes/components/discussion_notes_spec.js b/spec/frontend/notes/components/discussion_notes_spec.js
index 59ac75f00e698b..264c8905df2e36 100644
--- a/spec/frontend/notes/components/discussion_notes_spec.js
+++ b/spec/frontend/notes/components/discussion_notes_spec.js
@@ -1,6 +1,10 @@
 import { getByRole } from '@testing-library/dom';
 import { shallowMount, mount } from '@vue/test-utils';
 import '~/behaviors/markdown/render_gfm';
+import {
+  discussionIntersectionObserver,
+  discussionIntersectionObserverHandlerFactory,
+} from '~/diffs/utils/discussions';
 import DiscussionNotes from '~/notes/components/discussion_notes.vue';
 import NoteableNote from '~/notes/components/noteable_note.vue';
 import { SYSTEM_NOTE } from '~/notes/constants';
@@ -26,6 +30,10 @@ describe('DiscussionNotes', () => {
   const createComponent = (props, mountingMethod = shallowMount) => {
     wrapper = mountingMethod(DiscussionNotes, {
       store,
+      provide: {
+        discussionObserver: discussionIntersectionObserver(),
+        discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+      },
       propsData: {
         discussion: discussionMock,
         isExpanded: false,
diff --git a/spec/frontend/notes/components/noteable_discussion_spec.js b/spec/frontend/notes/components/noteable_discussion_spec.js
index 727ef02dcbb8db..8da256f2ef25aa 100644
--- a/spec/frontend/notes/components/noteable_discussion_spec.js
+++ b/spec/frontend/notes/components/noteable_discussion_spec.js
@@ -3,6 +3,10 @@ import { nextTick } from 'vue';
 import discussionWithTwoUnresolvedNotes from 'test_fixtures/merge_requests/resolved_diff_discussion.json';
 import { trimText } from 'helpers/text_helper';
 import mockDiffFile from 'jest/diffs/mock_data/diff_file';
+import {
+  discussionIntersectionObserver,
+  discussionIntersectionObserverHandlerFactory,
+} from '~/diffs/utils/discussions';
 import DiscussionNotes from '~/notes/components/discussion_notes.vue';
 import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
 import ResolveWithIssueButton from '~/notes/components/discussion_resolve_with_issue_button.vue';
@@ -31,6 +35,10 @@ describe('noteable_discussion component', () => {
 
     wrapper = mount(NoteableDiscussion, {
       store,
+      provide: {
+        discussionObserver: discussionIntersectionObserver(),
+        discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+      },
       propsData: { discussion: discussionMock },
     });
   });
@@ -167,6 +175,10 @@ describe('noteable_discussion component', () => {
 
         wrapper = mount(NoteableDiscussion, {
           store,
+          provide: {
+            discussionObserver: discussionIntersectionObserver(),
+            discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+          },
           propsData: { discussion: discussionMock },
         });
       });
@@ -185,6 +197,10 @@ describe('noteable_discussion component', () => {
 
         wrapper = mount(NoteableDiscussion, {
           store,
+          provide: {
+            discussionObserver: discussionIntersectionObserver(),
+            discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+          },
           propsData: { discussion: discussionMock },
         });
       });
diff --git a/spec/frontend/notes/components/notes_app_spec.js b/spec/frontend/notes/components/notes_app_spec.js
index 84d94857fe5f94..3bc51f4a26fbe0 100644
--- a/spec/frontend/notes/components/notes_app_spec.js
+++ b/spec/frontend/notes/components/notes_app_spec.js
@@ -9,6 +9,10 @@ import DraftNote from '~/batch_comments/components/draft_note.vue';
 import batchComments from '~/batch_comments/stores/modules/batch_comments';
 import axios from '~/lib/utils/axios_utils';
 import * as urlUtility from '~/lib/utils/url_utility';
+import {
+  discussionIntersectionObserver,
+  discussionIntersectionObserverHandlerFactory,
+} from '~/diffs/utils/discussions';
 import CommentForm from '~/notes/components/comment_form.vue';
 import NotesApp from '~/notes/components/notes_app.vue';
 import * as constants from '~/notes/constants';
@@ -78,6 +82,10 @@ describe('note_app', () => {
           </div>`,
         },
         {
+          provide: {
+            discussionObserver: discussionIntersectionObserver(),
+            discussionObserverHandler: discussionIntersectionObserverHandlerFactory(),
+          },
           propsData,
           store,
         },
diff --git a/spec/frontend/notes/stores/getters_spec.js b/spec/frontend/notes/stores/getters_spec.js
index 9a11fdba508512..622d500fce7c9d 100644
--- a/spec/frontend/notes/stores/getters_spec.js
+++ b/spec/frontend/notes/stores/getters_spec.js
@@ -398,7 +398,7 @@ describe('Getters Notes Store', () => {
       });
     });
 
-    describe('previosuUnresolvedDiscussionId', () => {
+    describe('previousUnresolvedDiscussionId', () => {
       it('should return result of find neighbor', () => {
         const expectedParams = createDiscussionNeighborParams('123', true, -1);
         const result = getters.previousUnresolvedDiscussionId(state, localGetters)('123', true);
-- 
GitLab


From 2c4d5fecca3a11ef695fc35468bfbe76aafe67f4 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Mon, 6 Dec 2021 10:55:07 -0700
Subject: [PATCH 2/3] Only pass the discussion ID since that's all that's
 needed

---
 app/assets/javascripts/diffs/utils/discussions.js         | 8 +++++---
 .../javascripts/notes/components/discussion_notes.vue     | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/diffs/utils/discussions.js b/app/assets/javascripts/diffs/utils/discussions.js
index ecb22db7e030ed..7a5f36d27995d4 100644
--- a/app/assets/javascripts/diffs/utils/discussions.js
+++ b/app/assets/javascripts/diffs/utils/discussions.js
@@ -70,17 +70,19 @@ export function discussionIntersectionObserverHandlerFactory() {
         .forEach((discussionObservationContainer) => {
           const {
             entry: { type },
-            currentDiscussion: { id },
+            currentDiscussionId,
             isFirstUnresolved,
             isDiffsPage,
             functions: { setCurrentDiscussionId, getPreviousUnresolvedDiscussionId },
           } = discussionObservationContainer;
 
           if (type === 'intersection') {
-            setCurrentDiscussionId(id);
+            setCurrentDiscussionId(currentDiscussionId);
           } else if (type === 'scrollUp') {
             setCurrentDiscussionId(
-              isFirstUnresolved ? null : getPreviousUnresolvedDiscussionId(id, isDiffsPage),
+              isFirstUnresolved
+                ? null
+                : getPreviousUnresolvedDiscussionId(currentDiscussionId, isDiffsPage),
             );
           }
         });
diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue
index c596468b0e6c8a..c0048a1bfad5bf 100644
--- a/app/assets/javascripts/notes/components/discussion_notes.vue
+++ b/app/assets/javascripts/notes/components/discussion_notes.vue
@@ -104,7 +104,7 @@ export default {
         this.discussionObserverHandler({
           entry: observerEntry,
           isFirstUnresolved: this.isFirstUnresolved,
-          currentDiscussion: { ...this.discussion },
+          currentDiscussionId: this.discussion.id,
           discussionIds: this.discussionIds,
           isDiffsPage: !this.isOverviewTab,
           functions: {
-- 
GitLab


From c2ce18036c9d1bacaaeaf98c9a1416f4a6bd4d14 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Mon, 6 Dec 2021 11:42:15 -0700
Subject: [PATCH 3/3] Use only else instead of else if

Right now, there are no other type options
---
 app/assets/javascripts/diffs/utils/discussions.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/javascripts/diffs/utils/discussions.js b/app/assets/javascripts/diffs/utils/discussions.js
index 7a5f36d27995d4..e041c76b081f86 100644
--- a/app/assets/javascripts/diffs/utils/discussions.js
+++ b/app/assets/javascripts/diffs/utils/discussions.js
@@ -78,7 +78,7 @@ export function discussionIntersectionObserverHandlerFactory() {
 
           if (type === 'intersection') {
             setCurrentDiscussionId(currentDiscussionId);
-          } else if (type === 'scrollUp') {
+          } else {
             setCurrentDiscussionId(
               isFirstUnresolved
                 ? null
-- 
GitLab