Delete description diff in notes

- Follow vuex patterns for actions and mutations

app/assets/javascripts/notes/constants.js

@@ -18,6 +18,7 @@ export const HISTORY_ONLY_FILTER_VALUE = 2;
 export const DISCUSSION_FILTERS_DEFAULT_VALUE = 0;
 export const DISCUSSION_TAB_LABEL = 'show';
 export const NOTE_UNDERSCORE = 'note_';
+export const TIME_DIFFERENCE_VALUE = 10;
 
 export const NOTEABLE_TYPE_MAPPING = {
   Issue: ISSUE_NOTEABLE_TYPE,

app/assets/javascripts/notes/mixins/description_version_history.js

@@ -3,12 +3,12 @@
 export default {
   computed: {
     canSeeDescriptionVersion() {},
+    canDeleteDescriptionVersion() {},
     shouldShowDescriptionVersion() {},
     descriptionVersionToggleIcon() {},
   },
   methods: {
     toggleDescriptionVersion() {},
-    canDeleteDescriptionVersion() {},
     deleteDescriptionVersion() {},
   },
 };

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

@@ -491,39 +491,66 @@ export const convertToDiscussion = ({ commit }, noteId) =>
 export const removeConvertedDiscussion = ({ commit }, noteId) =>
   commit(types.REMOVE_CONVERTED_DISCUSSION, noteId);
 
-export const fetchDescriptionVersion = (_, { endpoint, startingVersion }) => {
+export const setCurrentDiscussionId = ({ commit }, discussionId) =>
+  commit(types.SET_CURRENT_DISCUSSION_ID, discussionId);
+
+export const fetchDescriptionVersion = ({ dispatch }, { endpoint, startingVersion }) => {
   let requestUrl = endpoint;
 
   if (startingVersion) {
     requestUrl = mergeUrlParams({ start_version_id: startingVersion }, requestUrl);
   }
+  dispatch('requestDescriptionVersion');
 
   return axios
     .get(requestUrl)
-    .then(res => res.data)
-    .catch(() => {
+    .then(res => {
+      dispatch('receiveDescriptionVersion', res.data);
+    })
+    .catch(error => {
+      dispatch('receiveDescriptionVersionError', error);
       Flash(__('Something went wrong while fetching description changes. Please try again.'));
     });
 };
 
-export const setCurrentDiscussionId = ({ commit }, discussionId) =>
-  commit(types.SET_CURRENT_DISCUSSION_ID, discussionId);
+export const requestDescriptionVersion = ({ commit }) => {
+  commit(types.REQUEST_DESCRIPTION_VERSION);
+};
+export const receiveDescriptionVersion = ({ commit }, descriptionVersion) => {
+  commit(types.RECEIVE_DESCRIPTION_VERSION, descriptionVersion);
+};
+export const receiveDescriptionVersionError = ({ commit }, error) => {
+  commit(types.RECEIVE_DESCRIPTION_VERSION_ERROR, error);
+};
 
-export const softDeleteDescriptionVersion = (_, { endpoint, startingVersion }) => {
+export const softDeleteDescriptionVersion = ({ dispatch }, { endpoint, startingVersion }) => {
   let requestUrl = endpoint;
 
   if (startingVersion) {
     requestUrl = mergeUrlParams({ start_version_id: startingVersion }, requestUrl);
   }
+  dispatch('requestDeleteDescriptionVersion');
 
   return axios
     .delete(requestUrl)
-    .then(res => res.data)
-    .catch(e => {
+    .then(() => {
+      dispatch('receiveDeleteDescriptionVersion');
+    })
+    .catch(error => {
+      dispatch('receiveDeleteDescriptionVersionError', error);
       Flash(__('Something went wrong while deleting description changes. Please try again.'));
-      return Promise.reject(e);
     });
 };
 
+export const requestDeleteDescriptionVersion = ({ commit }) => {
+  commit(types.REQUEST_DELETE_DESCRIPTION_VERSION);
+};
+export const receiveDeleteDescriptionVersion = ({ commit }) => {
+  commit(types.RECEIVE_DELETE_DESCRIPTION_VERSION, __('Deleted'));
+};
+export const receiveDeleteDescriptionVersionError = ({ commit }, error) => {
+  commit(types.RECEIVE_DELETE_DESCRIPTION_VERSION_ERROR, error);
+};
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

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

-import { DESCRIPTION_TYPE } from '../constants';
+import { DESCRIPTION_TYPE, TIME_DIFFERENCE_VALUE } from '../constants';
 
 /**
  * Checks the time difference between two notes from their 'created_at' dates
@@ -46,7 +46,7 @@ export const collapseSystemNotes = notes => {
 
         // are they less than 10 minutes apart from the same user?
         if (
-          timeDifferenceMinutes > 10 ||
+          timeDifferenceMinutes > TIME_DIFFERENCE_VALUE ||
           note.author.id !== lastDescriptionSystemNote.author.id ||
           lastDescriptionSystemNote.description_version_deleted
         ) {

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

@@ -14,6 +14,7 @@ export default () => ({
     isToggleStateButtonLoading: false,
     isNotesFetched: false,
     isLoading: true,
+    isLoadingDescriptionVersion: false,
 
     // holds endpoints and permissions provided through haml
     notesData: {
@@ -27,6 +28,7 @@ export default () => ({
     commentsDisabled: false,
     resolvableDiscussionsCount: 0,
     unresolvedDiscussionsCount: 0,
+    descriptionVersion: null,
   },
   actions,
   getters,

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

@@ -31,3 +31,11 @@ export const SET_CURRENT_DISCUSSION_ID = 'SET_CURRENT_DISCUSSION_ID';
 export const CLOSE_ISSUE = 'CLOSE_ISSUE';
 export const REOPEN_ISSUE = 'REOPEN_ISSUE';
 export const TOGGLE_STATE_BUTTON_LOADING = 'TOGGLE_STATE_BUTTON_LOADING';
+
+// Description version
+export const REQUEST_DESCRIPTION_VERSION = 'REQUEST_DESCRIPTION_VERSION';
+export const RECEIVE_DESCRIPTION_VERSION = 'RECEIVE_DESCRIPTION_VERSION';
+export const RECEIVE_DESCRIPTION_VERSION_ERROR = 'RECEIVE_DESCRIPTION_VERSION_ERROR';
+export const REQUEST_DELETE_DESCRIPTION_VERSION = 'REQUEST_DELETE_DESCRIPTION_VERSION';
+export const RECEIVE_DELETE_DESCRIPTION_VERSION = 'RECEIVE_DELETE_DESCRIPTION_VERSION';
+export const RECEIVE_DELETE_DESCRIPTION_VERSION_ERROR = 'RECEIVE_DELETE_DESCRIPTION_VERSION_ERROR';

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

@@ -284,4 +284,25 @@ export default {
   [types.SET_CURRENT_DISCUSSION_ID](state, discussionId) {
     state.currentDiscussionId = discussionId;
   },
+
+  [types.REQUEST_DESCRIPTION_VERSION](state) {
+    state.isLoadingDescriptionVersion = true;
+  },
+  [types.RECEIVE_DESCRIPTION_VERSION](state, descriptionVersion) {
+    state.isLoadingDescriptionVersion = false;
+    state.descriptionVersion = descriptionVersion;
+  },
+  [types.RECEIVE_DESCRIPTION_VERSION_ERROR](state) {
+    state.isLoadingDescriptionVersion = false;
+  },
+  [types.REQUEST_DELETE_DESCRIPTION_VERSION](state) {
+    state.isLoadingDescriptionVersion = true;
+  },
+  [types.RECEIVE_DELETE_DESCRIPTION_VERSION](state, descriptionVersion) {
+    state.isLoadingDescriptionVersion = false;
+    state.descriptionVersion = descriptionVersion;
+  },
+  [types.RECEIVE_DELETE_DESCRIPTION_VERSION_ERROR](state) {
+    state.isLoadingDescriptionVersion = false;
+  },
 };

app/assets/javascripts/vue_shared/components/notes/system_note.vue

@@ -17,8 +17,8 @@
  *   />
  */
 import $ from 'jquery';
-import { mapGetters, mapActions } from 'vuex';
-import { GlSkeletonLoading, GlTooltipDirective } from '@gitlab/ui';
+import { mapGetters, mapActions, mapState } from 'vuex';
+import { GlButton, GlSkeletonLoading, GlTooltipDirective } from '@gitlab/ui';
 import descriptionVersionHistoryMixin from 'ee_else_ce/notes/mixins/description_version_history';
 import noteHeader from '~/notes/components/note_header.vue';
 import Icon from '~/vue_shared/components/icon.vue';
@@ -35,6 +35,7 @@ export default {
     Icon,
     noteHeader,
     TimelineEntryItem,
+    GlButton,
     GlSkeletonLoading,
   },
   directives: {
@@ -54,6 +55,7 @@ export default {
   },
   computed: {
     ...mapGetters(['targetNoteHash']),
+    ...mapState(['descriptionVersion', 'isLoadingDescriptionVersion']),
     noteAnchorId() {
       return `note_${this.note.id}`;
     },
@@ -126,16 +128,16 @@ export default {
             <gl-skeleton-loading />
           </pre>
           <pre v-else class="wrapper mt-2" v-html="descriptionVersion"></pre>
-          <button
+          <gl-button
             v-if="canDeleteDescriptionVersion"
+            ref="deleteDescriptionVersionButton"
             v-gl-tooltip
-            type="button"
-            title="Remove description history"
-            class="btn btn-transparent delete-description-history"
+            :title="__('Remove description history')"
+            class="btn-transparent delete-description-history"
             @click="deleteDescriptionVersion"
           >
             <icon name="remove" />
-          </button>
+          </gl-button>
         </div>
       </div>
     </div>

app/assets/stylesheets/pages/notes.scss

@@ -322,10 +322,7 @@ $note-form-margin-left: 72px;
         pre {
           max-height: $dropdown-max-height-lg;
           white-space: pre-wrap;
-
-          &.loading-state {
-            height: 94px;
-          }
+          padding-right: 30px;
         }
       }
 

ee/app/assets/javascripts/notes/mixins/description_version_history.js

-import { s__ } from '~/locale';
-
 export default {
   data() {
     return {
-      isLoadingDescriptionVersion: false,
       isDescriptionVersionExpanded: false,
-      descriptionVersion: '',
     };
   },
   computed: {
@@ -34,22 +30,16 @@ export default {
         return false;
       }
 
-      this.isLoadingDescriptionVersion = true;
       const endpoint = this.note.description_diff_path;
       const startingVersion = this.note.start_description_version_id;
 
-      return this.fetchDescriptionVersion({ endpoint, startingVersion }).then(diff => {
-        this.isLoadingDescriptionVersion = false;
-        this.descriptionVersion = diff;
-      });
+      return this.fetchDescriptionVersion({ endpoint, startingVersion });
     },
     deleteDescriptionVersion() {
       const endpoint = this.note.delete_description_version_path;
       const startingVersion = this.note.start_description_version_id;
 
-      return this.softDeleteDescriptionVersion({ endpoint, startingVersion }).then(() => {
-        this.descriptionVersion = s__('Deleted');
-      });
+      return this.softDeleteDescriptionVersion({ endpoint, startingVersion });
     },
   },
 };

ee/spec/frontend/vue_shared/components/notes/system_note_spec.js

@@ -20,6 +20,10 @@ describe('system note component', () => {
     mock.onDelete('/path/to/diff/1').replyOnce(200);
   }
 
+  const findBlankBtn = () => wrapper.find('.note-headline-light .btn-blank');
+
+  const findDescriptionVersion = () => wrapper.find('.description-version');
+
   beforeEach(() => {
     props = {
       note: {
@@ -63,24 +67,24 @@ describe('system note component', () => {
   });
 
   it('should display button to toggle description diff, description version does not display', () => {
-    const button = wrapper.find('.note-headline-light .btn-blank');
+    const button = findBlankBtn();
     expect(button.exists()).toBe(true);
     expect(button.text()).toContain('Compare with previous version');
-    expect(wrapper.find('.description-version').exists()).toBe(false);
+    expect(findDescriptionVersion().exists()).toBe(false);
   });
 
   it('click on button to toggle description diff displays description diff with delete icon button', done => {
     mockFetchDiff();
-    expect(wrapper.find('.description-version').exists()).toBe(false);
+    expect(findDescriptionVersion().exists()).toBe(false);
 
-    const button = wrapper.find('.note-headline-light .btn-blank');
+    const button = findBlankBtn();
     button.trigger('click');
     return wrapper.vm
       .$nextTick()
       .then(() => waitForPromises())
       .then(() => {
-        expect(wrapper.find('.description-version').exists()).toBe(true);
-        expect(wrapper.find('.description-version').html()).toContain(diffData);
+        expect(findDescriptionVersion().exists()).toBe(true);
+        expect(findDescriptionVersion().html()).toContain(diffData);
         expect(
           wrapper
             .find('.description-version button.delete-description-history svg.ic-remove')
@@ -93,17 +97,18 @@ describe('system note component', () => {
   it('click on delete icon button deletes description diff', done => {
     mockFetchDiff();
     mockDeleteDiff();
-    wrapper.find('.note-headline-light .btn-blank').trigger('click');
+    const button = findBlankBtn();
+    button.trigger('click');
     return wrapper.vm
       .$nextTick()
       .then(() => waitForPromises())
       .then(() => {
-        const button = wrapper.find('.description-version button.delete-description-history');
-        button.trigger('click');
+        const deleteButton = wrapper.find({ ref: 'deleteDescriptionVersionButton' });
+        deleteButton.trigger('click');
       })
       .then(() => waitForPromises())
       .then(() => {
-        expect(wrapper.find('.description-version').text()).toContain('Deleted');
+        expect(findDescriptionVersion().text()).toContain('Deleted');
         done();
       });
   });

locale/gitlab.pot

@@ -15860,6 +15860,9 @@ msgstr ""
 msgid "Remove child epic from an epic"
 msgstr ""
 
+msgid "Remove description history"
+msgstr ""
+
 msgid "Remove due date"
 msgstr ""