Merge branch 'issue_51323' into 'master'

Add 'only history' option to notes filter

Closes #51323

See merge request gitlab-org/gitlab-ce!22544

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

 <script>
 import $ from 'jquery';
-import Icon from '~/vue_shared/components/icon.vue';
 import { mapGetters, mapActions } from 'vuex';
+import Icon from '~/vue_shared/components/icon.vue';
+import { DISCUSSION_FILTERS_DEFAULT_VALUE, HISTORY_ONLY_FILTER_VALUE } from '../constants';
 
 export default {
   components: {
@@ -12,14 +13,17 @@ export default {
       type: Array,
       required: true,
     },
-    defaultValue: {
+    selectedValue: {
       type: Number,
       default: null,
       required: false,
     },
   },
   data() {
-    return { currentValue: this.defaultValue };
+    return {
+      currentValue: this.selectedValue,
+      defaultValue: DISCUSSION_FILTERS_DEFAULT_VALUE,
+    };
   },
   computed: {
     ...mapGetters(['getNotesDataByProp']),
@@ -28,8 +32,11 @@ export default {
       return this.filters.find(filter => filter.value === this.currentValue);
     },
   },
+  mounted() {
+    this.toggleCommentsForm();
+  },
   methods: {
-    ...mapActions(['filterDiscussion']),
+    ...mapActions(['filterDiscussion', 'setCommentsDisabled']),
     selectFilter(value) {
       const filter = parseInt(value, 10);
 
@@ -39,6 +46,10 @@ export default {
       if (filter === this.currentValue) return;
       this.currentValue = filter;
       this.filterDiscussion({ path: this.getNotesDataByProp('discussionsPath'), filter });
+      this.toggleCommentsForm();
+    },
+    toggleCommentsForm() {
+      this.setCommentsDisabled(this.currentValue === HISTORY_ONLY_FILTER_VALUE);
     },
   },
 };
@@ -73,6 +84,10 @@ export default {
             >
               {{ filter.title }}
             </button>
+            <div
+              v-if="filter.value === defaultValue"
+              class="dropdown-divider"
+            ></div>
           </li>
         </ul>
       </div>

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

@@ -60,6 +60,7 @@ export default {
       'getNotesDataByProp',
       'discussionCount',
       'isLoading',
+      'commentsDisabled',
     ]),
     noteableType() {
       return this.noteableData.noteableType;
@@ -206,6 +207,7 @@ export default {
     </ul>
 
     <comment-form
+      v-if="!commentsDisabled"
       :noteable-type="noteableType"
       :markdown-version="markdownVersion"
     />

app/assets/javascripts/notes/constants.js

@@ -15,6 +15,8 @@ export const MERGE_REQUEST_NOTEABLE_TYPE = 'MergeRequest';
 export const UNRESOLVE_NOTE_METHOD_NAME = 'delete';
 export const RESOLVE_NOTE_METHOD_NAME = 'post';
 export const DESCRIPTION_TYPE = 'changed the description';
+export const HISTORY_ONLY_FILTER_VALUE = 2;
+export const DISCUSSION_FILTERS_DEFAULT_VALUE = 0;
 
 export const NOTEABLE_TYPE_MAPPING = {
   Issue: ISSUE_NOTEABLE_TYPE,

app/assets/javascripts/notes/discussion_filters.js

@@ -6,7 +6,7 @@ export default store => {
 
   if (discussionFilterEl) {
     const { defaultFilter, notesFilters } = discussionFilterEl.dataset;
-    const defaultValue = defaultFilter ? parseInt(defaultFilter, 10) : null;
+    const selectedValue = defaultFilter ? parseInt(defaultFilter, 10) : null;
     const filterValues = notesFilters ? JSON.parse(notesFilters) : {};
     const filters = Object.keys(filterValues).map(entry => ({
       title: entry,
@@ -24,7 +24,7 @@ export default store => {
         return createElement('discussion-filter', {
           props: {
             filters,
-            defaultValue,
+            selectedValue,
           },
         });
       },

app/assets/javascripts/notes/stores/actions.js

@@ -364,5 +364,9 @@ export const filterDiscussion = ({ dispatch }, { path, filter }) => {
     });
 };
 
+export const setCommentsDisabled = ({ commit }, data) => {
+  commit(types.DISABLE_COMMENTS, data);
+};
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

app/assets/javascripts/notes/stores/getters.js

@@ -192,5 +192,7 @@ export const firstUnresolvedDiscussionId = (state, getters) => diffOrder => {
   return getters.unresolvedDiscussionsIdsByDate[0];
 };
 
+export const commentsDisabled = state => state.commentsDisabled;
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

app/assets/javascripts/notes/stores/modules/index.js

@@ -21,6 +21,7 @@ export default () => ({
     noteableData: {
       current_user: {},
     },
+    commentsDisabled: false,
   },
   actions,
   getters,

app/assets/javascripts/notes/stores/mutation_types.js

@@ -15,6 +15,7 @@ export const UPDATE_DISCUSSION = 'UPDATE_DISCUSSION';
 export const SET_DISCUSSION_DIFF_LINES = 'SET_DISCUSSION_DIFF_LINES';
 export const SET_NOTES_FETCHED_STATE = 'SET_NOTES_FETCHED_STATE';
 export const SET_NOTES_LOADING_STATE = 'SET_NOTES_LOADING_STATE';
+export const DISABLE_COMMENTS = 'DISABLE_COMMENTS';
 
 // DISCUSSION
 export const COLLAPSE_DISCUSSION = 'COLLAPSE_DISCUSSION';

app/assets/javascripts/notes/stores/mutations.js

@@ -225,4 +225,8 @@ export default {
 
     discussion.truncated_diff_lines = diffLines;
   },
+
+  [types.DISABLE_COMMENTS](state, value) {
+    state.commentsDisabled = value;
+  },
 };

app/models/note.rb

@@ -117,6 +117,8 @@ class Note < ActiveRecord::Base
     case notes_filter
     when UserPreference::NOTES_FILTERS[:only_comments]
       user
+    when UserPreference::NOTES_FILTERS[:only_activity]
+      system
     else
       all
     end

app/models/user_preference.rb

@@ -4,7 +4,7 @@ class UserPreference < ActiveRecord::Base
   # We could use enums, but Rails 4 doesn't support multiple
   # enum options with same name for multiple fields, also it creates
   # extra methods that aren't really needed here.
-  NOTES_FILTERS = { all_notes: 0, only_comments: 1 }.freeze
+  NOTES_FILTERS = { all_notes: 0, only_comments: 1, only_activity: 2 }.freeze
 
   belongs_to :user
 
@@ -14,7 +14,8 @@ class UserPreference < ActiveRecord::Base
     def notes_filters
       {
         s_('Notes|Show all activity') => NOTES_FILTERS[:all_notes],
-        s_('Notes|Show comments only') => NOTES_FILTERS[:only_comments]
+        s_('Notes|Show comments only') => NOTES_FILTERS[:only_comments],
+        s_('Notes|Show history only') => NOTES_FILTERS[:only_activity]
       }
     end
   end

app/serializers/user_preference_entity.rb

@@ -7,4 +7,8 @@ class UserPreferenceEntity < Grape::Entity
   expose :notes_filters do |user_preference|
     UserPreference.notes_filters
   end
+
+  expose :default_notes_filter do |user_preference|
+    UserPreference::NOTES_FILTERS[:all_notes]
+  end
 end

changelogs/unreleased/issue_51323.yml

+---
+title: Add 'only history' option to notes filter
+merge_request:
+author:
+type: changed

locale/gitlab.pot

@@ -4165,6 +4165,9 @@ msgstr ""
 msgid "Notes|Show comments only"
 msgstr ""
 
+msgid "Notes|Show history only"
+msgstr ""
+
 msgid "Notification events"
 msgstr ""
 

spec/finders/notes_finder_spec.rb

@@ -13,7 +13,7 @@ describe NotesFinder 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
+      it 'returns only user notes when using only_comments filter' do
         finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:only_comments])
 
         notes = finder.execute
@@ -21,6 +21,14 @@ describe NotesFinder do
         expect(notes).to match_array(comment)
       end
 
+      it 'returns only system notes when using only_activity filters' do
+        finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:only_activity])
+
+        notes = finder.execute
+
+        expect(notes).to match_array(system_note)
+      end
+
       it 'gets all notes' do
         finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:all_activity])
 

spec/javascripts/notes/components/discussion_filter_spec.js

@@ -19,7 +19,7 @@ describe('DiscussionFilter component', () => {
       },
     ];
     const Component = Vue.extend(DiscussionFilter);
-    const defaultValue = discussionFiltersMock[0].value;
+    const selectedValue = discussionFiltersMock[0].value;
 
     store.state.discussions = discussions;
     vm = mountComponentWithStore(Component, {
@@ -27,7 +27,7 @@ describe('DiscussionFilter component', () => {
       store,
       props: {
         filters: discussionFiltersMock,
-        defaultValue,
+        selectedValue,
       },
     });
   });
@@ -63,4 +63,24 @@ describe('DiscussionFilter component', () => {
 
     expect(vm.filterDiscussion).not.toHaveBeenCalled();
   });
+
+  it('disables commenting when "Show history only" filter is applied', () => {
+    const filterItem = vm.$el.querySelector('.dropdown-menu li:last-child button');
+    filterItem.click();
+
+    expect(vm.$store.state.commentsDisabled).toBe(true);
+  });
+
+  it('enables commenting when "Show history only" filter is not applied', () => {
+    const filterItem = vm.$el.querySelector('.dropdown-menu li:first-child button');
+    filterItem.click();
+
+    expect(vm.$store.state.commentsDisabled).toBe(false);
+  });
+
+  it('renders a dropdown divider for the default filter', () => {
+    const defaultFilter = vm.$el.querySelector('.dropdown-menu li:first-child');
+
+    expect(defaultFilter.lastChild.classList).toContain('dropdown-divider');
+  });
 });

