From faffc358604d8ff021722502ee329fe03f8c23e9 Mon Sep 17 00:00:00 2001
From: Jimmy Domagala-Tang <Jimmy.Domagala-Tang@overleaf.com>
Date: Thu, 31 Aug 2023 11:22:33 -0400
Subject: [PATCH] Merge pull request #14507 from
 overleaf/ds-upper-highlight-version

added conditional styling for upper selected version

GitOrigin-RevId: 3474058c5fdb204d9c8dd6538c79d2074d49222b
---
 .../change-list/history-version-details.tsx   |  4 +-
 .../change-list/history-version.tsx           |  5 +-
 .../features/history/utils/history-details.ts | 23 +++---
 .../history/components/change-list.spec.tsx   | 81 +++++++++++--------
 .../features/history/fixtures/updates.ts      | 64 +++++++++++++--
 5 files changed, 120 insertions(+), 57 deletions(-)

diff --git a/services/web/frontend/js/features/history/components/change-list/history-version-details.tsx b/services/web/frontend/js/features/history/components/change-list/history-version-details.tsx
index 48480f84e9..c0a2722ac5 100644
--- a/services/web/frontend/js/features/history/components/change-list/history-version-details.tsx
+++ b/services/web/frontend/js/features/history/components/change-list/history-version-details.tsx
@@ -37,7 +37,9 @@ function HistoryVersionDetails({
     <div
       className={classnames('history-version-details clearfix', {
         'history-version-selected':
-          selected === 'selectedEdge' || selected === 'selected',
+          selected === 'upperSelected' ||
+          selected === 'lowerSelected' ||
+          selected === 'selected',
         'history-version-within-selected': selected === 'withinSelected',
         'history-version-selectable': selectable,
       })}
diff --git a/services/web/frontend/js/features/history/components/change-list/history-version.tsx b/services/web/frontend/js/features/history/components/change-list/history-version.tsx
index 26f7cc258b..fcb5f053bf 100644
--- a/services/web/frontend/js/features/history/components/change-list/history-version.tsx
+++ b/services/web/frontend/js/features/history/components/change-list/history-version.tsx
@@ -178,7 +178,6 @@ function HistoryVersion({
   }, [closeDropdownForItem, update])
 
   const updateRange = updateRangeForUpdate(update)
-
   return (
     <>
       {popover}
@@ -187,7 +186,7 @@ function HistoryVersion({
           className={classNames({
             'history-version-divider-container': true,
             'version-element-within-selected ':
-              selected === 'withinSelected' || selected === 'selectedEdge',
+              selected === 'withinSelected' || selected === 'lowerSelected',
           })}
         >
           <hr className="history-version-divider" />
@@ -197,7 +196,7 @@ function HistoryVersion({
         <div
           className={classNames({
             'version-element-within-selected ':
-              selected === 'withinSelected' || selected === 'selectedEdge',
+              selected === 'withinSelected' || selected === 'lowerSelected',
           })}
         >
           <time className="history-version-day">
diff --git a/services/web/frontend/js/features/history/utils/history-details.ts b/services/web/frontend/js/features/history/utils/history-details.ts
index 46327707e6..bdd89167c1 100644
--- a/services/web/frontend/js/features/history/utils/history-details.ts
+++ b/services/web/frontend/js/features/history/utils/history-details.ts
@@ -38,7 +38,8 @@ export const getProjectOpDoc = (projectOp: ProjectOp) => {
 }
 
 export type ItemSelectionState =
-  | 'selectedEdge'
+  | 'upperSelected'
+  | 'lowerSelected'
   | 'withinSelected'
   | 'aboveSelected'
   | 'belowSelected'
@@ -73,11 +74,11 @@ export function isVersionSelected(
 
       // Condition for selectedEdge when the comparing versions are from labels list
       if (fromV === toV) {
-        if (
-          toV === selection.updateRange.fromV ||
-          fromV === selection.updateRange.toV
-        ) {
-          return 'selectedEdge'
+        if (fromV === selection.updateRange.toV) {
+          return 'upperSelected'
+        }
+        if (toV === selection.updateRange.fromV) {
+          return 'lowerSelected'
         }
       }
 
@@ -90,11 +91,11 @@ export function isVersionSelected(
         return 'belowSelected'
       }
 
-      if (
-        fromV === selection.updateRange.fromV ||
-        toV === selection.updateRange.toV
-      ) {
-        return 'selectedEdge'
+      if (toV === selection.updateRange.toV) {
+        return 'upperSelected'
+      }
+      if (fromV === selection.updateRange.fromV) {
+        return 'lowerSelected'
       }
     } else if (toV === selection.updateRange.toV) {
       // single version mode
diff --git a/services/web/test/frontend/features/history/components/change-list.spec.tsx b/services/web/test/frontend/features/history/components/change-list.spec.tsx
index 069bc642fb..0221b3a4b7 100644
--- a/services/web/test/frontend/features/history/components/change-list.spec.tsx
+++ b/services/web/test/frontend/features/history/components/change-list.spec.tsx
@@ -102,10 +102,10 @@ describe('change list', function () {
 
       cy.findByLabelText(/all history/i).click({ force: true })
       cy.findAllByTestId('history-version-details').as('details')
-      cy.get('@details').should('have.length', 3)
-      // 1st details entry
+      cy.get('@details').should('have.length', 5)
+      // start with 2nd details entry, as first has no tags
       cy.get('@details')
-        .eq(0)
+        .eq(1)
         .within(() => {
           cy.findAllByTestId('history-version-badge').as('tags')
         })
@@ -118,15 +118,15 @@ describe('change list', function () {
           cy.findByRole('button', { name: /delete/i })
         })
       )
-      // 2nd details entry
-      cy.get('@details')
-        .eq(1)
-        .within(() => {
-          cy.findAllByTestId('history-version-badge').should('have.length', 0)
-        })
       // 3rd details entry
       cy.get('@details')
         .eq(2)
+        .within(() => {
+          cy.findAllByTestId('history-version-badge').should('have.length', 0)
+        })
+      // 4th details entry
+      cy.get('@details')
+        .eq(3)
         .within(() => {
           cy.findAllByTestId('history-version-badge').as('tags')
         })
@@ -141,9 +141,10 @@ describe('change list', function () {
       )
       cy.findByLabelText(/labels/i).click({ force: true })
       cy.findAllByTestId('history-version-details').as('details')
-      cy.get('@details').should('have.length', 2)
+      // first details on labels is always "current version", start testing on second
+      cy.get('@details').should('have.length', 3)
       cy.get('@details')
-        .eq(0)
+        .eq(1)
         .within(() => {
           cy.findAllByTestId('history-version-badge').as('tags')
         })
@@ -151,7 +152,7 @@ describe('change list', function () {
       cy.get('@tags').eq(0).should('contain.text', 'tag-2')
       cy.get('@tags').eq(1).should('contain.text', 'tag-1')
       cy.get('@details')
-        .eq(1)
+        .eq(2)
         .within(() => {
           cy.findAllByTestId('history-version-badge').as('tags')
         })
@@ -174,7 +175,7 @@ describe('change list', function () {
       cy.findByLabelText(/all history/i).click({ force: true })
 
       const labelToDelete = 'tag-2'
-      cy.findAllByTestId('history-version-details').eq(0).as('details')
+      cy.findAllByTestId('history-version-details').eq(1).as('details')
       cy.get('@details').within(() => {
         cy.findAllByTestId('history-version-badge').eq(0).as('tag')
       })
@@ -236,7 +237,7 @@ describe('change list', function () {
       const stub = cy.stub().as('diffStub')
       cy.intercept('GET', '/project/*/filetree/diff*', stub).as('diff')
 
-      cy.findAllByTestId('history-version-details').eq(1).as('details')
+      cy.findAllByTestId('history-version-details').eq(2).as('details')
       cy.get('@details').click() // 1st click
       cy.wait('@diff')
       cy.get('@details').click() // 2nd click
@@ -263,7 +264,7 @@ describe('change list', function () {
 
     it('shows the date of the version', function () {
       cy.findAllByTestId('history-version-details')
-        .eq(0)
+        .eq(1)
         .within(() => {
           cy.findByTestId('history-version-metadata-time').should(
             'have.text',
@@ -274,7 +275,7 @@ describe('change list', function () {
 
     it('shows change action', function () {
       cy.findAllByTestId('history-version-details')
-        .eq(0)
+        .eq(1)
         .within(() => {
           cy.findByTestId('history-version-change-action').should(
             'have.text',
@@ -285,22 +286,22 @@ describe('change list', function () {
 
     it('shows changed document name', function () {
       cy.findAllByTestId('history-version-details')
-        .eq(1)
+        .eq(2)
         .within(() => {
           cy.findByTestId('history-version-change-doc').should(
             'have.text',
-            updates.updates[1].pathnames[0]
+            updates.updates[2].pathnames[0]
           )
         })
     })
 
     it('shows users', function () {
       cy.findAllByTestId('history-version-details')
-        .eq(0)
+        .eq(1)
         .within(() => {
           cy.findByTestId('history-version-metadata-users')
             .should('contain.text', 'You')
-            .and('contain.text', updates.updates[0].meta.users[1].first_name)
+            .and('contain.text', updates.updates[1].meta.users[1].first_name)
         })
     })
   })
@@ -341,35 +342,45 @@ describe('change list', function () {
         })
       cy.findByLabelText(/all history/i).click({ force: true })
       cy.findAllByTestId('history-version-details').should($versions => {
-        const [first, ...rest] = Array.from($versions)
-        expect(first.dataset.selected === 'selected').to.be.true
+        const [selected, ...rest] = Array.from($versions)
+        expect(selected).to.have.attr('data-selected', 'selected')
         expect(
-          rest.every(
-            version =>
-              version.dataset.selected === 'belowSelected' ||
-              version.dataset.selected === 'aboveSelected'
-          )
+          rest.every(version => version.dataset.selected === 'belowSelected')
         ).to.be.true
       })
     })
     it('opens the compare drop down and compares with selected version', function () {
       cy.findByLabelText(/all history/i).click({ force: true })
       cy.findAllByTestId('history-version-details')
-        .eq(2)
+        .eq(3)
         .within(() => {
           cy.findByRole('button', {
             name: /compare from this version/i,
           }).click()
         })
-      cy.findByRole('button', { name: /compare drop down/i }).click()
-      cy.findByRole('button', { name: /compare up to this version/i }).click()
+
+      cy.findAllByTestId('history-version-details')
+        .eq(1)
+        .within(() => {
+          cy.findByRole('button', { name: /compare drop down/i }).click()
+          cy.findByRole('button', {
+            name: /compare up to this version/i,
+          }).click()
+        })
 
       cy.findAllByTestId('history-version-details').should($versions => {
-        const [first, ...rest] = Array.from($versions)
-        expect(first).to.have.attr('data-selected', 'aboveSelected')
-        rest.forEach(version =>
-          expect(version).to.have.attr('data-selected', 'selectedEdge')
-        )
+        const [
+          aboveSelected,
+          upperSelected,
+          withinSelected,
+          lowerSelected,
+          belowSelected,
+        ] = Array.from($versions)
+        expect(aboveSelected).to.have.attr('data-selected', 'aboveSelected')
+        expect(upperSelected).to.have.attr('data-selected', 'upperSelected')
+        expect(withinSelected).to.have.attr('data-selected', 'withinSelected')
+        expect(lowerSelected).to.have.attr('data-selected', 'lowerSelected')
+        expect(belowSelected).to.have.attr('data-selected', 'belowSelected')
       })
     })
   })
diff --git a/services/web/test/frontend/features/history/fixtures/updates.ts b/services/web/test/frontend/features/history/fixtures/updates.ts
index 7484145189..667fe6b494 100644
--- a/services/web/test/frontend/features/history/fixtures/updates.ts
+++ b/services/web/test/frontend/features/history/fixtures/updates.ts
@@ -3,8 +3,33 @@ import { USER_ID, USER_EMAIL } from '../../../helpers/editor-providers'
 export const updates = {
   updates: [
     {
-      fromV: 3,
-      toV: 4,
+      fromV: 5,
+      toV: 6,
+      meta: {
+        users: [
+          {
+            first_name: 'testuser',
+            last_name: '',
+            email: USER_EMAIL,
+            id: USER_ID,
+          },
+          {
+            first_name: 'john.doe',
+            last_name: '',
+            email: 'john.doe@test.com',
+            id: '631710ab1094c5002647184e',
+          },
+        ],
+        start_ts: 1681220036519,
+        end_ts: 1681220036619,
+      },
+      labels: [],
+      pathnames: ['name.tex'],
+      project_ops: [],
+    },
+    {
+      fromV: 4,
+      toV: 5,
       meta: {
         users: [
           {
@@ -43,8 +68,8 @@ export const updates = {
       project_ops: [{ add: { pathname: 'name.tex' }, atV: 3 }],
     },
     {
-      fromV: 1,
-      toV: 3,
+      fromV: 2,
+      toV: 4,
       meta: {
         users: [
           {
@@ -62,8 +87,8 @@ export const updates = {
       project_ops: [],
     },
     {
-      fromV: 0,
-      toV: 1,
+      fromV: 1,
+      toV: 2,
       meta: {
         users: [
           {
@@ -92,8 +117,33 @@ export const updates = {
           created_at: '2023-04-12T14:15:20.814Z',
         },
       ],
+      pathnames: ['main.tex'],
+      project_ops: [],
+    },
+    {
+      fromV: 0,
+      toV: 1,
+      meta: {
+        users: [
+          {
+            first_name: 'testuser',
+            last_name: '',
+            email: USER_EMAIL,
+            id: USER_ID,
+          },
+          {
+            first_name: 'john.doe',
+            last_name: '',
+            email: 'john.doe@test.com',
+            id: '631710ab1094c5002647184e',
+          },
+        ],
+        start_ts: 1669218226500,
+        end_ts: 1669218226600,
+      },
+      labels: [],
       pathnames: [],
-      project_ops: [{ add: { pathname: 'main.tex' }, atV: 0 }],
+      project_ops: [{ add: { pathname: 'main.tex' }, atV: 3 }],
     },
   ],
 }