Omit query params for discussions.json

When visiting issuables with a note anchor, we omit the notes filter
query params if the user's current filter is already set to "Show all
activity".

This filter is a no-op and removing it allows us to reuse the
discussions.js request made by startup JS.

Changelog: performance

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

@@ -258,7 +258,13 @@ export default {
     getFetchDiscussionsConfig() {
       const defaultConfig = { path: this.getNotesDataByProp('discussionsPath') };
 
-      if (doesHashExistInUrl(constants.NOTE_UNDERSCORE)) {
+      const currentFilter =
+        this.getNotesDataByProp('notesFilter') || constants.DISCUSSION_FILTERS_DEFAULT_VALUE;
+
+      if (
+        doesHashExistInUrl(constants.NOTE_UNDERSCORE) &&
+        currentFilter !== constants.DISCUSSION_FILTERS_DEFAULT_VALUE
+      ) {
         return {
           ...defaultConfig,
           filter: constants.DISCUSSION_FILTERS_DEFAULT_VALUE,

app/helpers/notes_helper.rb

@@ -188,7 +188,8 @@ module NotesHelper
       reopenPath: reopen_issuable_path(issuable),
       notesPath: notes_url,
       prerenderedNotesCount: issuable.capped_notes_count(MAX_PRERENDERED_NOTES),
-      lastFetchedAt: initial_last_fetched_at
+      lastFetchedAt: initial_last_fetched_at,
+      notesFilter: current_user&.notes_filter_for(issuable)
     }
 
     if issuable.is_a?(MergeRequest)

spec/frontend/notes/components/notes_app_spec.js

@@ -2,7 +2,9 @@ import { mount, shallowMount } from '@vue/test-utils';
 import AxiosMockAdapter from 'axios-mock-adapter';
 import $ from 'jquery';
 import Vue from 'vue';
+import setWindowLocation from 'helpers/set_window_location_helper';
 import { setTestTimeout } from 'helpers/timeout';
+import waitForPromises from 'helpers/wait_for_promises';
 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';
@@ -430,4 +432,57 @@ describe('note_app', () => {
       );
     });
   });
+
+  describe('fetching discussions', () => {
+    describe('when note anchor is not present', () => {
+      it('does not include extra query params', async () => {
+        wrapper = shallowMount(NotesApp, { propsData, store: createStore() });
+        await waitForPromises();
+
+        expect(axiosMock.history.get[0].params).toBeUndefined();
+      });
+    });
+
+    describe('when note anchor is present', () => {
+      const mountWithNotesFilter = (notesFilter) =>
+        shallowMount(NotesApp, {
+          propsData: {
+            ...propsData,
+            notesData: {
+              ...propsData.notesData,
+              notesFilter,
+            },
+          },
+          store: createStore(),
+        });
+
+      beforeEach(() => {
+        setWindowLocation('#note_1');
+      });
+
+      it('does not include extra query params when filter is undefined', async () => {
+        wrapper = mountWithNotesFilter(undefined);
+        await waitForPromises();
+
+        expect(axiosMock.history.get[0].params).toBeUndefined();
+      });
+
+      it('does not include extra query params when filter is already set to default', async () => {
+        wrapper = mountWithNotesFilter(constants.DISCUSSION_FILTERS_DEFAULT_VALUE);
+        await waitForPromises();
+
+        expect(axiosMock.history.get[0].params).toBeUndefined();
+      });
+
+      it('includes extra query params when filter is not set to default', async () => {
+        wrapper = mountWithNotesFilter(constants.COMMENTS_ONLY_FILTER_VALUE);
+        await waitForPromises();
+
+        expect(axiosMock.history.get[0].params).toEqual({
+          notes_filter: constants.DISCUSSION_FILTERS_DEFAULT_VALUE,
+          persist_filter: false,
+        });
+      });
+    });
+  });
 });

spec/helpers/notes_helper_spec.rb

@@ -322,11 +322,21 @@ RSpec.describe NotesHelper do
   describe '#notes_data' do
     let(:issue) { create(:issue, project: project) }
 
-    it 'sets last_fetched_at to 0 when start_at_zero is true' do
+    before do
       @project = project
       @noteable = issue
 
+      allow(helper).to receive(:current_user).and_return(guest)
+    end
+
+    it 'sets last_fetched_at to 0 when start_at_zero is true' do
       expect(helper.notes_data(issue, true)[:lastFetchedAt]).to eq(0)
     end
+
+    it 'includes the current notes filter for the user' do
+      guest.set_notes_filter(UserPreference::NOTES_FILTERS[:only_comments], issue)
+
+      expect(helper.notes_data(issue)[:notesFilter]).to eq(UserPreference::NOTES_FILTERS[:only_comments])
+    end
   end
 end