Resolve "Filter discussion (tab) by comments or activity in issues and merge requests"

12 files changed, 274 insertions, 2 deletions

diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 9df77560320..80138183c07 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1028,6 +1028,13 @@ describe Projects::IssuesController do
           .not_to exceed_query_limit(control)
       end
 
+      context 'when user is setting notes filters' do
+        let(:issuable) { issue }
+        let!(:discussion_note) { create(:discussion_note_on_issue, :system, noteable: issuable, project: project) }
+
+        it_behaves_like 'issuable notes filter'
+      end
+
       context 'with cross-reference system note', :request_store do
         let(:new_issue) { create(:issue) }
         let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" }
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 78581dc37a4..dcfd6c05200 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -87,6 +87,14 @@ describe Projects::MergeRequestsController do
       end
     end
 
+    context 'when user is setting notes filters' do
+      let(:issuable) { merge_request }
+      let!(:discussion_note) { create(:discussion_note_on_merge_request, :system, noteable: issuable, project: project) }
+      let!(:discussion_comment) { create(:discussion_note_on_merge_request, noteable: issuable, project: project) }
+
+      it_behaves_like 'issuable notes filter'
+    end
+
     describe 'as json' do
       context 'with basic serializer param' do
         it 'renders basic MR entity as json' do
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index e48c9dea976..9ac7b8ee8a8 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -47,6 +47,37 @@ describe Projects::NotesController do
       get :index, request_params
     end
 
+    context 'when user notes_filter is present' do
+      let(:notes_json) { parsed_response[:notes] }
+      let!(:comment) { create(:note, noteable: issue, project: project) }
+      let!(:system_note) { create(:note, noteable: issue, project: project, system: true) }
+
+      it 'filters system notes by comments' do
+        user.set_notes_filter(UserPreference::NOTES_FILTERS[:only_comments], issue)
+
+        get :index, request_params
+
+        expect(notes_json.count).to eq(1)
+        expect(notes_json.first[:id].to_i).to eq(comment.id)
+      end
+
+      it 'returns all notes' do
+        user.set_notes_filter(UserPreference::NOTES_FILTERS[:all_notes], issue)
+
+        get :index, request_params
+
+        expect(notes_json.map { |note| note[:id].to_i }).to contain_exactly(comment.id, system_note.id)
+      end
+
+      it 'does not merge label event notes' do
+        user.set_notes_filter(UserPreference::NOTES_FILTERS[:only_comments], issue)
+
+        expect(ResourceEvents::MergeIntoNotesService).not_to receive(:new)
+
+        get :index, request_params
+      end
+    end
+
     context 'for a discussion note' do
       let(:project) { create(:project, :repository) }
       let!(:note) { create(:discussion_note_on_merge_request, project: project) }
diff --git a/spec/factories/user_preferences.rb b/spec/factories/user_preferences.rb
new file mode 100644
index 00000000000..19059a93625
--- /dev/null
+++ b/spec/factories/user_preferences.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :user_preference do
+    user
+
+    trait :only_comments do
+      issue_notes_filter { UserPreference::NOTES_FILTERS[:only_comments] }
+      merge_request_notes_filter { UserPreference::NOTE_FILTERS[:only_comments] }
+    end
+  end
+end
diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb
index b776e9d856a..de9974c45e1 100644
--- a/spec/finders/notes_finder_spec.rb
+++ b/spec/finders/notes_finder_spec.rb
@@ -9,6 +9,27 @@ describe NotesFinder do
   end
 
   describe '#execute' do
+    context 'when notes filter is present' do
+      let!(:comment) { create(:note_on_issue, project: project) }
+      let!(:system_note) { create(:note_on_issue, project: project, system: true) }
+
+      it 'filters system notes' do
+        finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:only_comments])
+
+        notes = finder.execute
+
+        expect(notes).to match_array(comment)
+      end
+
+      it 'gets all notes' do
+        finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:all_activity])
+
+        notes = finder.execute
+
+        expect(notes).to match_array([comment, system_note])
+      end
+    end
+
     it 'finds notes on merge requests' do
       create(:note_on_merge_request, project: project)
 
