From b1e495559ff986741f5c09af6bf44305511801d0 Mon Sep 17 00:00:00 2001 From: Caleb Cox Date: Wed, 23 Oct 2024 13:35:33 -0500 Subject: [PATCH 1/5] Fix filtering the map by selected contacts --- .../ContactsContext/ContactsContext.test.tsx | 279 +++++++----------- .../ContactsContext/ContactsContext.tsx | 17 +- 2 files changed, 123 insertions(+), 173 deletions(-) diff --git a/src/components/Contacts/ContactsContext/ContactsContext.test.tsx b/src/components/Contacts/ContactsContext/ContactsContext.test.tsx index ffc826a77..326643307 100644 --- a/src/components/Contacts/ContactsContext/ContactsContext.test.tsx +++ b/src/components/Contacts/ContactsContext/ContactsContext.test.tsx @@ -7,14 +7,10 @@ import TestRouter from '__tests__/util/TestRouter'; import { GqlMockedProvider } from '__tests__/util/graphqlMocking'; import { ContactFiltersQuery } from 'pages/accountLists/[accountListId]/contacts/Contacts.generated'; import { ContactsWrapper } from 'pages/accountLists/[accountListId]/contacts/ContactsWrapper'; -import { GetUserOptionsQuery } from 'src/components/Contacts/ContactFlow/GetUserOptions.generated'; +import { GetIdsForMassSelectionQuery } from 'src/hooks/GetIdsForMassSelection.generated'; import { UserOptionQuery } from 'src/hooks/UserPreference.generated'; -import { useMassSelection } from '../../../hooks/useMassSelection'; import theme from '../../../theme'; -import { - ListHeaderCheckBoxState, - TableViewModeEnum, -} from '../../Shared/Header/ListHeader'; +import { TableViewModeEnum } from '../../Shared/Header/ListHeader'; import { ContactsContext, ContactsContextSavedFilters, @@ -22,18 +18,8 @@ import { } from './ContactsContext'; const accountListId = 'account-list-1'; -const isReady = true; const replace = jest.fn(); -jest.mock('src/hooks/useMassSelection'); - -(useMassSelection as jest.Mock).mockReturnValue({ - selectionType: ListHeaderCheckBoxState.Unchecked, - isRowChecked: jest.fn(), - toggleSelectAll: jest.fn(), - toggleSelectionById: jest.fn(), -}); - const mockEnqueue = jest.fn(); jest.mock('notistack', () => ({ @@ -47,12 +33,74 @@ jest.mock('notistack', () => ({ }, })); -const TestRender: React.FC = () => { - const { viewMode, handleViewModeChange, userOptionsLoading } = useContext( - ContactsContext, - ) as ContactsType; +interface TestWrapper { + contactId?: string[]; + contactView?: string; + savedFilter?: string; + children: JSX.Element; +} + +const TestWrapper: React.FC = ({ + contactId, + contactView = 'list', + savedFilter, + children, +}) => { return ( - + + + + mocks={{ + GetIdsForMassSelection: { + contacts: { + nodes: [{ id: 'contact-1' }], + }, + }, + UserOption: { + userOption: { + key: 'contacts_view', + value: contactView, + }, + }, + ContactFilters: { + userOptions: [ + { + key: 'saved_contacts_filter_My_Cool_Filter', + value: savedFilter ?? null, + }, + ], + }, + }} + > + {children} + + + + ); +}; + +const InnerComponent: React.FC = () => { + const { + viewMode, + handleViewModeChange, + userOptionsLoading, + toggleSelectionById, + activeFilters, + } = useContext(ContactsContext) as ContactsType; + + return ( +
{!userOptionsLoading ? ( <> {viewMode} @@ -81,7 +129,11 @@ const TestRender: React.FC = () => { ) : ( <>Loading )} - + +
{JSON.stringify(activeFilters)}
+
); }; @@ -100,33 +152,11 @@ const TestRenderContactsFilters: React.FC = () => { describe('ContactsPageContext', () => { it('has a contact id and switches from list to flows', async () => { const { getByText, findByText } = render( - - - - mocks={{ - UserOption: { - userOption: { - id: 'test-id', - key: 'contacts_view', - value: 'flows', - }, - }, - }} - > - - - - - - , + + + , ); + expect(getByText('Loading')).toBeInTheDocument(); userEvent.click(await findByText('Flows Button')); expect(await findByText('flows')).toBeInTheDocument(); @@ -143,33 +173,11 @@ describe('ContactsPageContext', () => { it('has a contact id and switches twice', async () => { const { getByText, findByText } = render( - - - - mocks={{ - UserOption: { - userOption: { - id: 'test-id', - key: 'contacts_view', - value: 'flows', - }, - }, - }} - > - - - - - - , + + + , ); + expect(getByText('Loading')).toBeInTheDocument(); userEvent.click(await findByText('Map Button')); expect(await findByText('map')).toBeInTheDocument(); @@ -198,33 +206,11 @@ describe('ContactsPageContext', () => { it('does not have a contact id and changes to map', async () => { const { getByText, findByText } = render( - - - - mocks={{ - UserOption: { - userOption: { - id: 'test-id', - key: 'contacts_view', - value: 'list', - }, - }, - }} - > - - - - - - , + + + , ); + expect(getByText('Loading')).toBeInTheDocument(); userEvent.click(await findByText('Map Button')); expect(await findByText('map')).toBeInTheDocument(); @@ -239,84 +225,41 @@ describe('ContactsPageContext', () => { ); }); + it('shows the selected contacts on the map', async () => { + const { findByRole, getByRole, getByTestId } = render( + + + , + ); + + userEvent.click(getByRole('button', { name: 'Select contact' })); + userEvent.click(await findByRole('button', { name: 'Map Button' })); + expect(getByTestId('active-filters')).toHaveTextContent( + '{"ids":["contact-1"]}', + ); + + userEvent.click(getByRole('button', { name: 'List Button' })); + expect(getByTestId('active-filters')).toHaveTextContent('{}'); + }); + it('Saved Filters with correct JSON', async () => { - const userOptions = [ - { - id: '123', - key: 'saved_contacts_filter_My_Cool_Filter', - value: `{"any_tags":false,"account_list_id":"${accountListId}","params":{"status": "true"},"tags":null,"exclude_tags":null,"wildcard_search":""}`, - }, - ]; + const savedFilter = `{"any_tags":false,"account_list_id":"${accountListId}","params":{"status": "true"},"tags":null,"exclude_tags":null,"wildcard_search":""}`; const { findByTestId } = render( - - - - mocks={{ - GetUserOptions: { - userOptions, - }, - ContactFilters: { - userOptions, - }, - }} - > - - - - - - , + + + , ); expect(await findByTestId('savedfilters-testid')).toBeInTheDocument(); }); it('Saved Filters with incorrect JSON', async () => { - const userOptions = [ - { - id: '123', - key: 'saved_contacts_filter_My_Cool_Filter', - value: `{"any_tags":false,"account_list_id":"${accountListId}","params":{"status" error },"tags":null,"exclude_tags":null,"wildcard_search":""}`, - }, - ]; + const savedFilter = `{"any_tags":false,"account_list_id":"${accountListId}","params":{"status" error },"tags":null,"exclude_tags":null,"wildcard_search":""}`; const { queryByTestId } = render( - - - - mocks={{ - GetUserOptions: { - userOptions, - }, - ContactFilters: { - userOptions, - }, - }} - > - - - - - - , + + + , ); await waitFor(() => diff --git a/src/components/Contacts/ContactsContext/ContactsContext.tsx b/src/components/Contacts/ContactsContext/ContactsContext.tsx index 80f144916..a822778c3 100644 --- a/src/components/Contacts/ContactsContext/ContactsContext.tsx +++ b/src/components/Contacts/ContactsContext/ContactsContext.tsx @@ -150,16 +150,16 @@ export const ContactsProvider: React.FC = ({ [activeFilters], ); - const [contactsView, updateOptions, { loading: userOptionsLoading }] = + const [contactsView, saveContactsView, { loading: userOptionsLoading }] = useUserPreference({ key: 'contacts_view', defaultValue: TableViewModeEnum.List, }); useEffect(() => { - if (contactsView && !userOptionsLoading) { + if (contactsView) { setViewMode(contactsView); } - }, [contactsView, userOptionsLoading]); + }, [contactsView]); const contactsFilters = useMemo(() => { // Remove filters in the map view @@ -257,8 +257,15 @@ export const ContactsProvider: React.FC = ({ _: React.MouseEvent, view: string, ) => { - setViewMode(view as TableViewModeEnum); - updateOptions(view as TableViewModeEnum); + const newViewMode = view as TableViewModeEnum; + saveContactsView(newViewMode); + if (newViewMode === TableViewModeEnum.Map) { + // When switching to the map, make the filter only show the selected contacts, if any + setActiveFilters({ ids }); + } else if (viewMode === TableViewModeEnum.Map) { + // When switching away from the map, reset the filter to show all contacts + setActiveFilters({}); + } }; //#endregion From c20995af34788b8d3e2ce6a8adb37c14f2b05f54 Mon Sep 17 00:00:00 2001 From: Caleb Cox Date: Wed, 23 Oct 2024 14:35:06 -0500 Subject: [PATCH 2/5] Fix flaky test --- .../ContactsLeftPanel/ContactsLeftPanel.test.tsx | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/components/Contacts/ContactsLeftPanel/ContactsLeftPanel.test.tsx b/src/components/Contacts/ContactsLeftPanel/ContactsLeftPanel.test.tsx index 542382ff2..dcf7a7d81 100644 --- a/src/components/Contacts/ContactsLeftPanel/ContactsLeftPanel.test.tsx +++ b/src/components/Contacts/ContactsLeftPanel/ContactsLeftPanel.test.tsx @@ -1,6 +1,6 @@ import { useContext, useEffect } from 'react'; import { ThemeProvider } from '@emotion/react'; -import { render, waitFor } from '@testing-library/react'; +import { render } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import TestRouter from '__tests__/util/TestRouter'; import { GqlMockedProvider } from '__tests__/util/graphqlMocking'; @@ -9,6 +9,7 @@ import { ContactsContext, ContactsType, } from 'src/components/Contacts/ContactsContext/ContactsContext'; +import { StatusEnum } from 'src/graphql/types.generated'; import { UserOptionQuery } from 'src/hooks/UserPreference.generated'; import theme from 'src/theme'; import { ContactsLeftPanel } from './ContactsLeftPanel'; @@ -33,7 +34,6 @@ jest.mock('notistack', () => ({ const userOptions = [ { - id: 'test-id', key: 'contacts_view', value: 'map', }, @@ -44,6 +44,7 @@ const mocks = { nodes: [ { name: 'Contact 1', + status: StatusEnum.PartnerFinancial, primaryAddress: { geo: '10,20', }, @@ -76,7 +77,7 @@ describe('ContactsLeftPanel', () => { return null; }; - const { getByText } = render( + const { findByText } = render( mocks={mocks}> @@ -91,9 +92,7 @@ describe('ContactsLeftPanel', () => { , ); - await waitFor(() => expect(getByText('Contact 1')).toBeInTheDocument()); - - userEvent.click(getByText('Contact 1')); + userEvent.click(await findByText('Contact 1')); expect(panTo).toHaveBeenCalledWith({ lat: 10, lng: 20 }); }); }); From 2863c7db692336528b46e41ddeaaed66824e6ebb Mon Sep 17 00:00:00 2001 From: Caleb Cox Date: Thu, 24 Oct 2024 16:09:58 -0500 Subject: [PATCH 3/5] Preserve the active filters when switching to map view --- .../contacts/ContactsWrapper.tsx | 19 +++++++----- .../ContactsContext/ContactsContext.tsx | 29 +++++-------------- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx b/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx index 6cb8c3e72..2128b0bdd 100644 --- a/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx +++ b/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx @@ -77,6 +77,15 @@ export const ContactsWrapper: React.FC = ({ ); const [filterPanelOpen, setFilterPanelOpen] = useState(true); + // Only allow the ids filter in map view, and remove the ids filter in other views + const sanitizedFilters = useMemo(() => { + const { ids, ...otherFilters } = sanitizeFilters(activeFilters); + if (viewMode === TableViewModeEnum.Map) { + return ids?.length ? { ids } : {}; + } + return otherFilters; + }, [viewMode, activeFilters]); + const getContactHrefObject: GetContactHrefObject = useCallback( (contactId) => { // Omit the filters and searchTerm from the previous query because we don't want them in the URL @@ -93,11 +102,7 @@ export const ContactsWrapper: React.FC = ({ } newQuery.contactId = queryContactId; - const sanitizedFilters = sanitizeFilters(activeFilters); - if ( - viewMode !== TableViewModeEnum.Map && - Object.keys(sanitizedFilters).length - ) { + if (Object.keys(sanitizedFilters).length) { newQuery.filters = encodeURIComponent(JSON.stringify(sanitizedFilters)); } @@ -109,7 +114,7 @@ export const ContactsWrapper: React.FC = ({ query: newQuery, }; }, - [viewMode, activeFilters, searchTerm, pathname], + [viewMode, sanitizedFilters, searchTerm, pathname], ); const urlQuery = useMemo(() => { @@ -125,7 +130,7 @@ export const ContactsWrapper: React.FC = ({ return ( = ({ const locale = useLocale(); const accountListId = useAccountListId() ?? ''; - const sanitizedFilters = useMemo( - () => sanitizeFilters(activeFilters), - [activeFilters], - ); - const [contactsView, saveContactsView, { loading: userOptionsLoading }] = useUserPreference({ key: 'contacts_view', @@ -161,18 +155,14 @@ export const ContactsProvider: React.FC = ({ } }, [contactsView]); - const contactsFilters = useMemo(() => { - // Remove filters in the map view - const viewFilters = - viewMode === TableViewModeEnum.Map - ? { ids: sanitizedFilters.ids } - : sanitizedFilters; - return { - ...viewFilters, + const contactsFilters = useMemo( + () => ({ + ...activeFilters, ...starredFilter, wildcardSearch: searchTerm as string, - }; - }, [sanitizedFilters, viewMode, starredFilter, searchTerm]); + }), + [activeFilters, starredFilter, searchTerm], + ); const contactsQueryResult = useContactsQuery({ variables: { @@ -261,10 +251,7 @@ export const ContactsProvider: React.FC = ({ saveContactsView(newViewMode); if (newViewMode === TableViewModeEnum.Map) { // When switching to the map, make the filter only show the selected contacts, if any - setActiveFilters({ ids }); - } else if (viewMode === TableViewModeEnum.Map) { - // When switching away from the map, reset the filter to show all contacts - setActiveFilters({}); + setActiveFilters({ ...activeFilters, ids }); } }; //#endregion @@ -318,7 +305,7 @@ export const ContactsProvider: React.FC = ({ mapData: mapData, panTo: panTo, activeFilters: activeFilters, - sanitizedFilters, + sanitizedFilters: activeFilters, setActiveFilters: setActiveFilters, starredFilter: starredFilter, setStarredFilter: setStarredFilter, From 8cc491dc5205b10b2cc9ffbfac2549b279b15082 Mon Sep 17 00:00:00 2001 From: Caleb Cox Date: Thu, 24 Oct 2024 16:25:43 -0500 Subject: [PATCH 4/5] fixup! Preserve the active filters when switching to map view --- .../[accountListId]/contacts/ContactsWrapper.tsx | 13 +++++++------ .../Contacts/ContactsContext/ContactsContext.tsx | 12 +++++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx b/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx index 2128b0bdd..a5832ec16 100644 --- a/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx +++ b/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx @@ -66,7 +66,7 @@ export const ContactsWrapper: React.FC = ({ } }); - const [activeFilters, setActiveFilters] = useState< + const [activeFiltersRaw, setActiveFilters] = useState< ContactFilterSetInput & TaskFilterSetInput >(JSON.parse(decodeURIComponent(getQueryParam(query, 'filters') ?? '{}'))); const [starredFilter, setStarredFilter] = useState< @@ -78,13 +78,13 @@ export const ContactsWrapper: React.FC = ({ const [filterPanelOpen, setFilterPanelOpen] = useState(true); // Only allow the ids filter in map view, and remove the ids filter in other views - const sanitizedFilters = useMemo(() => { - const { ids, ...otherFilters } = sanitizeFilters(activeFilters); + const activeFilters = useMemo(() => { + const { ids, ...otherFilters } = activeFiltersRaw; if (viewMode === TableViewModeEnum.Map) { return ids?.length ? { ids } : {}; } return otherFilters; - }, [viewMode, activeFilters]); + }, [viewMode, activeFiltersRaw]); const getContactHrefObject: GetContactHrefObject = useCallback( (contactId) => { @@ -102,6 +102,7 @@ export const ContactsWrapper: React.FC = ({ } newQuery.contactId = queryContactId; + const sanitizedFilters = sanitizeFilters(activeFilters); if (Object.keys(sanitizedFilters).length) { newQuery.filters = encodeURIComponent(JSON.stringify(sanitizedFilters)); } @@ -114,7 +115,7 @@ export const ContactsWrapper: React.FC = ({ query: newQuery, }; }, - [viewMode, sanitizedFilters, searchTerm, pathname], + [viewMode, activeFilters, searchTerm, pathname], ); const urlQuery = useMemo(() => { @@ -130,7 +131,7 @@ export const ContactsWrapper: React.FC = ({ return ( = ({ const locale = useLocale(); const accountListId = useAccountListId() ?? ''; + const sanitizedFilters = useMemo( + () => sanitizeFilters(activeFilters), + [activeFilters], + ); + const [contactsView, saveContactsView, { loading: userOptionsLoading }] = useUserPreference({ key: 'contacts_view', @@ -157,11 +163,11 @@ export const ContactsProvider: React.FC = ({ const contactsFilters = useMemo( () => ({ - ...activeFilters, + ...sanitizedFilters, ...starredFilter, wildcardSearch: searchTerm as string, }), - [activeFilters, starredFilter, searchTerm], + [sanitizedFilters, starredFilter, searchTerm], ); const contactsQueryResult = useContactsQuery({ @@ -305,7 +311,7 @@ export const ContactsProvider: React.FC = ({ mapData: mapData, panTo: panTo, activeFilters: activeFilters, - sanitizedFilters: activeFilters, + sanitizedFilters, setActiveFilters: setActiveFilters, starredFilter: starredFilter, setStarredFilter: setStarredFilter, From e75e66509154b326fb8ccfd2ad8d1f0967ed6dcd Mon Sep 17 00:00:00 2001 From: Caleb Cox Date: Fri, 25 Oct 2024 13:59:00 -0500 Subject: [PATCH 5/5] fixup! Preserve the active filters when switching to map view --- .../[accountListId]/contacts/ContactsWrapper.tsx | 9 +++++---- .../Contacts/ContactsContext/ContactsContext.tsx | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx b/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx index a5832ec16..f08820de9 100644 --- a/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx +++ b/pages/accountLists/[accountListId]/contacts/ContactsWrapper.tsx @@ -1,6 +1,7 @@ import { ParsedUrlQuery } from 'node:querystring'; import { useRouter } from 'next/router'; import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { omit } from 'lodash'; import { ContactsProvider } from 'src/components/Contacts/ContactsContext/ContactsContext'; import { TableViewModeEnum } from 'src/components/Shared/Header/ListHeader'; import { @@ -79,11 +80,11 @@ export const ContactsWrapper: React.FC = ({ // Only allow the ids filter in map view, and remove the ids filter in other views const activeFilters = useMemo(() => { - const { ids, ...otherFilters } = activeFiltersRaw; if (viewMode === TableViewModeEnum.Map) { - return ids?.length ? { ids } : {}; + return activeFiltersRaw; + } else { + return omit(activeFiltersRaw, 'ids'); } - return otherFilters; }, [viewMode, activeFiltersRaw]); const getContactHrefObject: GetContactHrefObject = useCallback( @@ -91,7 +92,7 @@ export const ContactsWrapper: React.FC = ({ // Omit the filters and searchTerm from the previous query because we don't want them in the URL // if they are empty and Next.js will still add them to the URL query even if they are undefined. // i.e. { filters: undefined, searchTerm: '' } results in a querystring of ?filters=&searchTerm - const { filters: _filters, searchTerm: _searchTerm, ...newQuery } = query; + const newQuery = omit(query, ['filters', 'searchTerm']); const queryContactId: string[] = []; if (addViewMode && viewMode !== TableViewModeEnum.List) { diff --git a/src/components/Contacts/ContactsContext/ContactsContext.tsx b/src/components/Contacts/ContactsContext/ContactsContext.tsx index 1c0d95713..9dc0c610c 100644 --- a/src/components/Contacts/ContactsContext/ContactsContext.tsx +++ b/src/components/Contacts/ContactsContext/ContactsContext.tsx @@ -255,7 +255,7 @@ export const ContactsProvider: React.FC = ({ ) => { const newViewMode = view as TableViewModeEnum; saveContactsView(newViewMode); - if (newViewMode === TableViewModeEnum.Map) { + if (newViewMode === TableViewModeEnum.Map && ids.length > 0) { // When switching to the map, make the filter only show the selected contacts, if any setActiveFilters({ ...activeFilters, ids }); }