Merge branch 'ss/sort-issue-local-storage' into 'master'

Add localStorage support for sort issue and MR notes

See merge request gitlab-org/gitlab!28143

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

+gs
 <script>
 import { GlIcon } from '@gitlab/ui';
 import { mapActions, mapGetters } from 'vuex';
 import { __ } from '~/locale';
+import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue';
 import Tracking from '~/tracking';
 import { ASC, DESC } from '../constants';
 
@@ -14,16 +16,20 @@ export default {
   SORT_OPTIONS,
   components: {
     GlIcon,
+    LocalStorageSync,
   },
   mixins: [Tracking.mixin()],
   computed: {
-    ...mapGetters(['sortDirection']),
+    ...mapGetters(['sortDirection', 'noteableType']),
     selectedOption() {
       return SORT_OPTIONS.find(({ key }) => this.sortDirection === key);
     },
     dropdownText() {
       return this.selectedOption.text;
     },
+    storageKey() {
+      return `sort_direction_${this.noteableType.toLowerCase()}`;
+    },
   },
   methods: {
     ...mapActions(['setDiscussionSortDirection']),
@@ -44,6 +50,11 @@ export default {
 
 <template>
   <div class="mr-2 d-inline-block align-bottom full-width-mobile">
+    <local-storage-sync
+      :value="sortDirection"
+      :storage-key="storageKey"
+      @input="setDiscussionSortDirection"
+    />
     <button class="btn btn-sm js-dropdown-text" data-toggle="dropdown" aria-expanded="false">
       {{ dropdownText }}
       <gl-icon name="chevron-down" />

app/assets/javascripts/vue_shared/components/local_storage_sync.vue

+<script>
+export default {
+  props: {
+    storageKey: {
+      type: String,
+      required: true,
+    },
+    value: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  watch: {
+    value(newVal) {
+      this.saveValue(newVal);
+    },
+  },
+  mounted() {
+    // On mount, trigger update if we actually have a localStorageValue
+    const value = this.getValue();
+
+    if (value && this.value !== value) {
+      this.$emit('input', value);
+    }
+  },
+  methods: {
+    getValue() {
+      return localStorage.getItem(this.storageKey);
+    },
+    saveValue(val) {
+      localStorage.setItem(this.storageKey, val);
+    },
+  },
+  render() {
+    return this.$slots.default;
+  },
+};
+</script>

spec/features/issues/user_sorts_issue_comments_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Comment sort direction' do
+  let_it_be(:project) { create(:project, :public, :repository) }
+  let_it_be(:issue) { create(:issue, project: project) }
+  let_it_be(:comment_1) { create(:note_on_issue, noteable: issue, project: project, note: 'written first') }
+  let_it_be(:comment_2) { create(:note_on_issue, noteable: issue, project: project, note: 'written second') }
+
+  context 'on issue page', :js do
+    before do
+      visit project_issue_path(project, issue)
+    end
+
+    it 'saves sort order' do
+      # open dropdown, and select 'Newest first'
+      page.within('.issuable-details') do
+        click_button('Oldest first')
+        click_button('Newest first')
+      end
+
+      expect(first_comment).to have_content(comment_2.note)
+      expect(last_comment).to have_content(comment_1.note)
+
+      visit project_issue_path(project, issue)
+      wait_for_requests
+
+      expect(first_comment).to have_content(comment_2.note)
+      expect(last_comment).to have_content(comment_1.note)
+    end
+  end
+
+  def all_comments
+    all('.timeline > .note.timeline-entry')
+  end
+
+  def first_comment
+    all_comments.first
+  end
+
+  def last_comment
+    all_comments.last
+  end
+end

spec/frontend/notes/components/sort_discussion_spec.js

 import { shallowMount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
 import SortDiscussion from '~/notes/components/sort_discussion.vue';
+import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue';
 import createStore from '~/notes/stores';
 import { ASC, DESC } from '~/notes/constants';
 import Tracking from '~/tracking';
@@ -21,6 +22,8 @@ describe('Sort Discussion component', () => {
     });
   };
 
+  const findLocalStorageSync = () => wrapper.find(LocalStorageSync);
+
   beforeEach(() => {
     store = createStore();
     jest.spyOn(Tracking, 'event');
@@ -31,6 +34,22 @@ describe('Sort Discussion component', () => {
     wrapper = null;
   });
 
+  describe('default', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
+    it('has local storage sync', () => {
+      expect(findLocalStorageSync().exists()).toBe(true);
+    });
+
+    it('calls setDiscussionSortDirection when update is emitted', () => {
+      findLocalStorageSync().vm.$emit('input', ASC);
+
+      expect(store.dispatch).toHaveBeenCalledWith('setDiscussionSortDirection', ASC);
+    });
+  });
+
   describe('when asc', () => {
     describe('when the dropdown is clicked', () => {
       it('calls the right actions', () => {

spec/frontend/vue_shared/components/local_storage_sync_spec.js

+import { shallowMount } from '@vue/test-utils';
+import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue';
+
+describe('Local Storage Sync', () => {
+  let wrapper;
+
+  const createComponent = ({ props = {}, slots = {} } = {}) => {
+    wrapper = shallowMount(LocalStorageSync, {
+      propsData: props,
+      slots,
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+    localStorage.clear();
+  });
+
+  it('is a renderless component', () => {
+    const html = '<div class="test-slot"></div>';
+    createComponent({
+      props: {
+        storageKey: 'key',
+      },
+      slots: {
+        default: html,
+      },
+    });
+
+    expect(wrapper.html()).toBe(html);
+  });
+
+  describe('localStorage empty', () => {
+    const storageKey = 'issue_list_order';
+
+    it('does not emit input event', () => {
+      createComponent({
+        props: {
+          storageKey,
+          value: 'ascending',
+        },
+      });
+
+      expect(wrapper.emitted('input')).toBeFalsy();
+    });
+
+    it('saves updated value to localStorage', () => {
+      createComponent({
+        props: {
+          storageKey,
+          value: 'ascending',
+        },
+      });
+
+      const newValue = 'descending';
+      wrapper.setProps({
+        value: newValue,
+      });
+
+      return wrapper.vm.$nextTick().then(() => {
+        expect(localStorage.getItem(storageKey)).toBe(newValue);
+      });
+    });
+
+    it('does not save default value', () => {
+      const value = 'ascending';
+
+      createComponent({
+        props: {
+          storageKey,
+          value,
+        },
+      });
+
+      expect(localStorage.getItem(storageKey)).toBe(null);
+    });
+  });
+
+  describe('localStorage has saved value', () => {
+    const storageKey = 'issue_list_order_by';
+    const savedValue = 'last_updated';
+
+    beforeEach(() => {
+      localStorage.setItem(storageKey, savedValue);
+    });
+
+    it('emits input event with saved value', () => {
+      createComponent({
+        props: {
+          storageKey,
+          value: 'ascending',
+        },
+      });
+
+      expect(wrapper.emitted('input')[0][0]).toBe(savedValue);
+    });
+
+    it('does not overwrite localStorage with prop value', () => {
+      createComponent({
+        props: {
+          storageKey,
+          value: 'created',
+        },
+      });
+
+      expect(localStorage.getItem(storageKey)).toBe(savedValue);
+    });
+
+    it('updating the value updates localStorage', () => {
+      createComponent({
+        props: {
+          storageKey,
+          value: 'created',
+        },
+      });
+
+      const newValue = 'last_updated';
+      wrapper.setProps({
+        value: newValue,
+      });
+
+      return wrapper.vm.$nextTick().then(() => {
+        expect(localStorage.getItem(storageKey)).toBe(newValue);
+      });
+    });
+  });
+});