Merge branch 'jivl-smarter-system-notes' into 'master'

Support smarter system notes

Closes #33166

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

app/assets/javascripts/notes/constants.js

@@ -14,6 +14,7 @@ export const EPIC_NOTEABLE_TYPE = 'epic';
 export const MERGE_REQUEST_NOTEABLE_TYPE = 'merge_request';
 export const UNRESOLVE_NOTE_METHOD_NAME = 'delete';
 export const RESOLVE_NOTE_METHOD_NAME = 'post';
+export const DESCRIPTION_TYPE = 'changed the description';
 
 export const NOTEABLE_TYPE_MAPPING = {
   Issue: ISSUE_NOTEABLE_TYPE,

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

+import { n__, s__, sprintf } from '~/locale';
+import { DESCRIPTION_TYPE } from '../constants';
+
+/**
+ * Changes the description from a note, returns 'changed the description n number of times'
+ */
+export const changeDescriptionNote = (note, descriptionChangedTimes, timeDifferenceMinutes) => {
+  const descriptionNote = Object.assign({}, note);
+
+  descriptionNote.note_html = sprintf(
+    s__(`MergeRequest|
+  %{paragraphStart}changed the description %{descriptionChangedTimes} times %{timeDifferenceMinutes}%{paragraphEnd}`),
+    {
+      paragraphStart: '<p dir="auto">',
+      paragraphEnd: '</p>',
+      descriptionChangedTimes,
+      timeDifferenceMinutes: n__('within %d minute ', 'within %d minutes ', timeDifferenceMinutes),
+    },
+    false,
+  );
+
+  descriptionNote.times_updated = descriptionChangedTimes;
+
+  return descriptionNote;
+};
+
+/**
+ * Checks the time difference between two notes from their 'created_at' dates
+ * returns an integer
+ */
+
+export const getTimeDifferenceMinutes = (noteBeggining, noteEnd) => {
+  const descriptionNoteBegin = new Date(noteBeggining.created_at);
+  const descriptionNoteEnd = new Date(noteEnd.created_at);
+  const timeDifferenceMinutes = (descriptionNoteEnd - descriptionNoteBegin) / 1000 / 60;
+
+  return Math.ceil(timeDifferenceMinutes);
+};
+
+/**
+ * Checks if a note is a system note and if the content is description
+ *
+ * @param {Object} note
+ * @returns {Boolean}
+ */
+export const isDescriptionSystemNote = note => note.system && note.note === DESCRIPTION_TYPE;
+
+/**
+ * Collapses the system notes of a description type, e.g. Changed the description, n minutes ago
+ * the notes will collapse as long as they happen no more than 10 minutes away from each away
+ * in between the notes can be anything, another type of system note
+ * (such as 'changed the weight') or a comment.
+ *
+ * @param {Array} notes
+ * @returns {Array}
+ */
+export const collapseSystemNotes = notes => {
+  let lastDescriptionSystemNote = null;
+  let lastDescriptionSystemNoteIndex = -1;
+  let descriptionChangedTimes = 1;
+
+  return notes.slice(0).reduce((acc, currentNote) => {
+    const note = currentNote.notes[0];
+
+    if (isDescriptionSystemNote(note)) {
+      // is it the first one?
+      if (!lastDescriptionSystemNote) {
+        lastDescriptionSystemNote = note;
+        lastDescriptionSystemNoteIndex = acc.length;
+      } else if (lastDescriptionSystemNote) {
+        const timeDifferenceMinutes = getTimeDifferenceMinutes(
+          lastDescriptionSystemNote,
+          note,
+        );
+
+        // are they less than 10 minutes appart?
+        if (timeDifferenceMinutes > 10) {
+          // reset counter
+          descriptionChangedTimes = 1;
+          // update the previous system note
+          lastDescriptionSystemNote = note;
+          lastDescriptionSystemNoteIndex = acc.length;
+        } else {
+          // increase counter
+          descriptionChangedTimes += 1;
+
+          // delete the previous one
+          acc.splice(lastDescriptionSystemNoteIndex, 1);
+
+          // replace the text of the current system note with the collapsed note.
+          currentNote.notes.splice(
+            0,
+            1,
+            changeDescriptionNote(note, descriptionChangedTimes, timeDifferenceMinutes),
+          );
+
+          // update the previous system note index
+          lastDescriptionSystemNoteIndex = acc.length;
+        }
+      }
+    }
+    acc.push(currentNote);
+    return acc;
+  }, []);
+};
+
+// for babel-rewire
+export default {};

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

 import _ from 'underscore';
+import { collapseSystemNotes } from './collapse_utils';
+
+export const notes = state => collapseSystemNotes(state.notes);
 
-export const notes = state => state.notes;
 export const targetNoteHash = state => state.targetNoteHash;
 
 export const getNotesData = state => state.notesData;

changelogs/unreleased/jivl-smarter-system-notes.yml

+---
+title: Add support for smarter system notes
+merge_request: 17164
+author:
+type: changed

spec/javascripts/notes/stores/collapse_utils_spec.js

+import {
+  isDescriptionSystemNote,
+  changeDescriptionNote,
+  getTimeDifferenceMinutes,
+  collapseSystemNotes,
+} from '~/notes/stores/collapse_utils';
+import {
+  notesWithDescriptionChanges,
+  collapsedSystemNotes,
+} from '../mock_data';
+
+describe('Collapse utils', () => {
+  const mockSystemNote = {
+    note: 'changed the description',
+    note_html: '<p dir="auto">changed the description</p>',
+    system: true,
+    created_at: '2018-05-14T21:28:00.000Z',
+  };
+
+  it('checks if a system note is of a description type', () => {
+    expect(isDescriptionSystemNote(mockSystemNote)).toEqual(true);
+  });
+
+  it('returns false when a system note is not a description type', () => {
+    expect(isDescriptionSystemNote(Object.assign({}, mockSystemNote, { note: 'foo' }))).toEqual(false);
+  });
+
+  it('changes the description to contain the number of changed times', () => {
+    const changedNote = changeDescriptionNote(mockSystemNote, 3, 5);
+
+    expect(changedNote.times_updated).toEqual(3);
+    expect(changedNote.note_html.trim()).toContain('<p dir="auto">changed the description 3 times within 5 minutes </p>');
+  });
+
+  it('gets the time difference between two notes', () => {
+    const anotherSystemNote = {
+      created_at: '2018-05-14T21:33:00.000Z',
+    };
+
+    expect(getTimeDifferenceMinutes(mockSystemNote, anotherSystemNote)).toEqual(5);
+  });
+
+  it('collapses all description system notes made within 10 minutes or less from each other', () => {
+    expect(collapseSystemNotes(notesWithDescriptionChanges)).toEqual(collapsedSystemNotes);
+  });
+});

spec/javascripts/notes/stores/getters_spec.js

 import * as getters from '~/notes/stores/getters';
-import { notesDataMock, userDataMock, noteableDataMock, individualNote } from '../mock_data';
+import { notesDataMock, userDataMock, noteableDataMock, individualNote, collapseNotesMock } from '../mock_data';
 
 describe('Getters Notes Store', () => {
   let state;
+
   beforeEach(() => {
     state = {
       notes: [individualNote],
@@ -20,6 +21,22 @@ describe('Getters Notes Store', () => {
     });
   });
 
+  describe('Collapsed notes', () => {
+    const stateCollapsedNotes = {
+      notes: collapseNotesMock,
+      targetNoteHash: 'hash',
+      lastFetchedAt: 'timestamp',
+
+      notesData: notesDataMock,
+      userData: userDataMock,
+      noteableData: noteableDataMock,
+    };
+
+    it('should return a single system note when a description was updated multiple times', () => {
+      expect(getters.notes(stateCollapsedNotes).length).toEqual(1);
+    });
+  });
+
   describe('targetNoteHash', () => {
     it('should return `targetNoteHash`', () => {
       expect(getters.targetNoteHash(state)).toEqual('hash');