spec/javascripts/notes/components/note_app_spec.js

@@ -121,6 +121,13 @@ describe('note_app', () => {
       ).toEqual('Write a comment or drag your files here…');
     });
 
+    it('should not render form when commenting is disabled', () => {
+      store.state.commentsDisabled = true;
+      vm = mountComponent();
+
+      expect(vm.$el.querySelector('.js-main-target-form')).toEqual(null);
+    });
+
     it('should render form comment button as disabled', () => {
       expect(vm.$el.querySelector('.js-note-new-discussion').getAttribute('disabled')).toEqual(
         'disabled',

spec/javascripts/notes/stores/actions_spec.js

@@ -509,4 +509,17 @@ describe('Actions Notes Store', () => {
       expect(mrWidgetEventHub.$emit).toHaveBeenCalledWith('mr.discussion.updated');
     });
   });
+
+  describe('setCommentsDisabled', () => {
+    it('should set comments disabled state', done => {
+      testAction(
+        actions.setCommentsDisabled,
+        true,
+        null,
+        [{ type: 'DISABLE_COMMENTS', payload: true }],
+        [],
+        done,
+      );
+    });
+  });
 });

spec/javascripts/notes/stores/mutation_spec.js

@@ -427,4 +427,14 @@ describe('Notes Store mutations', () => {
       expect(state.discussions[0].expanded).toBe(true);
     });
   });
