Merge branch 'refactor-epics-sort-for-roadmap' into 'master'

Refactor epic sort comparator for roadmap

See merge request gitlab-org/gitlab!50673

ee/app/assets/javascripts/roadmap/constants.js

@@ -60,10 +60,6 @@ export const PRESET_DEFAULTS = {
   },
 };
 
-export const PAST_DATE = new Date(new Date().getFullYear() - 100, 0, 1);
-
-export const FUTURE_DATE = new Date(new Date().getFullYear() + 100, 0, 1);
-
 export const EPIC_LEVEL_MARGIN = {
   1: 'ml-4',
   2: 'ml-6',

ee/app/assets/javascripts/roadmap/utils/roadmap_utils.js

@@ -6,8 +6,6 @@ import {
   EXTEND_AS,
   TIMELINE_CELL_MIN_WIDTH,
   DAYS_IN_WEEK,
-  PAST_DATE,
-  FUTURE_DATE,
 } from '../constants';
 
 const monthsForQuarters = {
@@ -406,49 +404,39 @@ export const getEpicsTimeframeRange = ({ presetType = '', timeframe = [] }) => {
   };
 };
 
-/**
- * This function takes two epics and return sortable dates depending on the '
- * type of sorting order -- startDate or endDate.
- */
-export function assignDates(a, b, { dateUndefined, outOfRange, originalDate, date, proxyDate }) {
-  let aDate;
-  let bDate;
-
-  if (a[dateUndefined]) {
-    // Set proxy date to be either far in the past or
-    // far in the future to ensure sort order is
-    // correct.
-    aDate = proxyDate;
-  } else {
-    aDate = a[outOfRange] ? a[originalDate] : a[date];
-  }
-
-  if (b[dateUndefined]) {
-    bDate = proxyDate;
-  } else {
-    bDate = b[outOfRange] ? b[originalDate] : b[date];
-  }
-
-  return [aDate, bDate];
-}
-
 export const sortEpics = (epics, sortedBy) => {
   const sortByStartDate = sortedBy.indexOf('start_date') > -1;
   const sortOrderAsc = sortedBy.indexOf('asc') > -1;
 
   epics.sort((a, b) => {
-    const [aDate, bDate] = assignDates(a, b, {
-      dateUndefined: sortByStartDate ? 'startDateUndefined' : 'endDateUndefined',
-      outOfRange: sortByStartDate ? 'startDateOutOfRange' : 'endDateOutOfRange',
-      originalDate: sortByStartDate ? 'originalStartDate' : 'originalEndDate',
-      date: sortByStartDate ? 'startDate' : 'endDate',
-      proxyDate: sortByStartDate ? PAST_DATE : FUTURE_DATE,
-    });
+    let aDate;
+    let bDate;
+
+    if (sortByStartDate) {
+      // Always use the original start date.
+      // if originalStartDate exists, it means startDate was changed to a proxy date
+      // (refer to roadmap_item_utils.js)
+      const startDateForA = a.originalStartDate ? a.originalStartDate : a.startDate;
+      const startDateForB = b.originalStartDate ? b.originalStartDate : b.startDate;
+
+      // When epic has no fixed start date, use Number.NEGATIVE_INFINITY for comparison.
+      // In other words, epics without fixed start date should, in theory, have the earliest start date.
+      // (the actual min possible value for Date object is much smaller; ECMA-262 20.4.1.1)
+      aDate = a.startDateUndefined ? Number.NEGATIVE_INFINITY : startDateForA.getTime();
+      bDate = b.startDateUndefined ? Number.NEGATIVE_INFINITY : startDateForB.getTime();
+    } else {
+      const endDateForA = a.originalEndDate ? a.originalEndDate : a.endDate;
+      const endDateForB = b.originalEndDate ? b.originalEndDate : b.endDate;
+
+      // Similarly, use Infinity when epic has no fixed due date.
+      aDate = a.endDateUndefined ? Infinity : endDateForA.getTime();
+      bDate = b.endDateUndefined ? Infinity : endDateForB.getTime();
+    }
 
     // Sort in ascending or descending order
-    if (aDate.getTime() < bDate.getTime()) {
+    if (aDate < bDate) {
       return sortOrderAsc ? -1 : 1;
-    } else if (aDate.getTime() > bDate.getTime()) {
+    } else if (aDate > bDate) {
       return sortOrderAsc ? 1 : -1;
     }
     return 0;

ee/spec/frontend/roadmap/mock_data.js

@@ -449,18 +449,46 @@ export const rawEpics = [
 
 export const mockUnsortedEpics = [
   {
+    title: 'Nov 10 2013 ~ Jun 01 2014; actual start date is Feb 1 2013',
+    originalStartDate: dateFromString('Feb 1 2013'),
+    startDate: dateFromString('Nov 10 2013'),
+    endDate: dateFromString('Jun 1, 2014'),
+  },
+  {
+    title: 'Oct 01 2013 ~ Nov 01 2013; actual due date is Nov 1 2014',
+    startDate: dateFromString('Oct 1 2013'),
+    originalEndDate: dateFromString('Nov 1 2014'),
+    endDate: dateFromString('Nov 1, 2013'),
+  },
+  {
+    title: 'Jan 01 2020 ~ Dec 01 2020; no fixed start date',
+    startDateUndefined: true,
+    startDate: dateFromString('Jan 1 2020'),
+    endDate: dateFromString('Dec 1 2020'),
+  },
+  {
+    title: 'Mar 01 2013 ~ Dec 01 2013; no fixed due date',
+    startDate: dateFromString('Mar 1 2013'),
+    endDateUndefined: true,
+    endDate: dateFromString('Dec 1 2013'),
+  },
+  {
+    title: 'Mar 12 2017 ~ Aug 20 2017',
     startDate: new Date(2017, 2, 12),
     endDate: new Date(2017, 7, 20),
   },
   {
+    title: 'Jun 08 2015 ~ Apr 01 2016',
     startDate: new Date(2015, 5, 8),
     endDate: new Date(2016, 3, 1),
   },
   {
+    title: 'Apr 12 2019 ~ Aug 30 2019',
     startDate: new Date(2019, 4, 12),
     endDate: new Date(2019, 7, 30),
   },
   {
+    title: 'Mar 17 2014 ~ Aug 15 2015',
     startDate: new Date(2014, 3, 17),
     endDate: new Date(2015, 7, 15),
   },

ee/spec/frontend/roadmap/utils/roadmap_utils_spec.js

@@ -8,7 +8,6 @@ import {
   extendTimeframeForWeeksView,
   extendTimeframeForAvailableWidth,
   getEpicsTimeframeRange,
-  assignDates,
   sortEpics,
 } from 'ee/roadmap/utils/roadmap_utils';
 
@@ -347,10 +346,14 @@ describe('sortEpics', () => {
   it('sorts epics list by startDate in ascending order when `sortedBy` param is `start_date_asc`', () => {
     const epics = mockUnsortedEpics.slice();
     const sortedOrder = [
-      new Date(2014, 3, 17),
-      new Date(2015, 5, 8),
-      new Date(2017, 2, 12),
-      new Date(2019, 4, 12),
+      'Jan 01 2020 ~ Dec 01 2020; no fixed start date',
+      'Nov 10 2013 ~ Jun 01 2014; actual start date is Feb 1 2013',
+      'Mar 01 2013 ~ Dec 01 2013; no fixed due date',
+      'Oct 01 2013 ~ Nov 01 2013; actual due date is Nov 1 2014',
+      'Mar 17 2014 ~ Aug 15 2015',
+      'Jun 08 2015 ~ Apr 01 2016',
+      'Mar 12 2017 ~ Aug 20 2017',
+      'Apr 12 2019 ~ Aug 30 2019',
     ];
 
     sortEpics(epics, 'start_date_asc');
@@ -358,17 +361,21 @@ describe('sortEpics', () => {
     expect(epics).toHaveLength(mockUnsortedEpics.length);
 
     epics.forEach((epic, index) => {
-      expect(epic.startDate.getTime()).toBe(sortedOrder[index].getTime());
+      expect(epic.title).toEqual(sortedOrder[index]);
     });
   });
 
   it('sorts epics list by startDate in descending order when `sortedBy` param is `start_date_desc`', () => {
     const epics = mockUnsortedEpics.slice();
     const sortedOrder = [
-      new Date(2019, 4, 12),
-      new Date(2017, 2, 12),
-      new Date(2015, 5, 8),
-      new Date(2014, 3, 17),
+      'Apr 12 2019 ~ Aug 30 2019',
+      'Mar 12 2017 ~ Aug 20 2017',
+      'Jun 08 2015 ~ Apr 01 2016',
+      'Mar 17 2014 ~ Aug 15 2015',
+      'Oct 01 2013 ~ Nov 01 2013; actual due date is Nov 1 2014',
+      'Mar 01 2013 ~ Dec 01 2013; no fixed due date',
+      'Nov 10 2013 ~ Jun 01 2014; actual start date is Feb 1 2013',
+      'Jan 01 2020 ~ Dec 01 2020; no fixed start date',
     ];
 
     sortEpics(epics, 'start_date_desc');
@@ -376,17 +383,21 @@ describe('sortEpics', () => {
     expect(epics).toHaveLength(mockUnsortedEpics.length);
 
     epics.forEach((epic, index) => {
-      expect(epic.startDate.getTime()).toBe(sortedOrder[index].getTime());
+      expect(epic.title).toEqual(sortedOrder[index]);
     });
   });
 
   it('sorts epics list by endDate in ascending order when `sortedBy` param is `end_date_asc`', () => {
     const epics = mockUnsortedEpics.slice();
     const sortedOrder = [
-      new Date(2015, 7, 15),
-      new Date(2016, 3, 1),
-      new Date(2017, 7, 20),
-      new Date(2019, 7, 30),
+      'Nov 10 2013 ~ Jun 01 2014; actual start date is Feb 1 2013',
+      'Oct 01 2013 ~ Nov 01 2013; actual due date is Nov 1 2014',
+      'Mar 17 2014 ~ Aug 15 2015',
+      'Jun 08 2015 ~ Apr 01 2016',
+      'Mar 12 2017 ~ Aug 20 2017',
+      'Apr 12 2019 ~ Aug 30 2019',
+      'Jan 01 2020 ~ Dec 01 2020; no fixed start date',
+      'Mar 01 2013 ~ Dec 01 2013; no fixed due date',
     ];
 
     sortEpics(epics, 'end_date_asc');
@@ -394,17 +405,21 @@ describe('sortEpics', () => {
     expect(epics).toHaveLength(mockUnsortedEpics.length);
 
     epics.forEach((epic, index) => {
-      expect(epic.endDate.getTime()).toBe(sortedOrder[index].getTime());
+      expect(epic.title).toEqual(sortedOrder[index]);
     });
   });
 
   it('sorts epics list by endDate in descending order when `sortedBy` param is `end_date_desc`', () => {
     const epics = mockUnsortedEpics.slice();
     const sortedOrder = [
-      new Date(2019, 7, 30),
-      new Date(2017, 7, 20),
-      new Date(2016, 3, 1),
-      new Date(2015, 7, 15),
+      'Mar 01 2013 ~ Dec 01 2013; no fixed due date',
+      'Jan 01 2020 ~ Dec 01 2020; no fixed start date',
+      'Apr 12 2019 ~ Aug 30 2019',
+      'Mar 12 2017 ~ Aug 20 2017',
+      'Jun 08 2015 ~ Apr 01 2016',
+      'Mar 17 2014 ~ Aug 15 2015',
+      'Oct 01 2013 ~ Nov 01 2013; actual due date is Nov 1 2014',
+      'Nov 10 2013 ~ Jun 01 2014; actual start date is Feb 1 2013',
     ];
 
     sortEpics(epics, 'end_date_desc');
@@ -412,118 +427,7 @@ describe('sortEpics', () => {
     expect(epics).toHaveLength(mockUnsortedEpics.length);
 
     epics.forEach((epic, index) => {
-      expect(epic.endDate.getTime()).toBe(sortedOrder[index].getTime());
+      expect(epic.title).toEqual(sortedOrder[index]);
     });
   });
 });
-
-describe('assignDates', () => {
-  const startDateProps = {
-    dateUndefined: 'startDateUndefined',
-    outOfRange: 'startDateOutOfRange',
-    originalDate: 'originalStartDate',
-    date: 'startDate',
-    proxyDate: new Date('1900'),
-  };
-  const endDateProps = {
-    dateUndefined: 'endDateUndefined',
-    outOfRange: 'endDateOutOfRange',
-    originalDate: 'originalEndDate',
-    date: 'endDate',
-    proxyDate: new Date('2200'),
-  };
-
-  it('returns proxyDate if startDate is undefined', () => {
-    const epic1 = { startDateUndefined: true };
-    const epic2 = { startDateUndefined: false };
-
-    let [aDate, bDate] = assignDates(epic1, epic2, startDateProps);
-
-    expect(aDate).toEqual(startDateProps.proxyDate);
-    expect(bDate).not.toEqual(startDateProps.proxyDate);
-
-    epic1.startDateUndefined = false;
-    epic2.startDateUndefined = true;
-    [aDate, bDate] = assignDates(epic1, epic2, startDateProps);
-
-    expect(aDate).not.toEqual(startDateProps.proxyDate);
-    expect(bDate).toEqual(startDateProps.proxyDate);
-  });
-
-  it('returns proxyDate if endDate is undefined', () => {
-    const epic1 = { endDateUndefined: true };
-    const epic2 = { endDateUndefined: false };
-
-    let [aDate, bDate] = assignDates(epic1, epic2, endDateProps);
-
-    expect(aDate).toEqual(endDateProps.proxyDate);
-    expect(bDate).not.toEqual(endDateProps.proxyDate);
-
-    epic1.endDateUndefined = false;
-    epic2.endDateUndefined = true;
-    [aDate, bDate] = assignDates(epic1, epic2, endDateProps);
-
-    expect(aDate).not.toEqual(endDateProps.proxyDate);
-    expect(bDate).toEqual(endDateProps.proxyDate);
-  });
-
-  it('assigns originalStartDate if date is out of range', () => {
-    const epic1 = {
-      startDateUndefined: false,
-      originalStartDate: new Date('2000'),
-      startDate: new Date('2010'),
-      startDateOutOfRange: true,
-    };
-    const epic2 = { ...epic1, originalStartDate: new Date('2005') };
-
-    const [aDate, bDate] = assignDates(epic1, epic2, startDateProps);
-
-    expect(aDate).toEqual(epic1.originalStartDate);
-    expect(bDate).toEqual(epic2.originalStartDate);
-  });
-
-  it('assigns originalEndDate if date is out of range', () => {
-    const epic1 = {
-      endDateUndefined: false,
-      originalEndDate: new Date('2000'),
-      endDate: new Date('2010'),
-      endDateOutOfRange: true,
-    };
-    const epic2 = { ...epic1, originalEndDate: new Date('2005') };
-
-    const [aDate, bDate] = assignDates(epic1, epic2, endDateProps);
-
-    expect(aDate).toEqual(epic1.originalEndDate);
-    expect(bDate).toEqual(epic2.originalEndDate);
-  });
-
-  it('assigns startDate if date is in the range', () => {
-    const epic1 = {
-      startDateUndefined: false,
-      originalStartDate: new Date('2000'),
-      startDate: new Date('2010'),
-      startDateOutOfRange: false,
-    };
-    const epic2 = { ...epic1, startDate: new Date('2005') };
-
-    const [aDate, bDate] = assignDates(epic1, epic2, startDateProps);
-
-    expect(aDate).toEqual(epic1.startDate);
-    expect(bDate).toEqual(epic2.startDate);
-  });
-
-  it('assigns endDate if date is in the range', () => {
-    const epic1 = {
-      endDateUndefined: false,
-      originalEndDate: new Date('2000'),
-      endDate: new Date('2010'),
-      endDateOutOfRange: false,
-    };
-    const epic2 = { ...epic1, endDate: new Date('2005') };
-
-    const [aDate, bDate] = assignDates(epic1, epic2, endDateProps);
-
-    expect(aDate).toEqual(epic1.endDate);
-    expect(bDate).toEqual(epic2.endDate);
-  });
-});