From f24ea8b7a9d3eeec3b120a928650509a6ecf45dc Mon Sep 17 00:00:00 2001 From: Caleb Cox Date: Wed, 23 Oct 2024 11:13:58 -0500 Subject: [PATCH 1/3] Deselect removed ids --- .../[accountListId]/tasks/Tasks.test.tsx | 11 ++++++++++- src/hooks/useMassSelection.test.ts | 16 +++++++++++++++- src/hooks/useMassSelection.ts | 15 ++++++++++----- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/pages/accountLists/[accountListId]/tasks/Tasks.test.tsx b/pages/accountLists/[accountListId]/tasks/Tasks.test.tsx index 9dd78d764..76b797488 100644 --- a/pages/accountLists/[accountListId]/tasks/Tasks.test.tsx +++ b/pages/accountLists/[accountListId]/tasks/Tasks.test.tsx @@ -5,6 +5,7 @@ import userEvent from '@testing-library/user-event'; import { VirtuosoMockContext } from 'react-virtuoso'; import TestRouter from '__tests__/util/TestRouter'; import { GqlMockedProvider } from '__tests__/util/graphqlMocking'; +import { GetTaskIdsForMassSelectionQuery } from 'src/hooks/GetIdsForMassSelection.generated'; import useTaskModal from 'src/hooks/useTaskModal'; import { dispatch } from 'src/lib/analytics'; import theme from 'src/theme'; @@ -141,7 +142,10 @@ describe('tasks page', () => { it('should dispatch one analytics event per task', async () => { const { getAllByTestId, getByRole, findByRole } = render( - + mocks={{ Tasks: { tasks: { @@ -154,6 +158,11 @@ describe('tasks page', () => { pageInfo: { hasNextPage: false }, }, }, + GetTaskIdsForMassSelection: { + tasks: { + nodes: [{ id: '1' }, { id: '2' }, { id: '3' }], + }, + }, }} > diff --git a/src/hooks/useMassSelection.test.ts b/src/hooks/useMassSelection.test.ts index bbe5aaca8..10a146d7f 100644 --- a/src/hooks/useMassSelection.test.ts +++ b/src/hooks/useMassSelection.test.ts @@ -1,6 +1,6 @@ import { renderHook } from '@testing-library/react'; import { ListHeaderCheckBoxState } from 'src/components/Shared/Header/ListHeader'; -import { useMassSelection } from './useMassSelection'; +import { UseMassSelectionResult, useMassSelection } from './useMassSelection'; const defaultIdsList = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10']; @@ -117,4 +117,18 @@ describe('useMassSelection', () => { expect(result.current.ids).toEqual(['1', '2', '3', '7', '8', '9', '10']); }); }); + + it('deselects removed ids', () => { + const { result, rerender } = renderHook< + UseMassSelectionResult, + { ids: string[] } + >(({ ids }) => useMassSelection(ids), { + initialProps: { ids: defaultIdsList }, + }); + + result.current.selectMultipleIds(['1', '2', '3']); + + rerender({ ids: ['2', '3', '4'] }); + expect(result.current.ids).toEqual(['2', '3']); + }); }); diff --git a/src/hooks/useMassSelection.ts b/src/hooks/useMassSelection.ts index d9a849e3a..91366e1d0 100644 --- a/src/hooks/useMassSelection.ts +++ b/src/hooks/useMassSelection.ts @@ -1,9 +1,7 @@ -import { useState } from 'react'; +import { useEffect, useState } from 'react'; import { ListHeaderCheckBoxState } from '../components/Shared/Header/ListHeader'; -export const useMassSelection = ( - idsList: string[], -): { +export interface UseMassSelectionResult { ids: string[]; selectionType: ListHeaderCheckBoxState; isRowChecked: (id: string) => boolean; @@ -12,11 +10,18 @@ export const useMassSelection = ( toggleSelectionById: (id: string) => void; selectMultipleIds: (ids: string[]) => void; deselectMultipleIds: (ids: string[]) => void; -} => { +} + +export const useMassSelection = (idsList: string[]): UseMassSelectionResult => { const totalCount = idsList.length; const [ids, setIds] = useState([]); + // When the idsList change, deselect any ids that were removed + useEffect(() => { + setIds((previousIds) => previousIds.filter((id) => idsList.includes(id))); + }, [idsList]); + const toggleSelectionById = (id: string) => { if (ids.includes(id)) { setIds((previousIds) => From 58906359f2eb7e3a621552b385192ed21b86d8fb Mon Sep 17 00:00:00 2001 From: Caleb Cox Date: Wed, 23 Oct 2024 11:16:00 -0500 Subject: [PATCH 2/3] Use function form of useState This change is for consistency and also so that you can modify the selected ids multiple times during the same render and get the expected results. --- src/hooks/useMassSelection.ts | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/hooks/useMassSelection.ts b/src/hooks/useMassSelection.ts index 91366e1d0..7a9e004a6 100644 --- a/src/hooks/useMassSelection.ts +++ b/src/hooks/useMassSelection.ts @@ -23,21 +23,26 @@ export const useMassSelection = (idsList: string[]): UseMassSelectionResult => { }, [idsList]); const toggleSelectionById = (id: string) => { - if (ids.includes(id)) { - setIds((previousIds) => - previousIds.filter((selectedIds) => selectedIds !== id), - ); - } else { - setIds((previousIds) => [...previousIds, id]); - } + setIds((previousIds) => { + if (previousIds.includes(id)) { + return previousIds.filter((selectedIds) => selectedIds !== id); + } else { + return [...previousIds, id]; + } + }); }; const selectMultipleIds = (newIds: string[]) => { - setIds([...ids, ...newIds.filter((newId) => !ids.includes(newId))]); + setIds((previousIds) => [ + ...previousIds, + ...newIds.filter((newId) => !ids.includes(newId)), + ]); }; const deselectMultipleIds = (idsToRemove: string[]) => { - setIds(ids.filter((id) => !idsToRemove.includes(id))); + setIds((previousIds) => + previousIds.filter((id) => !idsToRemove.includes(id)), + ); }; const deselectAll = () => { From 6e9c8e4ddc6185a6542534cab44379955d310b29 Mon Sep 17 00:00:00 2001 From: Caleb Cox Date: Thu, 24 Oct 2024 14:01:33 -0500 Subject: [PATCH 3/3] Use previous mass selection ids Also speeds up loading mass selection ids by not waiting for the contacts or tasks to load first. --- .../tasks/[[...contactId]].page.tsx | 23 ++++----- .../ContactTasksTab/ContactTasksTab.tsx | 22 +++++---- .../ContactsContext/ContactsContext.tsx | 25 +++++----- .../PartnerGivingAnalysisReport.tsx | 47 ++++++++++--------- .../Appeal/AppealsContext/AppealsContext.tsx | 39 ++++++++------- .../ContactFlowColumn/ContactFlowColumn.tsx | 26 +++++----- src/hooks/GetIdsForMassSelection.graphql | 6 +-- src/hooks/useUpdateTasksQueries.ts | 5 +- 8 files changed, 104 insertions(+), 89 deletions(-) diff --git a/pages/accountLists/[accountListId]/tasks/[[...contactId]].page.tsx b/pages/accountLists/[accountListId]/tasks/[[...contactId]].page.tsx index 1dc903e5f..ff19425c4 100644 --- a/pages/accountLists/[accountListId]/tasks/[[...contactId]].page.tsx +++ b/pages/accountLists/[accountListId]/tasks/[[...contactId]].page.tsx @@ -184,18 +184,19 @@ const TasksPage: React.FC = () => { //#region Mass Actions - const taskCount = data?.tasks.totalCount ?? 0; - const { data: allTasks } = useGetTaskIdsForMassSelectionQuery({ - variables: { - accountListId, - first: taskCount, - tasksFilter, - }, - skip: taskCount === 0, - }); + const { data: allTasks, previousData: allTasksPrevious } = + useGetTaskIdsForMassSelectionQuery({ + variables: { + accountListId, + tasksFilter, + }, + }); + // When the next batch of task ids is loading, use the previous batch of task ids in the + // meantime to avoid throwing out the selected task ids. const allTaskIds = useMemo( - () => allTasks?.tasks.nodes.map((task) => task.id) ?? [], - [allTasks], + () => + (allTasks ?? allTasksPrevious)?.tasks.nodes.map((task) => task.id) ?? [], + [allTasks, allTasksPrevious], ); const { diff --git a/src/components/Contacts/ContactDetails/ContactTasksTab/ContactTasksTab.tsx b/src/components/Contacts/ContactDetails/ContactTasksTab/ContactTasksTab.tsx index 6315bae06..99df7939a 100644 --- a/src/components/Contacts/ContactDetails/ContactTasksTab/ContactTasksTab.tsx +++ b/src/components/Contacts/ContactDetails/ContactTasksTab/ContactTasksTab.tsx @@ -137,17 +137,19 @@ export const ContactTasksTab: React.FC = ({ [starredFilter, searchTerm], ); const taskCount = data?.tasks.totalCount ?? 0; - const { data: allTasks } = useGetTaskIdsForMassSelectionQuery({ - variables: { - accountListId, - first: taskCount, - tasksFilter, - }, - skip: taskCount === 0, - }); + const { data: allTasks, previousData: allTasksPrevious } = + useGetTaskIdsForMassSelectionQuery({ + variables: { + accountListId, + tasksFilter, + }, + }); + // When the next batch of task ids is loading, use the previous batch of task ids in the + // meantime to avoid throwing out the selected task ids. const allTaskIds = useMemo( - () => allTasks?.tasks.nodes.map((task) => task.id) ?? [], - [allTasks], + () => + (allTasks ?? allTasksPrevious)?.tasks.nodes.map((task) => task.id) ?? [], + [allTasks, allTasksPrevious], ); //#region Mass Actions const { diff --git a/src/components/Contacts/ContactsContext/ContactsContext.tsx b/src/components/Contacts/ContactsContext/ContactsContext.tsx index 80f144916..e74c1df50 100644 --- a/src/components/Contacts/ContactsContext/ContactsContext.tsx +++ b/src/components/Contacts/ContactsContext/ContactsContext.tsx @@ -186,18 +186,21 @@ export const ContactsProvider: React.FC = ({ //#region Mass Actions - const contactCount = data?.contacts.totalCount ?? 0; - const { data: allContacts } = useGetIdsForMassSelectionQuery({ - variables: { - accountListId, - first: contactCount, - contactsFilters, - }, - skip: contactCount === 0, - }); + const { data: allContacts, previousData: allContactsPrevious } = + useGetIdsForMassSelectionQuery({ + variables: { + accountListId, + contactsFilters, + }, + }); + // When the next batch of contact ids is loading, use the previous batch of contact ids in the + // meantime to avoid throwing out the selected contact ids. const allContactIds = useMemo( - () => allContacts?.contacts.nodes.map((contact) => contact.id) ?? [], - [allContacts], + () => + (allContacts ?? allContactsPrevious)?.contacts.nodes.map( + (contact) => contact.id, + ) ?? [], + [allContacts, allContactsPrevious], ); const { diff --git a/src/components/Reports/PartnerGivingAnalysisReport/PartnerGivingAnalysisReport.tsx b/src/components/Reports/PartnerGivingAnalysisReport/PartnerGivingAnalysisReport.tsx index 8ed7b8e1c..4ff9828a1 100644 --- a/src/components/Reports/PartnerGivingAnalysisReport/PartnerGivingAnalysisReport.tsx +++ b/src/components/Reports/PartnerGivingAnalysisReport/PartnerGivingAnalysisReport.tsx @@ -88,26 +88,28 @@ export const PartnerGivingAnalysisReport = forwardRef< ? Object.keys(activeFilters).length > 0 : false; - const { data, loading } = useGetPartnerGivingAnalysisReportQuery({ - variables: { - input: { - accountListId, - // Page 1 is the first page for the API - page: page + 1, - pageSize: limit, - sortField: orderBy ?? '', - sortDirection: - order === 'asc' - ? SortDirection.Ascending - : SortDirection.Descending, - contactFilters, + const { data, previousData, loading } = + useGetPartnerGivingAnalysisReportQuery({ + variables: { + input: { + accountListId, + // Page 1 is the first page for the API + page: page + 1, + pageSize: limit, + sortField: orderBy ?? '', + sortDirection: + order === 'asc' + ? SortDirection.Ascending + : SortDirection.Descending, + contactFilters, + }, }, - }, - }); + }); const contacts = data?.partnerGivingAnalysisReport.contacts ?? []; - const contactCount = data?.partnerGivingAnalysisReport?.totalContacts ?? 0; - const { data: allContacts } = + const contactCount = + (data ?? previousData)?.partnerGivingAnalysisReport?.totalContacts ?? 0; + const { data: allContacts, previousData: allContactsPrevious } = useGetPartnerGivingAnalysisIdsForMassSelectionQuery({ variables: { input: { @@ -121,12 +123,15 @@ export const PartnerGivingAnalysisReport = forwardRef< }, skip: contactCount === 0, }); + // When the next batch of contact ids is loading, use the previous batch of contact ids in the + // meantime to avoid throwing out the selected contact ids. const allContactIds = useMemo( () => - allContacts?.partnerGivingAnalysisReport?.contacts.map( - (contact) => contact.id, - ) ?? [], - [allContacts], + ( + allContacts ?? allContactsPrevious + )?.partnerGivingAnalysisReport?.contacts.map((contact) => contact.id) ?? + [], + [allContacts, allContactsPrevious], ); const { diff --git a/src/components/Tool/Appeal/AppealsContext/AppealsContext.tsx b/src/components/Tool/Appeal/AppealsContext/AppealsContext.tsx index 2f231510e..6d39b6770 100644 --- a/src/components/Tool/Appeal/AppealsContext/AppealsContext.tsx +++ b/src/components/Tool/Appeal/AppealsContext/AppealsContext.tsx @@ -173,38 +173,45 @@ export const AppealsProvider: React.FC = ({ }, skip: !accountListId, }); - const { data } = contactsQueryResult; //#region Mass Actions - const contactCount = data?.contacts.totalCount ?? 0; - const { data: allContacts } = useGetIdsForMassSelectionQuery({ - variables: { - accountListId, - first: contactCount, - contactsFilters, - }, - skip: contactCount === 0, - }); + const { data: allContacts, previousData: allContactsPrevious } = + useGetIdsForMassSelectionQuery({ + variables: { + accountListId, + contactsFilters, + }, + }); // In the flows view, we need the total count of all contacts in every column, but the API // filters out contacts excluded from an appeal. We have to load excluded contacts also and // manually merge them with the other contacts. - const { data: allExcludedContacts } = useGetIdsForMassSelectionQuery({ + const { + data: allExcludedContacts, + previousData: allExcludedContactsPrevious, + } = useGetIdsForMassSelectionQuery({ variables: { accountListId, - first: contactCount, contactsFilters: { ...contactsFilters, appealStatus: 'excluded' }, }, // Skip this query when there is an appealStatus filter from the list view - skip: contactCount === 0 || !!contactsFilters.appealStatus, + skip: !!contactsFilters.appealStatus, }); + // When the next batch of contact ids is loading, use the previous batch of contact ids in the + // meantime to avoid throwing out the selected contact ids. const allContactIds = useMemo( () => [ - ...(allContacts?.contacts.nodes ?? []), - ...(allExcludedContacts?.contacts.nodes ?? []), + ...((allContacts ?? allContactsPrevious)?.contacts.nodes ?? []), + ...((allExcludedContacts ?? allExcludedContactsPrevious)?.contacts + .nodes ?? []), ].map((contact) => contact.id), - [allContacts, allExcludedContacts], + [ + allContacts, + allContactsPrevious, + allExcludedContacts, + allExcludedContactsPrevious, + ], ); const { diff --git a/src/components/Tool/Appeal/Flow/ContactFlowColumn/ContactFlowColumn.tsx b/src/components/Tool/Appeal/Flow/ContactFlowColumn/ContactFlowColumn.tsx index 73ca2b50f..5dab4670f 100644 --- a/src/components/Tool/Appeal/Flow/ContactFlowColumn/ContactFlowColumn.tsx +++ b/src/components/Tool/Appeal/Flow/ContactFlowColumn/ContactFlowColumn.tsx @@ -100,19 +100,21 @@ export const ContactFlowColumn: React.FC = ({ skip: !accountListId || !appealStatus, }); - const contactCount = data?.contacts.totalCount ?? 0; - const { data: allContacts } = useGetIdsForMassSelectionQuery({ - variables: { - accountListId, - first: contactCount, - contactsFilters, - }, - skip: contactCount === 0, - }); - + const { data: allContacts, previousData: allContactsPrevious } = + useGetIdsForMassSelectionQuery({ + variables: { + accountListId, + contactsFilters, + }, + }); + // When the next batch of contact ids is loading, use the previous batch of contact ids in the + // meantime to avoid throwing out the selected contact ids. const allContactIds = useMemo( - () => allContacts?.contacts.nodes.map((contact) => contact.id) ?? [], - [allContacts], + () => + (allContacts ?? allContactsPrevious)?.contacts.nodes.map( + (contact) => contact.id, + ) ?? [], + [allContacts, allContactsPrevious], ); const { data: excludedContacts } = useExcludedAppealContactsQuery({ diff --git a/src/hooks/GetIdsForMassSelection.graphql b/src/hooks/GetIdsForMassSelection.graphql index 0935d8920..cbef975ab 100644 --- a/src/hooks/GetIdsForMassSelection.graphql +++ b/src/hooks/GetIdsForMassSelection.graphql @@ -1,12 +1,11 @@ query GetIdsForMassSelection( $accountListId: ID! - $first: Int! $contactsFilters: ContactFilterSetInput ) { contacts( accountListId: $accountListId - first: $first contactsFilter: $contactsFilters + first: 20000 # Maximum allowed by the API ) { nodes { id @@ -16,13 +15,12 @@ query GetIdsForMassSelection( query GetTaskIdsForMassSelection( $accountListId: ID! - $first: Int! $tasksFilter: TaskFilterSetInput ) { tasks( accountListId: $accountListId - first: $first tasksFilter: $tasksFilter + first: 1000 # Maximum allowed by the API ) { nodes { id diff --git a/src/hooks/useUpdateTasksQueries.ts b/src/hooks/useUpdateTasksQueries.ts index cee6d01c2..9ec4b09db 100644 --- a/src/hooks/useUpdateTasksQueries.ts +++ b/src/hooks/useUpdateTasksQueries.ts @@ -57,10 +57,7 @@ export const useUpdateTasksQueries = (): { GetTaskIdsForMassSelectionQueryVariables >({ query: GetTaskIdsForMassSelectionDocument, - variables: { - ...variables, - first: observableQuery.getLastResult()?.data.tasks.nodes.length, - }, + variables: variables, }); if (!taskIds) { return;