+
+  describe('DISABLE_COMMENTS', () => {
+    it('should set comments disabled state', () => {
+      const state = {};
+
+      mutations.DISABLE_COMMENTS(state, true);
+
+      expect(state.commentsDisabled).toEqual(true);
+    });
+  });
 });

spec/models/user_preference_spec.rb

@@ -6,22 +6,43 @@ 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] }
 
+    shared_examples 'setting system notes' do
       it 'returns updated discussion filter' do
         filter_name =
-        user_preference.set_notes_filter(only_comments, issuable)
+          user_preference.set_notes_filter(filter, issuable)
 
-      expect(filter_name).to eq(only_comments)
+        expect(filter_name).to eq(filter)
       end
 
       it 'updates discussion filter for issuable class' do
-      user_preference.set_notes_filter(only_comments, issuable)
+        user_preference.set_notes_filter(filter, issuable)
+
+        expect(user_preference.reload.issue_notes_filter).to eq(filter)
+      end
+    end
 
-      expect(user_preference.reload.issue_notes_filter).to eq(only_comments)
+    context 'when filter is set to all notes' do
+      let(:filter) { described_class::NOTES_FILTERS[:all_notes] }
+
+      it_behaves_like 'setting system notes'
+    end
+
+    context 'when filter is set to only comments' do
+      let(:filter) { described_class::NOTES_FILTERS[:only_comments] }
+
+      it_behaves_like 'setting system notes'
+    end
+
+    context 'when filter is set to only activity' do
+      let(:filter) { described_class::NOTES_FILTERS[:only_activity] }
+
+      it_behaves_like 'setting system notes'
     end
 
     context 'when notes_filter parameter is invalid' do
+      let(:only_comments) { described_class::NOTES_FILTERS[:only_comments] }
+
       it 'returns the current notes filter' do
         user_preference.set_notes_filter(only_comments, issuable)
 

spec/support/shared_examples/controllers/issuable_notes_filter_shared_examples.rb

@@ -34,12 +34,24 @@ shared_examples 'issuable notes filter' do
     expect(user.reload.notes_filter_for(issuable)).to eq(0)
   end
 
-  it 'returns no system note' do
+  it 'returns only user comments' do
     user.set_notes_filter(UserPreference::NOTES_FILTERS[:only_comments], issuable)
 
     get :discussions, namespace_id: project.namespace, project_id: project, id: issuable.iid
+    discussions = JSON.parse(response.body)
 
-    expect(JSON.parse(response.body).count).to eq(1)
+    expect(discussions.count).to eq(1)
+    expect(discussions.first["notes"].first["system"]).to be(false)
+  end
+
+  it 'returns only activity notes' do
+    user.set_notes_filter(UserPreference::NOTES_FILTERS[:only_activity], issuable)
+
+    get :discussions, namespace_id: project.namespace, project_id: project, id: issuable.iid
+    discussions = JSON.parse(response.body)
+
+    expect(discussions.count).to eq(1)
+    expect(discussions.first["notes"].first["system"]).to be(true)
   end
 
   context 'when filter is set to "only_comments"' do