From 08596f1fb3f0fa606bd5dd63421398017b619e5d Mon Sep 17 00:00:00 2001
From: Jessica Lawshe <jessica.lawshe@overleaf.com>
Date: Tue, 27 Sep 2022 11:51:40 -0500
Subject: [PATCH] Merge pull request #9744 from
 overleaf/jel-refactor-hidden-projects

[web] Refactor hidden projects

GitOrigin-RevId: e9d685842959d34ffa8dfe403b1afffddfe4ab1d
---
 .../project-list/components/load-more.tsx     |   8 +-
 .../context/project-list-context.tsx          |  50 ++++-----
 .../components/load-more.test.tsx             | 100 ++++++------------
 .../components/project-list-root.test.tsx     |  54 ++++++++--
 .../project-list/fixtures/projects-data.ts    |  19 ++++
 5 files changed, 119 insertions(+), 112 deletions(-)

diff --git a/services/web/frontend/js/features/project-list/components/load-more.tsx b/services/web/frontend/js/features/project-list/components/load-more.tsx
index 6353eafb44..d6d7c33b11 100644
--- a/services/web/frontend/js/features/project-list/components/load-more.tsx
+++ b/services/web/frontend/js/features/project-list/components/load-more.tsx
@@ -5,7 +5,7 @@ import { useProjectListContext } from '../context/project-list-context'
 export default function LoadMore() {
   const {
     visibleProjects,
-    hiddenProjects,
+    hiddenProjectsCount,
     loadMoreCount,
     showAllProjects,
     loadMoreProjects,
@@ -14,7 +14,7 @@ export default function LoadMore() {
 
   return (
     <div className="text-centered">
-      {hiddenProjects.length > 0 ? (
+      {hiddenProjectsCount > 0 ? (
         <Button
           bsStyle="info"
           className="project-list-load-more-button"
@@ -25,12 +25,12 @@ export default function LoadMore() {
         </Button>
       ) : null}
       <p>
-        {hiddenProjects.length > 0 ? (
+        {hiddenProjectsCount > 0 ? (
           <>
             <span aria-live="polite">
               {t('showing_x_out_of_n_projects', {
                 x: visibleProjects.length,
-                n: visibleProjects.length + hiddenProjects.length,
+                n: visibleProjects.length + hiddenProjectsCount,
               })}
             </span>{' '}
             <button
diff --git a/services/web/frontend/js/features/project-list/context/project-list-context.tsx b/services/web/frontend/js/features/project-list/context/project-list-context.tsx
index 245cb0ff30..bfc5b51c88 100644
--- a/services/web/frontend/js/features/project-list/context/project-list-context.tsx
+++ b/services/web/frontend/js/features/project-list/context/project-list-context.tsx
@@ -87,7 +87,7 @@ type ProjectListContextValue = {
   searchText: string
   setSearchText: React.Dispatch<React.SetStateAction<string>>
   selectedProjects: Project[]
-  hiddenProjects: Project[]
+  hiddenProjectsCount: number
   loadMoreCount: number
   showAllProjects: () => void
   loadMoreProjects: () => void
@@ -104,7 +104,9 @@ type ProjectListProviderProps = {
 export function ProjectListProvider({ children }: ProjectListProviderProps) {
   const [loadedProjects, setLoadedProjects] = useState<Project[]>([])
   const [visibleProjects, setVisibleProjects] = useState<Project[]>([])
-  const [hiddenProjects, setHiddenProjects] = useState<Project[]>([])
+  const [maxVisibleProjects, setMaxVisibleProjects] =
+    useState(MAX_PROJECT_PER_PAGE)
+  const [hiddenProjectsCount, setHiddenProjectsCount] = useState(0)
   const [loadMoreCount, setLoadMoreCount] = useState<number>(0)
   const [loadProgress, setLoadProgress] = useState(20)
   const [totalProjectsCount, setTotalProjectsCount] = useState<number>(0)
@@ -183,29 +185,31 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
       setLoadedProjects(loadedProjectsSorted)
     }
 
-    if (filteredProjects.length > MAX_PROJECT_PER_PAGE) {
+    if (filteredProjects.length > maxVisibleProjects) {
       const visibleFilteredProjects = filteredProjects.slice(
         0,
-        MAX_PROJECT_PER_PAGE
+        maxVisibleProjects
       )
+
+      const hiddenFilteredProjectsCount =
+        filteredProjects.slice(maxVisibleProjects).length
+
       setVisibleProjects(visibleFilteredProjects)
+      setHiddenProjectsCount(hiddenFilteredProjectsCount)
 
-      const hiddenFilteredProjects =
-        filteredProjects.slice(MAX_PROJECT_PER_PAGE)
-      setHiddenProjects(hiddenFilteredProjects)
-
-      if (hiddenFilteredProjects.length > MAX_PROJECT_PER_PAGE) {
+      if (hiddenFilteredProjectsCount > MAX_PROJECT_PER_PAGE) {
         setLoadMoreCount(MAX_PROJECT_PER_PAGE)
       } else {
-        setLoadMoreCount(hiddenFilteredProjects.length)
+        setLoadMoreCount(hiddenFilteredProjectsCount)
       }
     } else {
-      setHiddenProjects([])
       setVisibleProjects(filteredProjects)
       setLoadMoreCount(0)
+      setHiddenProjectsCount(0)
     }
   }, [
     loadedProjects,
+    maxVisibleProjects,
     tags,
     filter,
     setFilter,
@@ -221,23 +225,13 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
 
   const showAllProjects = useCallback(() => {
     setLoadMoreCount(0)
-    setVisibleProjects([...visibleProjects, ...hiddenProjects])
-    setHiddenProjects([])
-  }, [hiddenProjects, visibleProjects])
+    setHiddenProjectsCount(0)
+    setMaxVisibleProjects(maxVisibleProjects + hiddenProjectsCount)
+  }, [hiddenProjectsCount, maxVisibleProjects])
 
   const loadMoreProjects = useCallback(() => {
-    const newVisibleProjects = [
-      ...visibleProjects,
-      ...hiddenProjects.slice(0, loadMoreCount),
-    ]
-    const newHiddenProjects = hiddenProjects.slice(loadMoreCount)
-
-    setVisibleProjects(newVisibleProjects)
-    setHiddenProjects(newHiddenProjects)
-    if (newHiddenProjects.length < MAX_PROJECT_PER_PAGE) {
-      setLoadMoreCount(newHiddenProjects.length)
-    }
-  }, [visibleProjects, hiddenProjects, loadMoreCount])
+    setMaxVisibleProjects(maxVisibleProjects + loadMoreCount)
+  }, [maxVisibleProjects, loadMoreCount])
 
   const selectedProjects = useMemo(() => {
     return visibleProjects.filter(project => project.selected)
@@ -378,7 +372,7 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
       deleteTag,
       error,
       filter,
-      hiddenProjects,
+      hiddenProjectsCount,
       isLoading,
       loadMoreCount,
       loadMoreProjects,
@@ -408,7 +402,7 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) {
       deleteTag,
       error,
       filter,
-      hiddenProjects,
+      hiddenProjectsCount,
       isLoading,
       loadMoreCount,
       loadMoreProjects,
diff --git a/services/web/test/frontend/features/project-list/components/load-more.test.tsx b/services/web/test/frontend/features/project-list/components/load-more.test.tsx
index b44ae8be12..25d55e104a 100644
--- a/services/web/test/frontend/features/project-list/components/load-more.test.tsx
+++ b/services/web/test/frontend/features/project-list/components/load-more.test.tsx
@@ -2,8 +2,11 @@ import { fireEvent, screen, waitFor } from '@testing-library/dom'
 import { expect } from 'chai'
 import fetchMock from 'fetch-mock'
 import LoadMore from '../../../../../frontend/js/features/project-list/components/load-more'
-import { Project } from '../../../../../types/project/dashboard/api'
-import { projectsData } from '../fixtures/projects-data'
+import {
+  projectsData,
+  makeLongProjectList,
+  currentProjects,
+} from '../fixtures/projects-data'
 import { renderWithProjectListContext } from '../helpers/render-with-context'
 
 describe('<LoadMore />', function () {
@@ -12,26 +15,17 @@ describe('<LoadMore />', function () {
   })
 
   it('renders on a project list longer than 40', async function () {
-    // archived and trashed projects are currently not shown
-    const filteredProjects = projectsData.filter(
-      ({ archived, trashed }) => !archived && !trashed
-    )
-    let longProjectsData: Project[] = filteredProjects
-    const MULTIPLY_FACTOR = 10
-    // longProjectsData.length = 55
-    for (let i = 0; i < MULTIPLY_FACTOR; i++) {
-      longProjectsData = [...longProjectsData, ...filteredProjects]
-    }
+    const { fullList, currentList } = makeLongProjectList(55)
 
-    renderWithProjectListContext(<LoadMore />, { projects: longProjectsData })
+    renderWithProjectListContext(<LoadMore />, {
+      projects: fullList,
+    })
 
     await screen.findByRole('button', {
       name: /Show 20 more projects/i,
     })
 
-    await screen.findByText(
-      `Showing 20 out of ${longProjectsData.length} projects.`
-    )
+    await screen.findByText(`Showing 20 out of ${currentList.length} projects.`)
 
     await screen.findByRole('button', {
       name: /Show all projects/i,
@@ -39,29 +33,15 @@ describe('<LoadMore />', function () {
   })
 
   it('renders on a project list longer than 20 and shorter than 40', async function () {
-    // archived and trashed projects are currently not shown
-    const filteredProjects = projectsData.filter(
-      ({ archived, trashed }) => !archived && !trashed
-    )
-    let longProjectsData: Project[] = filteredProjects
-    const MULTIPLY_FACTOR = 5
-    // longProjectsData.length = 30
-    for (let i = 0; i < MULTIPLY_FACTOR; i++) {
-      longProjectsData = [...longProjectsData, ...filteredProjects]
-    }
+    const { fullList, currentList } = makeLongProjectList(30)
 
-    renderWithProjectListContext(<LoadMore />, { projects: longProjectsData })
+    renderWithProjectListContext(<LoadMore />, { projects: fullList })
 
     await screen.findByRole('button', {
-      name: new RegExp(
-        `Show ${longProjectsData.length - 20} more projects`,
-        'i'
-      ),
+      name: new RegExp(`Show ${currentList.length - 20} more projects`, 'i'),
     })
 
-    await screen.findByText(
-      `Showing 20 out of ${longProjectsData.length} projects.`
-    )
+    await screen.findByText(`Showing 20 out of ${currentList.length} projects.`)
 
     await screen.findByRole('button', {
       name: /Show all projects/i,
@@ -69,72 +49,52 @@ describe('<LoadMore />', function () {
   })
 
   it('renders on a project list shorter than 20', async function () {
-    // archived and trashed projects are currently not shown
-    // filteredProjects.length = 5
-    const filteredProjects = projectsData.filter(
-      ({ archived, trashed }) => !archived && !trashed
-    )
-
-    renderWithProjectListContext(<LoadMore />, { projects: filteredProjects })
+    renderWithProjectListContext(<LoadMore />, { projects: projectsData })
 
     await waitFor(() => {
       expect(screen.queryByRole('button', { name: 'Show all' })).to.not.exist
       screen.getByText(
-        `Showing ${filteredProjects.length} out of ${filteredProjects.length} projects.`
+        `Showing ${currentProjects.length} out of ${currentProjects.length} projects.`
       )
     })
   })
 
   it('change text when pressing the "Show 20 more" once for project list longer than 40', async function () {
-    // archived and trashed projects are currently not shown
-    const filteredProjects = projectsData.filter(
-      ({ archived, trashed }) => !archived && !trashed
-    )
-    let longProjectsData: Project[] = filteredProjects
-    const MULTIPLY_FACTOR = 10
-    // longProjectsData.length = 55
-    for (let i = 0; i < MULTIPLY_FACTOR; i++) {
-      longProjectsData = [...longProjectsData, ...filteredProjects]
-    }
+    const { fullList, currentList } = makeLongProjectList(55)
 
-    renderWithProjectListContext(<LoadMore />, { projects: longProjectsData })
+    renderWithProjectListContext(<LoadMore />, { projects: fullList })
 
     await waitFor(() => {
       const showMoreBtn = screen.getByRole('button', {
         name: /Show 20 more projects/i,
       })
       fireEvent.click(showMoreBtn)
+    })
 
-      screen.getByRole('button', {
-        name: /Show 15 more/i,
-      })
-      screen.getByText(`Showing 40 out of ${longProjectsData.length} projects.`)
+    await waitFor(() => {
+      screen.getByLabelText(
+        `Show ${currentList.length - 20 - 20} more projects`
+      )
+      screen.getByText(`Showing 40 out of ${currentList.length} projects.`)
     })
   })
 
   it('change text when pressing the "Show 20 more" once for project list longer than 20 and shorter than 40', async function () {
-    // archived and trashed projects are currently not shown
-    const filteredProjects = projectsData.filter(
-      ({ archived, trashed }) => !archived && !trashed
-    )
-    let longProjectsData: Project[] = filteredProjects
-    const MULTIPLY_FACTOR = 5
-    // longProjectsData.length = 30
-    for (let i = 0; i < MULTIPLY_FACTOR; i++) {
-      longProjectsData = [...longProjectsData, ...filteredProjects]
-    }
+    const { fullList, currentList } = makeLongProjectList(30)
 
-    renderWithProjectListContext(<LoadMore />, { projects: longProjectsData })
+    renderWithProjectListContext(<LoadMore />, { projects: fullList })
 
     await waitFor(() => {
       const showMoreBtn = screen.getByRole('button', {
-        name: /Show 10 more projects/i,
+        name: /Show 7 more projects/i,
       })
       fireEvent.click(showMoreBtn)
+    })
 
+    await waitFor(() => {
       expect(screen.queryByRole('button', { name: /Show/ })).to.not.exist
       screen.getByText(
-        `Showing ${longProjectsData.length} out of ${longProjectsData.length} projects.`
+        `Showing ${currentList.length} out of ${currentList.length} projects.`
       )
     })
   })
diff --git a/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx b/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx
index 6457fcb963..1d66a33e43 100644
--- a/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx
+++ b/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx
@@ -4,11 +4,8 @@ import fetchMock from 'fetch-mock'
 import sinon from 'sinon'
 import ProjectListRoot from '../../../../../frontend/js/features/project-list/components/project-list-root'
 import { renderWithProjectListContext } from '../helpers/render-with-context'
-import {
-  currentProjects,
-  owner,
-  trashedProjects,
-} from '../fixtures/projects-data'
+import { owner, makeLongProjectList } from '../fixtures/projects-data'
+const { fullList, currentList, trashedList } = makeLongProjectList(40)
 
 const userId = owner.id
 
@@ -26,7 +23,9 @@ describe('<ProjectListRoot />', function () {
       value: { assign: locationStub },
     })
 
-    renderWithProjectListContext(<ProjectListRoot />)
+    renderWithProjectListContext(<ProjectListRoot />, {
+      projects: fullList,
+    })
     await fetchMock.flush(true)
     await waitFor(() => {
       screen.findByRole('table')
@@ -145,6 +144,41 @@ describe('<ProjectListRoot />', function () {
         expect(projectRequest2Url).to.equal(`/project/${project2Id}/trash`)
         expect(projectRequest2Headers?.method).to.equal('POST')
       })
+
+      it('only checks the projects that are viewable when there is a load more button', async function () {
+        // first one is the select all checkbox
+        fireEvent.click(allCheckboxes[0])
+
+        allCheckboxes = screen.getAllByRole<HTMLInputElement>('checkbox')
+        let checked = allCheckboxes.filter(c => c.checked)
+        expect(checked.length).to.equal(21) // max projects viewable by default is 20, and plus one for check all
+
+        const loadMoreButton = screen.getByLabelText('Show 17 more projects')
+        fireEvent.click(loadMoreButton)
+
+        allCheckboxes = screen.getAllByRole<HTMLInputElement>('checkbox')
+        expect(allCheckboxes.length).to.equal(currentList.length + 1)
+        checked = allCheckboxes.filter(c => c.checked)
+        expect(checked.length).to.equal(20) // remains same even after showing more
+      })
+
+      it('maintains viewable and selected projects after loading more and then selecting all', async function () {
+        const loadMoreButton = screen.getByLabelText('Show 17 more projects')
+        fireEvent.click(loadMoreButton)
+        // verify button gone
+        screen.getByText(
+          `Showing ${currentList.length} out of ${currentList.length} projects.`
+        )
+        // first one is the select all checkbox
+        fireEvent.click(allCheckboxes[0])
+        // verify button still gone
+        screen.getByText(
+          `Showing ${currentList.length} out of ${currentList.length} projects.`
+        )
+
+        allCheckboxes = screen.getAllByRole<HTMLInputElement>('checkbox')
+        expect(allCheckboxes.length).to.equal(currentList.length + 1)
+      })
     })
 
     describe('archived projects', function () {
@@ -170,14 +204,14 @@ describe('<ProjectListRoot />', function () {
 
         allCheckboxes = screen.getAllByRole<HTMLInputElement>('checkbox')
         // + 1 because of select all
-        expect(allCheckboxes.length).to.equal(trashedProjects.length + 1)
+        expect(allCheckboxes.length).to.equal(trashedList.length + 1)
 
         // first one is the select all checkbox
         fireEvent.click(allCheckboxes[0])
 
         const allCheckboxesChecked = allCheckboxes.filter(c => c.checked)
         // + 1 because of select all
-        expect(allCheckboxesChecked.length).to.equal(trashedProjects.length + 1)
+        expect(allCheckboxesChecked.length).to.equal(trashedList.length + 1)
 
         actionsToolbar = screen.getAllByRole('toolbar')[0]
       })
@@ -218,7 +252,7 @@ describe('<ProjectListRoot />', function () {
         fireEvent.click(allCheckboxes[1])
 
         const allCheckboxesChecked = allCheckboxes.filter(c => c.checked)
-        expect(allCheckboxesChecked.length).to.equal(trashedProjects.length - 1)
+        expect(allCheckboxesChecked.length).to.equal(trashedList.length - 1)
 
         await fetchMock.flush(true)
         expect(fetchMock.done()).to.be.true
@@ -257,7 +291,7 @@ describe('<ProjectListRoot />', function () {
       const input = screen.getAllByRole('textbox', {
         name: /search projects/i,
       })[0]
-      const value = currentProjects[0].name
+      const value = currentList[0].name
 
       fireEvent.change(input, { target: { value } })
 
diff --git a/services/web/test/frontend/features/project-list/fixtures/projects-data.ts b/services/web/test/frontend/features/project-list/fixtures/projects-data.ts
index f390f2f399..e04aa1e148 100644
--- a/services/web/test/frontend/features/project-list/fixtures/projects-data.ts
+++ b/services/web/test/frontend/features/project-list/fixtures/projects-data.ts
@@ -139,3 +139,22 @@ export const currentProjects: Array<Project> = projectsData.filter(
 export const trashedProjects: Array<Project> = projectsData.filter(
   ({ trashed }) => trashed
 )
+
+export const makeLongProjectList = (listLength: number) => {
+  const longList = [...projectsData]
+  while (longList.length < listLength) {
+    longList.push(
+      Object.assign({}, copyableProject, {
+        id: `newProjectId${longList.length}`,
+      })
+    )
+  }
+
+  return {
+    fullList: longList,
+    currentList: longList.filter(
+      ({ archived, trashed }) => !archived && !trashed
+    ),
+    trashedList: longList.filter(({ trashed }) => trashed),
+  }
+}