Merge pull request #9589 from overleaf/jel-dash-clone-sort

[web] Sort cloned project on dash and maintain sort across filters

GitOrigin-RevId: 011bbada85384aa608777c3bf6c680b794f04d70

services/web/frontend/js/features/project-list/components/table/project-list-table.tsx

@@ -1,12 +1,7 @@
-import { useEffect } from 'react'
 import { useTranslation } from 'react-i18next'
 import Icon from '../../../../shared/components/icon'
 import ProjectListTableRow from './project-list-table-row'
 import { useProjectListContext } from '../../context/project-list-context'
-import {
-  ownerNameComparator,
-  defaultComparator,
-} from '../../util/sort-comparators'
 import { Project, Sort } from '../../../../../../types/project/dashboard/api'
 import { SortingOrder } from '../../../../../../types/sorting-order'
 
@@ -42,41 +37,9 @@ const toggleSort = (order: SortingOrder): SortingOrder => {
   return order === 'asc' ? 'desc' : 'asc'
 }
 
-const order = (order: SortingOrder, projects: Project[]) => {
-  return order === 'asc' ? [...projects] : projects.reverse()
-}
-
 function ProjectListTable() {
   const { t } = useTranslation()
-  const { visibleProjects, setVisibleProjects, sort, setSort } =
-    useProjectListContext()
-
-  useEffect(() => {
-    if (sort.by === 'title') {
-      setVisibleProjects(prevProjects => {
-        const sorted = [...prevProjects].sort((...args) => {
-          return defaultComparator(...args, 'name')
-        })
-        return order(sort.order, sorted)
-      })
-    }
-
-    if (sort.by === 'lastUpdated') {
-      setVisibleProjects(prevProjects => {
-        const sorted = [...prevProjects].sort((...args) => {
-          return defaultComparator(...args, 'lastUpdated')
-        })
-        return order(sort.order, sorted)
-      })
-    }
-
-    if (sort.by === 'owner') {
-      setVisibleProjects(prevProjects => {
-        const sorted = [...prevProjects].sort(ownerNameComparator)
-        return order(sort.order, sorted)
-      })
-    }
-  }, [sort.by, sort.order, setVisibleProjects])
+  const { visibleProjects, sort, setSort } = useProjectListContext()
 
   const handleSortClick = (by: Sort['by']) => {
     setSort(prev => ({

services/web/frontend/js/features/project-list/context/project-list-context.tsx

@@ -6,6 +6,7 @@ import {
   useContext,
   useEffect,
   useMemo,
+  useRef,
   useState,
 } from 'react'
 import { Tag } from '../../../../../app/src/Features/Tags/types'
@@ -18,6 +19,7 @@ import usePersistedState from '../../../shared/hooks/use-persisted-state'
 import getMeta from '../../../utils/meta'
 import useAsync from '../../../shared/hooks/use-async'
 import { getProjects } from '../util/api'
+import sortProjects from '../util/sort-projects'
 
 export type Filter = 'all' | 'owned' | 'shared' | 'archived' | 'trashed'
 type FilterMap = {
@@ -52,7 +54,6 @@ export const UNCATEGORIZED_KEY = 'uncategorized'
 type ProjectListContextValue = {
   addClonedProjectToViewData: (project: Project) => void
   visibleProjects: Project[]
-  setVisibleProjects: React.Dispatch<React.SetStateAction<Project[]>>
   totalProjectsCount: number
   error: Error | null
   isLoading: ReturnType<typeof useAsync>['isLoading']
@@ -94,6 +95,7 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
     'project-list-filter',
     'all'
   )
+  const prevSortRef = useRef<Sort>(sort)
   const [selectedTagId, setSelectedTagId] = usePersistedState<
     string | undefined
   >('project-list-selected-tag-id', undefined)
@@ -154,6 +156,13 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
     } else {
       filteredProjects = _.filter(filteredProjects, filters[filter])
     }
+
+    if (prevSortRef.current !== sort) {
+      filteredProjects = sortProjects(filteredProjects, sort)
+      const loadedProjectsSorted = sortProjects(loadedProjects, sort)
+      setLoadedProjects(loadedProjectsSorted)
+    }
+
     setVisibleProjects(filteredProjects)
   }, [
     loadedProjects,
@@ -163,8 +172,13 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
     selectedTagId,
     setSelectedTagId,
     searchText,
+    sort,
   ])
 
+  useEffect(() => {
+    prevSortRef.current = sort
+  }, [sort])
+
   const untaggedProjectsCount = useMemo(() => {
     const taggedProjectIds = _.uniq(_.flatten(tags.map(tag => tag.project_ids)))
     return loadedProjects.filter(
@@ -227,12 +241,13 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
       project.source = 'owner'
       project.trashed = false
       project.archived = false
-      const projects = [...loadedProjects]
-      projects.push(project)
-      setLoadedProjects(projects)
-      // to do: sort projects after loaded projects updated, otherwise, it's at bottom of list
+      loadedProjects.push(project)
+      const loadedProjectsSorted = sortProjects(loadedProjects, sort)
+      const visibleProjectsSorted = sortProjects(visibleProjects, sort)
+      setVisibleProjects(visibleProjectsSorted)
+      setLoadedProjects(loadedProjectsSorted)
     },
-    [loadedProjects, setLoadedProjects]
+    [loadedProjects, visibleProjects, sort]
   )
 
   const updateProjectViewData = useCallback(
@@ -273,7 +288,6 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
       selectTag,
       setSearchText,
       setSort,
-      setVisibleProjects,
       sort,
       tags,
       totalProjectsCount,
@@ -296,7 +310,6 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
       selectTag,
       setSearchText,
       setSort,
-      setVisibleProjects,
       sort,
       tags,
       totalProjectsCount,

services/web/frontend/js/features/project-list/util/sort-comparators.ts → services/web/frontend/js/features/project-list/util/sort-projects.ts

@@ -1,7 +1,12 @@
+import { Project, Sort } from '../../../../../types/project/dashboard/api'
+import { SortingOrder } from '../../../../../types/sorting-order'
 import { getOwnerName } from './project'
-import { Project } from '../../../../../types/project/dashboard/api'
 import { Compare } from '../../../../../types/array/sort'
 
+const order = (order: SortingOrder, projects: Project[]) => {
+  return order === 'asc' ? [...projects] : projects.reverse()
+}
+
 export const ownerNameComparator = (v1: Project, v2: Project) => {
   const ownerNameV1 = getOwnerName(v1)
   const ownerNameV2 = getOwnerName(v2)
@@ -63,3 +68,26 @@ export const defaultComparator = (
 
   return Compare.SORT_KEEP_ORDER
 }
+
+export default function sortProjects(projects: Project[], sort: Sort) {
+  let sorted = [...projects]
+  if (sort.by === 'title') {
+    sorted = sorted.sort((...args) => {
+      return defaultComparator(...args, 'name')
+    })
+  }
+
+  if (sort.by === 'lastUpdated') {
+    sorted = sorted.sort((...args) => {
+      return defaultComparator(...args, 'lastUpdated')
+    })
+  }
+
+  if (sort.by === 'owner') {
+    sorted = sorted.sort((...args) => {
+      return ownerNameComparator(...args)
+    })
+  }
+
+  return order(sort.order, sorted)
+}

services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx

@@ -1,14 +1,9 @@
-import {
-  render,
-  fireEvent,
-  screen,
-  waitFor,
-  within,
-} from '@testing-library/react'
+import { fireEvent, screen, waitFor, within } from '@testing-library/react'
 import { assert, expect } from 'chai'
 import fetchMock from 'fetch-mock'
 import TagsList from '../../../../../../frontend/js/features/project-list/components/sidebar/tags-list'
-import { ProjectListProvider } from '../../../../../../frontend/js/features/project-list/context/project-list-context'
+import { projectsData } from '../../fixtures/projects-data'
+import { renderWithProjectListContext } from '../../helpers/render-with-context'
 
 describe('<TagsList />', function () {
   beforeEach(async function () {
@@ -16,34 +11,15 @@ describe('<TagsList />', function () {
       {
         _id: 'abc123def456',
         name: 'Tag 1',
-        project_ids: ['456fea789bcd'],
+        project_ids: [projectsData[0].id],
       },
       {
         _id: 'bcd234efg567',
         name: 'Another tag',
-        project_ids: ['456fea789bcd', '567efa890bcd'],
+        project_ids: [projectsData[0].id, projectsData[1].id],
       },
     ])
 
-    fetchMock.post('/api/project', {
-      projects: [
-        {
-          id: '456fea789bcd',
-          archived: false,
-          trashed: false,
-        },
-        {
-          id: '567efa890bcd',
-          archived: false,
-          trashed: false,
-        },
-        {
-          id: '999fff999fff',
-          archived: false,
-          trashed: false,
-        },
-      ],
-    })
     fetchMock.post('/tag', {
       _id: 'eee888eee888',
       name: 'New Tag',
@@ -52,11 +28,7 @@ describe('<TagsList />', function () {
     fetchMock.post('express:/tag/:tagId/rename', 200)
     fetchMock.delete('express:/tag/:tagId', 200)
 
-    render(<TagsList />, {
-      wrapper: ({ children }) => (
-        <ProjectListProvider>{children}</ProjectListProvider>
-      ),
-    })
+    renderWithProjectListContext(<TagsList />)
 
     await waitFor(() => expect(fetchMock.called('/api/project')))
   })
@@ -79,7 +51,7 @@ describe('<TagsList />', function () {
       name: 'Another tag (2)',
     })
     screen.getByRole('button', {
-      name: 'Uncategorized (1)',
+      name: 'Uncategorized (3)',
     })
   })
 

services/web/test/frontend/features/project-list/unit/sort-comparators.test.ts → services/web/test/frontend/features/project-list/unit/sort-projects.test.ts

@@ -2,7 +2,7 @@ import { expect } from 'chai'
 import {
   ownerNameComparator,
   defaultComparator,
-} from '../../../../../frontend/js/features/project-list/util/sort-comparators'
+} from '../../../../../frontend/js/features/project-list/util/sort-projects'
 import { Project } from '../../../../../types/project/dashboard/api'
 
 const now = new Date()