Merge pull request #12621 from overleaf/ii-history-react-delete-labels

[web] Delete history tags

GitOrigin-RevId: 0a2846bbb3e99ef632b9192a9f8c04f702645506

services/web/frontend/extracted-translations.json

@@ -353,6 +353,9 @@
   "hide_document_preamble": "",
   "hide_outline": "",
   "history": "",
+  "history_are_you_sure_delete_label": "",
+  "history_delete_label": "",
+  "history_deleting_label": "",
   "history_entry_origin_dropbox": "",
   "history_entry_origin_git": "",
   "history_entry_origin_github": "",

services/web/frontend/js/features/history/components/change-list/history-version.tsx

@@ -22,7 +22,10 @@ function HistoryVersion({ update }: HistoryEntryProps) {
           {relativeDate(update.meta.end_ts)}
         </time>
       )}
-      <div className="history-version-details">
+      <div
+        className="history-version-details"
+        data-testid="history-version-details"
+      >
         <time className="history-version-metadata-time">
           <b>{formatTime(update.meta.end_ts, 'Do MMMM, h:mm a')}</b>
         </time>

services/web/frontend/js/features/history/components/change-list/labels-list.tsx

@@ -27,7 +27,11 @@ function LabelsList() {
   return (
     <>
       {versionWithLabels.map(({ version, labels }) => (
-        <div key={version} className="history-version-details">
+        <div
+          key={version}
+          className="history-version-details"
+          data-testid="history-version-details"
+        >
           {labels.map(label => (
             <Fragment key={label.id}>
               <TagTooltip

services/web/frontend/js/features/history/components/change-list/tag-tooltip.tsx

@@ -1,9 +1,14 @@
+import { useState } from 'react'
 import { useTranslation } from 'react-i18next'
+import { Alert, Modal } from 'react-bootstrap'
+import { useHistoryContext } from '../../context/history-context'
 import Icon from '../../../../shared/components/icon'
 import Tooltip from '../../../../shared/components/tooltip'
 import Badge from '../../../../shared/components/badge'
-import { useHistoryContext } from '../../context/history-context'
+import AccessibleModal from '../../../../shared/components/accessible-modal'
-import { isPseudoLabel } from '../../utils/label'
+import useAsync from '../../../../shared/hooks/use-async'
+import { deleteJSON } from '../../../../infrastructure/fetch-json'
+import { isLabel, isPseudoLabel, loadLabels } from '../../utils/label'
 import { formatDate } from '../../../../utils/dates'
 import { LoadedLabel } from '../../services/types/label'
 
@@ -14,30 +19,128 @@ type TagProps = {
 
 function Tag({ label, currentUserId, ...props }: TagProps) {
   const { t } = useTranslation()
+  const [showDeleteModal, setShowDeleteModal] = useState(false)
+  const { projectId, updates, setUpdates, labels, setLabels } =
+    useHistoryContext()
+  const { isLoading, isSuccess, isError, error, runAsync } = useAsync()
   const isPseudoCurrentStateLabel = isPseudoLabel(label)
   const isOwnedByCurrentUser = !isPseudoCurrentStateLabel
     ? label.user_id === currentUserId
     : null
 
-  const handleDelete = (e: React.MouseEvent, label: LoadedLabel) => {
+  const showConfirmationModal = (e: React.MouseEvent) => {
     e.stopPropagation()
+    setShowDeleteModal(true)
+  }
+
+  const handleModalExited = () => {
+    if (!isSuccess) return
+
+    const tempUpdates = [...updates]
+    for (const [i, update] of tempUpdates.entries()) {
+      if (update.toV === label.version) {
+        tempUpdates[i] = {
+          ...update,
+          labels: update.labels.filter(({ id }) => id !== label.id),
+        }
+        break
+      }
+    }
+
+    setUpdates(tempUpdates)
+
+    if (labels) {
+      const nonPseudoLabels = labels.filter(isLabel)
+      const filteredLabels = nonPseudoLabels.filter(({ id }) => id !== label.id)
+      setLabels(loadLabels(filteredLabels, tempUpdates[0].toV))
+    }
+
+    setShowDeleteModal(false)
+
+    // TODO _handleHistoryUIStateChange
+  }
+
+  const localDeleteHandler = () => {
+    runAsync(deleteJSON(`/project/${projectId}/labels/${label.id}`))
+      .then(() => {
+        setShowDeleteModal(false)
+      })
+      .catch(console.error)
+  }
+
+  const responseError = error as unknown as {
+    response: Response
+    data?: {
+      message?: string
+    }
   }
 
   return (
-    <Badge
+    <>
-      prepend={<Icon type="tag" fw />}
+      <Badge
-      onClose={e => handleDelete(e, label)}
+        prepend={<Icon type="tag" fw />}
-      showCloseButton={Boolean(
+        onClose={showConfirmationModal}
-        isOwnedByCurrentUser && !isPseudoCurrentStateLabel
+        closeButton={Boolean(
+          isOwnedByCurrentUser && !isPseudoCurrentStateLabel
+        )}
+        closeBtnProps={{ 'aria-label': t('delete') }}
+        className="history-version-badge"
+        data-testid="history-version-badge"
+        {...props}
+      >
+        {isPseudoCurrentStateLabel
+          ? t('history_label_project_current_state')
+          : label.comment}
+      </Badge>
+      {!isPseudoCurrentStateLabel && (
+        <AccessibleModal
+          show={showDeleteModal}
+          onExited={handleModalExited}
+          onHide={() => setShowDeleteModal(false)}
+          id="delete-history-label"
+        >
+          <Modal.Header>
+            <Modal.Title>{t('history_delete_label')}</Modal.Title>
+          </Modal.Header>
+          <Modal.Body>
+            {isError ? (
+              responseError.response.status === 400 &&
+              responseError?.data?.message ? (
+                <Alert bsStyle="danger">{responseError.data.message}</Alert>
+              ) : (
+                <Alert bsStyle="danger">
+                  {t('generic_something_went_wrong')}
+                </Alert>
+              )
+            ) : null}
+            <p>
+              {t('history_are_you_sure_delete_label')}&nbsp;
+              <strong>"{label.comment}"</strong>?
+            </p>
+          </Modal.Body>
+          <Modal.Footer>
+            <button
+              type="button"
+              className="btn btn-secondary"
+              disabled={isLoading}
+              onClick={() => setShowDeleteModal(false)}
+            >
+              {t('cancel')}
+            </button>
+            <button
+              type="button"
+              className="btn btn-danger"
+              disabled={isLoading}
+              onClick={localDeleteHandler}
+            >
+              {isLoading
+                ? t('history_deleting_label')
+                : t('history_delete_label')}
+            </button>
+          </Modal.Footer>
+        </AccessibleModal>
       )}
-      closeBtnProps={{ 'aria-label': t('delete') }}
+    </>
-      className="history-version-badge"
-      {...props}
-    >
-      {isPseudoCurrentStateLabel
-        ? t('history_label_project_current_state')
-        : label.comment}
-    </Badge>
   )
 }
 
@@ -60,7 +163,6 @@ function TagTooltip({ label, currentUserId, showTooltip }: LabelBadgesProps) {
 
   return showTooltip && !isPseudoCurrentStateLabel ? (
     <Tooltip
-      key={label.id}
       description={
         <div className="history-version-label-tooltip">
           <div className="history-version-label-tooltip-row">

services/web/frontend/js/features/history/context/history-context.tsx

@@ -97,6 +97,9 @@ function useHistory() {
       setUpdates(updates.concat(loadedUpdates))
 
       // TODO first load
+      // if (firstLoad) {
+      //   _handleHistoryUIStateChange()
+      // }
     }
 
     if (atEnd) return
@@ -192,9 +195,11 @@ function useHistory() {
       isLoading,
       freeHistoryLimitHit,
       labels,
+      setLabels,
       loadingFileTree,
       nextBeforeTimestamp,
       updates,
+      setUpdates,
       userHasFullFeature,
       projectId,
       fileSelection,
@@ -208,9 +213,11 @@ function useHistory() {
       isLoading,
       freeHistoryLimitHit,
       labels,
+      setLabels,
       loadingFileTree,
       nextBeforeTimestamp,
       updates,
+      setUpdates,
       userHasFullFeature,
       projectId,
       fileSelection,

services/web/frontend/js/features/history/context/types/history-context-value.ts

@@ -5,6 +5,9 @@ import { FileSelection } from '../../services/types/file'
 
 export type HistoryContextValue = {
   updates: LoadedUpdate[]
+  setUpdates: React.Dispatch<
+    React.SetStateAction<HistoryContextValue['updates']>
+  >
   nextBeforeTimestamp: number | undefined
   atEnd: boolean
   userHasFullFeature: boolean | undefined
@@ -12,6 +15,7 @@ export type HistoryContextValue = {
   isLoading: boolean
   error: Nullable<unknown>
   labels: Nullable<LoadedLabel[]>
+  setLabels: React.Dispatch<React.SetStateAction<HistoryContextValue['labels']>>
   loadingFileTree: boolean
   projectId: string
   fileSelection: FileSelection | null

services/web/frontend/js/features/history/utils/label.ts

@@ -12,6 +12,10 @@ export const isPseudoLabel = (
   return (label as PseudoCurrentStateLabel).isPseudoCurrentStateLabel === true
 }
 
+export const isLabel = (label: LoadedLabel): label is Label => {
+  return !isPseudoLabel(label)
+}
+
 const sortLabelsByVersionAndDate = (labels: LoadedLabel[]) => {
   return orderBy(
     labels,

services/web/frontend/js/shared/components/badge.tsx

@@ -7,7 +7,7 @@ type BadgeProps = MergeAndOverride<
     prepend?: React.ReactNode
     children: React.ReactNode
     className?: string
-    showCloseButton?: boolean
+    closeButton?: boolean
     onClose?: (e: React.MouseEvent<HTMLButtonElement>) => void
     closeBtnProps?: React.ComponentProps<'button'>
   }
@@ -17,7 +17,7 @@ function Badge({
   prepend,
   children,
   className,
-  showCloseButton = false,
+  closeButton = false,
   onClose,
   closeBtnProps,
   ...rest
@@ -26,7 +26,7 @@ function Badge({
     <span className={classnames('badge-new', className)} {...rest}>
       {prepend}
       <span className="badge-new-comment">{children}</span>
-      {showCloseButton && (
+      {closeButton && (
         <button
           type="button"
           className="badge-new-close"

services/web/frontend/stories/badge.stories.tsx

@@ -0,0 +1,53 @@
+import Badge from '../js/shared/components/badge'
+import Icon from '../js/shared/components/icon'
+
+type Args = React.ComponentProps<typeof Badge>
+
+export const NewBadge = (args: Args) => {
+  return <Badge {...args} />
+}
+
+export const NewBadgePrepend = (args: Args) => {
+  return <Badge prepend={<Icon type="tag" fw />} {...args} />
+}
+
+export const NewBadgeWithCloseButton = (args: Args) => {
+  return (
+    <Badge
+      prepend={<Icon type="tag" fw />}
+      closeButton
+      onClose={() => alert('Close triggered!')}
+      {...args}
+    />
+  )
+}
+
+export default {
+  title: 'Shared / Components / Badge',
+  component: Badge,
+  args: {
+    children: 'content',
+  },
+  argTypes: {
+    prepend: {
+      table: {
+        disable: true,
+      },
+    },
+    closeButton: {
+      table: {
+        disable: true,
+      },
+    },
+    onClose: {
+      table: {
+        disable: true,
+      },
+    },
+    closeBtnProps: {
+      table: {
+        disable: true,
+      },
+    },
+  },
+}

services/web/test/frontend/features/history/components/change-list.spec.tsx

@@ -0,0 +1,197 @@
+import { useState } from 'react'
+import ToggleSwitch from '../../../../../frontend/js/features/history/components/change-list/toggle-switch'
+import ChangeList from '../../../../../frontend/js/features/history/components/change-list/change-list'
+import { EditorProviders } from '../../../helpers/editor-providers'
+import { HistoryProvider } from '../../../../../frontend/js/features/history/context/history-context'
+import { updates } from '../fixtures/updates'
+import { labels } from '../fixtures/labels'
+
+const mountChangeList = (scope: Record<string, unknown> = {}) => {
+  cy.mount(
+    <EditorProviders scope={scope}>
+      <HistoryProvider>
+        <div style={{ display: 'flex', justifyContent: 'center' }}>
+          <div className="history-react">
+            <ChangeList />
+          </div>
+        </div>
+      </HistoryProvider>
+    </EditorProviders>
+  )
+}
+
+describe('change list', function () {
+  describe('toggle switch', function () {
+    it('renders switch buttons', function () {
+      cy.mount(<ToggleSwitch labelsOnly={false} setLabelsOnly={() => {}} />)
+
+      cy.findByLabelText(/all history/i)
+      cy.findByLabelText(/labels/i)
+    })
+
+    it('toggles "all history" and "labels" buttons', function () {
+      function ToggleSwitchWrapped({ labelsOnly }: { labelsOnly: boolean }) {
+        const [labelsOnlyLocal, setLabelsOnlyLocal] = useState(labelsOnly)
+        return (
+          <ToggleSwitch
+            labelsOnly={labelsOnlyLocal}
+            setLabelsOnly={setLabelsOnlyLocal}
+          />
+        )
+      }
+
+      cy.mount(<ToggleSwitchWrapped labelsOnly={false} />)
+
+      cy.findByLabelText(/all history/i).as('all-history')
+      cy.findByLabelText(/labels/i).as('labels')
+      cy.get('@all-history').should('be.checked')
+      cy.get('@labels').should('not.be.checked')
+      cy.get('@labels').click({ force: true })
+      cy.get('@all-history').should('not.be.checked')
+      cy.get('@labels').should('be.checked')
+    })
+  })
+
+  describe('tags', function () {
+    const scope = {
+      ui: { view: 'history', pdfLayout: 'sideBySide', chatOpen: true },
+    }
+
+    const waitForData = () => {
+      cy.wait('@updates')
+      cy.wait('@labels')
+      cy.wait('@diff')
+    }
+
+    beforeEach(function () {
+      cy.intercept('GET', '/project/*/updates*', {
+        body: updates,
+      }).as('updates')
+      cy.intercept('GET', '/project/*/labels', {
+        body: labels,
+      }).as('labels')
+      cy.intercept('GET', '/project/*/filetree/diff*', {
+        body: { diff: [{ pathname: 'main.tex' }, { pathname: 'name.tex' }] },
+      }).as('diff')
+    })
+
+    it('renders tags', function () {
+      mountChangeList(scope)
+      waitForData()
+
+      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')
+        .eq(0)
+        .within(() => {
+          cy.findAllByTestId('history-version-badge').as('tags')
+        })
+      cy.get('@tags').should('have.length', 2)
+      cy.get('@tags').eq(0).should('contain.text', 'tag-2')
+      cy.get('@tags').eq(1).should('contain.text', 'tag-1')
+      // should have delete buttons
+      cy.get('@tags').each(tag =>
+        cy.wrap(tag).within(() => {
+          cy.findByRole('button', { name: /delete/i })
+        })
+      )
+      // 2nd details entry
+      cy.get('@details')
+        .eq(1)
+        .within(() => {
+          cy.findAllByTestId('history-version-badge').as('tags')
+        })
+      cy.get('@tags').should('have.length', 2)
+      cy.get('@tags').eq(0).should('contain.text', 'tag-4')
+      cy.get('@tags').eq(1).should('contain.text', 'tag-3')
+      // should not have delete buttons
+      cy.get('@tags').each(tag =>
+        cy.wrap(tag).within(() => {
+          cy.findByRole('button', { name: /delete/i }).should('not.exist')
+        })
+      )
+      // 3rd details entry
+      cy.get('@details')
+        .eq(2)
+        .within(() => {
+          cy.findAllByTestId('history-version-badge').should('have.length', 0)
+        })
+      cy.findByLabelText(/labels/i).click({ force: true })
+      cy.findAllByTestId('history-version-details').as('details')
+      cy.get('@details').should('have.length', 2)
+      cy.get('@details')
+        .eq(0)
+        .within(() => {
+          cy.findAllByTestId('history-version-badge').as('tags')
+        })
+      cy.get('@tags').should('have.length', 2)
+      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)
+        .within(() => {
+          cy.findAllByTestId('history-version-badge').as('tags')
+        })
+      cy.get('@tags').should('have.length', 3)
+      cy.get('@tags').eq(0).should('contain.text', 'tag-5')
+      cy.get('@tags').eq(1).should('contain.text', 'tag-4')
+      cy.get('@tags').eq(2).should('contain.text', 'tag-3')
+    })
+
+    it('deletes tag', function () {
+      mountChangeList(scope)
+      waitForData()
+
+      cy.findByLabelText(/all history/i).click({ force: true })
+
+      const labelToDelete = 'tag-2'
+      cy.findAllByTestId('history-version-details').eq(0).as('details')
+      cy.get('@details').within(() => {
+        cy.findAllByTestId('history-version-badge').eq(0).as('tag')
+      })
+      cy.get('@tag').should('contain.text', labelToDelete)
+      cy.get('@tag').within(() => {
+        cy.findByRole('button', { name: /delete/i }).as('delete-btn')
+      })
+      cy.get('@delete-btn').click()
+      cy.findByRole('dialog').as('modal')
+      cy.get('@modal').within(() => {
+        cy.findByRole('heading', { name: /delete label/i })
+      })
+      cy.get('@modal').contains(
+        new RegExp(
+          `are you sure you want to delete the following label "${labelToDelete}"?`,
+          'i'
+        )
+      )
+      cy.get('@modal').within(() => {
+        cy.findByRole('button', { name: /cancel/i }).click()
+      })
+      cy.findByRole('dialog').should('not.exist')
+      cy.get('@delete-btn').click()
+      cy.findByRole('dialog').as('modal')
+      cy.intercept('DELETE', '/project/*/labels/*', {
+        statusCode: 500,
+      }).as('delete')
+      cy.get('@modal').within(() => {
+        cy.findByRole('button', { name: /delete/i }).click()
+      })
+      cy.wait('@delete')
+      cy.get('@modal').within(() => {
+        cy.findByRole('alert').within(() => {
+          cy.contains(/sorry, something went wrong/i)
+        })
+      })
+      cy.intercept('DELETE', '/project/*/labels/*', {
+        statusCode: 204,
+      }).as('delete')
+      cy.get('@modal').within(() => {
+        cy.findByRole('button', { name: /delete/i }).click()
+      })
+      cy.wait('@delete')
+      cy.findByText(labelToDelete).should('not.exist')
+    })
+  })
+})

services/web/test/frontend/features/history/components/chnage-list.spec.tsx

@@ -1,35 +0,0 @@
-import { useState } from 'react'
-import ToggleSwitch from '../../../../../frontend/js/features/history/components/change-list/toggle-switch'
-
-describe('change list', function () {
-  describe('toggle switch', function () {
-    it('renders switch buttons', function () {
-      cy.mount(<ToggleSwitch labelsOnly={false} setLabelsOnly={() => {}} />)
-
-      cy.findByLabelText(/all history/i)
-      cy.findByLabelText(/labels/i)
-    })
-
-    it('toggles "all history" and "labels" buttons', function () {
-      function ToggleSwitchWrapped({ labelsOnly }: { labelsOnly: boolean }) {
-        const [labelsOnlyLocal, setLabelsOnlyLocal] = useState(labelsOnly)
-        return (
-          <ToggleSwitch
-            labelsOnly={labelsOnlyLocal}
-            setLabelsOnly={setLabelsOnlyLocal}
-          />
-        )
-      }
-
-      cy.mount(<ToggleSwitchWrapped labelsOnly={false} />)
-
-      cy.findByLabelText(/all history/i).as('all-history')
-      cy.findByLabelText(/labels/i).as('labels')
-      cy.get('@all-history').should('be.checked')
-      cy.get('@labels').should('not.be.checked')
-      cy.get('@labels').click({ force: true })
-      cy.get('@all-history').should('not.be.checked')
-      cy.get('@labels').should('be.checked')
-    })
-  })
-})

services/web/test/frontend/features/history/fixtures/labels.ts

@@ -0,0 +1,44 @@
+import { USER_ID } from '../../../helpers/editor-providers'
+
+export const labels = [
+  {
+    id: '643561cdfa2b2beac88f0024',
+    comment: 'tag-1',
+    version: 4,
+    user_id: USER_ID,
+    created_at: '2023-04-11T13:34:05.856Z',
+    user_display_name: 'john.doe',
+  },
+  {
+    id: '643561d1fa2b2beac88f0025',
+    comment: 'tag-2',
+    version: 4,
+    user_id: USER_ID,
+    created_at: '2023-04-11T13:34:09.280Z',
+    user_display_name: 'john.doe',
+  },
+  {
+    id: '6436bcf630293cb49e7f13a4',
+    comment: 'tag-3',
+    version: 3,
+    user_id: '631710ab1094c5002647184e',
+    created_at: '2023-04-12T14:15:18.892Z',
+    user_display_name: 'bobby.lapointe',
+  },
+  {
+    id: '6436bcf830293cb49e7f13a5',
+    comment: 'tag-4',
+    version: 3,
+    user_id: '631710ab1094c5002647184e',
+    created_at: '2023-04-12T14:15:20.814Z',
+    user_display_name: 'bobby.lapointe',
+  },
+  {
+    id: '6436bcfb30293cb49e7f13a6',
+    comment: 'tag-5',
+    version: 3,
+    user_id: '631710ab1094c5002647184e',
+    created_at: '2023-04-12T14:15:23.481Z',
+    user_display_name: 'bobby.lapointe',
+  },
+]

services/web/test/frontend/features/history/fixtures/updates.ts

@@ -0,0 +1,93 @@
+import { USER_ID } from '../../../helpers/editor-providers'
+
+export const updates = {
+  updates: [
+    {
+      fromV: 3,
+      toV: 4,
+      meta: {
+        users: [
+          {
+            first_name: 'john.doe',
+            last_name: '',
+            email: 'john.doe@test.com',
+            id: '631710ab1094c5002647184e',
+          },
+        ],
+        start_ts: 1681220036419,
+        end_ts: 1681220036419,
+      },
+      labels: [
+        {
+          id: '643561cdfa2b2beac88f0024',
+          comment: 'tag-1',
+          version: 4,
+          user_id: USER_ID,
+          created_at: '2023-04-11T13:34:05.856Z',
+        },
+        {
+          id: '643561d1fa2b2beac88f0025',
+          comment: 'tag-2',
+          version: 4,
+          user_id: USER_ID,
+          created_at: '2023-04-11T13:34:09.280Z',
+        },
+      ],
+      pathnames: [],
+      project_ops: [{ add: { pathname: 'name.tex' }, atV: 3 }],
+    },
+    {
+      fromV: 1,
+      toV: 3,
+      meta: {
+        users: [
+          {
+            first_name: 'bobby.lapointe',
+            last_name: '',
+            email: 'bobby.lapointe@test.com',
+            id: '631710ab1094c5002647184e',
+          },
+        ],
+        start_ts: 1681220029569,
+        end_ts: 1681220031589,
+      },
+      labels: [
+        {
+          id: '6436bcf630293cb49e7f13a4',
+          comment: 'tag-3',
+          version: 3,
+          user_id: '631710ab1094c5002647184e',
+          created_at: '2023-04-12T14:15:18.892Z',
+        },
+        {
+          id: '6436bcf830293cb49e7f13a5',
+          comment: 'tag-4',
+          version: 3,
+          user_id: '631710ab1094c5002647184e',
+          created_at: '2023-04-12T14:15:20.814Z',
+        },
+      ],
+      pathnames: ['main.tex'],
+      project_ops: [],
+    },
+    {
+      fromV: 0,
+      toV: 1,
+      meta: {
+        users: [
+          {
+            first_name: 'john.doe',
+            last_name: '',
+            email: 'john.doe@test.com',
+            id: '631710ab1094c5002647184e',
+          },
+        ],
+        start_ts: 1669218226672,
+        end_ts: 1669218226672,
+      },
+      labels: [],
+      pathnames: [],
+      project_ops: [{ add: { pathname: 'main.tex' }, atV: 0 }],
+    },
+  ],
+}