diff --git a/spec/javascripts/notes/components/discussion_filter_spec.js b/spec/javascripts/notes/components/discussion_filter_spec.js
new file mode 100644
index 00000000000..70dd5bb3be5
--- /dev/null
+++ b/spec/javascripts/notes/components/discussion_filter_spec.js
@@ -0,0 +1,60 @@
+import Vue from 'vue';
+import createStore from '~/notes/stores';
+import DiscussionFilter from '~/notes/components/discussion_filter.vue';
+import { mountComponentWithStore } from '../../helpers/vue_mount_component_helper';
+import { discussionFiltersMock, discussionMock } from '../mock_data';
+
+describe('DiscussionFilter component', () => {
+  let vm;
+  let store;
+
+  beforeEach(() => {
+    store = createStore();
+
+    const discussions = [{
+      ...discussionMock,
+      id: discussionMock.id,
+      notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }],
+    }];
+    const Component = Vue.extend(DiscussionFilter);
+    const defaultValue = discussionFiltersMock[0].value;
+
+    store.state.discussions = discussions;
+    vm = mountComponentWithStore(Component, {
+      el: null,
+      store,
+      props: {
+        filters: discussionFiltersMock,
+        defaultValue,
+      },
+    });
+  });
+
+  afterEach(() => {
+    vm.$destroy();
+  });
+
+  it('renders the all filters', () => {
+    expect(vm.$el.querySelectorAll('.dropdown-menu li').length).toEqual(discussionFiltersMock.length);
+  });
+
+  it('renders the default selected item', () => {
+    expect(vm.$el.querySelector('#discussion-filter-dropdown').textContent.trim()).toEqual(discussionFiltersMock[0].title);
+  });
+
+  it('updates to the selected item', () => {
+    const filterItem = vm.$el.querySelector('.dropdown-menu li:last-child button');
+    filterItem.click();
+
+    expect(vm.currentFilter.title).toEqual(filterItem.textContent.trim());
+  });
+
+  it('only updates when selected filter changes', () => {
+    const filterItem = vm.$el.querySelector('.dropdown-menu li:first-child button');
+
+    spyOn(vm, 'filterDiscussion');
+    filterItem.click();
+
+    expect(vm.filterDiscussion).not.toHaveBeenCalled();
+  });
+});
diff --git a/spec/javascripts/notes/components/note_app_spec.js b/spec/javascripts/notes/components/note_app_spec.js
index 3e289a6b8e6..06b30375306 100644
--- a/spec/javascripts/notes/components/note_app_spec.js
+++ b/spec/javascripts/notes/components/note_app_spec.js
@@ -97,8 +97,7 @@ describe('note_app', () => {
     });
 
     it('should render list of notes', done => {
-      const note =
-        mockData.INDIVIDUAL_NOTE_RESPONSE_MAP.GET[
+      const note = mockData.INDIVIDUAL_NOTE_RESPONSE_MAP.GET[
           '/gitlab-org/gitlab-ce/issues/26/discussions.json'
         ][0].notes[0];
 
diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js
index 9a0e7f34a9c..ad0e793b915 100644
--- a/spec/javascripts/notes/mock_data.js
+++ b/spec/javascripts/notes/mock_data.js
@@ -1244,3 +1244,18 @@ export const discussion3 = {
 export const unresolvableDiscussion = {
   resolvable: false,
 };
+
+export const discussionFiltersMock = [
+  {
+    title: 'Show all activity',
+    value: 0,
+  },
+  {
+    title: 'Show comments only',
+    value: 1,
+  },
+  {
+    title: 'Show system notes only',
+    value: 2,
+  },
+];
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 1783dd3206b..f9be61e4768 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -865,5 +865,29 @@ describe Note do
         note.save!
       end
     end
+
+    describe '#with_notes_filter' do
+      let!(:comment) { create(:note) }
+      let!(:system_note) { create(:note, system: true) }
+
+      context 'when notes filter is nil' do
+        subject { described_class.with_notes_filter(nil) }
+
+        it { is_expected.to include(comment, system_note) }
+      end
+
+      context 'when notes filter is set to all notes' do
+        subject { described_class.with_notes_filter(UserPreference::NOTES_FILTERS[:all_notes]) }
+
+        it { is_expected.to include(comment, system_note) }
+      end
+
+      context 'when notes filter is set to only comments' do
+        subject { described_class.with_notes_filter(UserPreference::NOTES_FILTERS[:only_comments]) }
+
+        it { is_expected.to include(comment) }
+        it { is_expected.not_to include(system_note) }
+      end
+    end
   end
 end
diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb
new file mode 100644
index 00000000000..64d9d9a78b4
--- /dev/null
+++ b/spec/models/user_preference_spec.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe UserPreference do
+  describe '#set_notes_filter' do
+    let(:issuable) { build_stubbed(:issue) }
+    let(:user_preference) { create(:user_preference) }
+    let(:only_comments) { described_class::NOTES_FILTERS[:only_comments] }
+
+    it 'returns updated discussion filter' do
+      filter_name =
+        user_preference.set_notes_filter(only_comments, issuable)
+
+      expect(filter_name).to eq(only_comments)
+    end
+
+    it 'updates discussion filter for issuable class' do
+      user_preference.set_notes_filter(only_comments, issuable)
+
+      expect(user_preference.reload.issue_notes_filter).to eq(only_comments)
+    end
+
+    context 'when notes_filter parameter is invalid' do
+      it 'returns the current notes filter' do
+        user_preference.set_notes_filter(only_comments, issuable)
+
+        expect(user_preference.set_notes_filter(9999, issuable)).to eq(only_comments)
+      end
+    end
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 99d17f563d9..b3474e74aa4 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -715,6 +715,15 @@ describe User do
     end
   end
 
+  describe 'ensure user preference' do
+    it 'has user preference upon user initialization' do
+      user = build(:user)
+
+      expect(user.user_preference).to be_present
+      expect(user.user_preference).not_to be_persisted
+    end
+  end
+
   describe 'ensure incoming email token' do
     it 'has incoming email token' do
       user = create(:user)
diff --git a/spec/support/shared_examples/controllers/issuable_notes_filter_shared_examples.rb b/spec/support/shared_examples/controllers/issuable_notes_filter_shared_examples.rb
new file mode 100644
index 00000000000..9c9d7ad781e
--- /dev/null
+++ b/spec/support/shared_examples/controllers/issuable_notes_filter_shared_examples.rb
@@ -0,0 +1,54 @@
+shared_examples 'issuable notes filter' do
+  it 'sets discussion filter' do
+    notes_filter = UserPreference::NOTES_FILTERS[:only_comments]
+
+    get :discussions, namespace_id: project.namespace, project_id: project, id: issuable.iid, notes_filter: notes_filter
+
+    expect(user.reload.notes_filter_for(issuable)).to eq(notes_filter)
+    expect(UserPreference.count).to eq(1)
+  end
+
+  it 'expires notes e-tag cache for issuable if filter changed' do
+    notes_filter = UserPreference::NOTES_FILTERS[:only_comments]
+
+    expect_any_instance_of(issuable.class).to receive(:expire_note_etag_cache)
+
+    get :discussions, namespace_id: project.namespace, project_id: project, id: issuable.iid, notes_filter: notes_filter
+  end
+
+  it 'does not expires notes e-tag cache for issuable if filter did not change' do
+    notes_filter = UserPreference::NOTES_FILTERS[:only_comments]
+    user.set_notes_filter(notes_filter, issuable)
+
+    expect_any_instance_of(issuable.class).not_to receive(:expire_note_etag_cache)
+
+    get :discussions, namespace_id: project.namespace, project_id: project, id: issuable.iid, notes_filter: notes_filter
+  end
+
+  it 'does not set notes filter when database is in read only mode' do
+    allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+    notes_filter = UserPreference::NOTES_FILTERS[:only_comments]
+
+    get :discussions, namespace_id: project.namespace, project_id: project, id: issuable.iid, notes_filter: notes_filter
+
+    expect(user.reload.notes_filter_for(issuable)).to eq(0)
+  end
+
+  it 'returns no system note' do
+    user.set_notes_filter(UserPreference::NOTES_FILTERS[:only_comments], issuable)
+
+    get :discussions, namespace_id: project.namespace, project_id: project, id: issuable.iid
+
+    expect(JSON.parse(response.body).count).to eq(1)
+  end
+
+  context 'when filter is set to "only_comments"' do
+    it 'does not merge label event notes' do
+      user.set_notes_filter(UserPreference::NOTES_FILTERS[:only_comments], issuable)
+
+      expect(ResourceEvents::MergeIntoNotesService).not_to receive(:new)
+
+      get :discussions, namespace_id: project.namespace, project_id: project, id: issuable.iid
+    end
+  end
+end