Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [M3-7883] - Add search and alphabetical sorting to Switch Account drawer #10515

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10515-added-1716569243109.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Added
---

Alphabetical account sorting and search capabilities to Switch Account drawer ([#10515](https://github.com/linode/manager/pull/10515))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this test

Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,72 @@ describe('Parent/Child account switching', () => {
mockChildAccount.company
);
});

/*
* - Confirms search functionality in the account switching drawer.
*/
it('can search child accounts', () => {
mockGetProfile(mockParentProfile);
mockGetAccount(mockParentAccount);
mockGetChildAccounts([mockChildAccount, mockAlternateChildAccount]);
mockGetUser(mockParentUser);

cy.visitWithLogin('/');
cy.trackPageVisit().as('pageVisit');

// Confirm that Parent account username and company name are shown in user
// menu button, then click the button.
assertUserMenuButton(
mockParentProfile.username,
mockParentAccount.company
).click();

// Click "Switch Account" button in user menu.
ui.userMenu
.find()
.should('be.visible')
.within(() => {
ui.button
.findByTitle('Switch Account')
.should('be.visible')
.should('be.enabled')
.click();
});

// Confirm search functionality.
ui.drawer
.findByTitle('Switch Account')
.should('be.visible')
.within(() => {
// Confirm all child accounts are displayed when drawer loads.
cy.findByText(mockChildAccount.company).should('be.visible');
cy.findByText(mockAlternateChildAccount.company).should('be.visible');

// Confirm no results message.
mockGetChildAccounts([]).as('getEmptySearchResults');
cy.findByPlaceholderText('Search').click().type('Fake Name');
cy.wait('@getEmptySearchResults');

cy.contains(mockChildAccount.company).should('not.exist');
cy.findByText(
'There are no child accounts that match this query.'
).should('be.visible');

// Confirm filtering by company name displays only one search result.
mockGetChildAccounts([mockChildAccount]).as('getSearchResults');
cy.findByPlaceholderText('Search')
.click()
.clear()
.type(mockChildAccount.company);
cy.wait('@getSearchResults');

cy.findByText(mockChildAccount.company).should('be.visible');
cy.contains(mockAlternateChildAccount.company).should('not.exist');
cy.contains(
'There are no child accounts that match this query.'
).should('not.exist');
});
});
});

/**
Expand Down Expand Up @@ -378,9 +444,7 @@ describe('Parent/Child account switching', () => {
.findByTitle('Switch Account')
.should('be.visible')
.within(() => {
cy.findByText('There are no indirect customer accounts.').should(
'be.visible'
);
cy.findByText('There are no child accounts.').should('be.visible');
cy.findByText('switch back to your account')
.should('be.visible')
.click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ describe('SwitchAccountDrawer', () => {
).toBeInTheDocument();
});

it('should have a search bar', () => {
const { getByText } = renderWithTheme(<SwitchAccountDrawer {...props} />);

expect(getByText('Search')).toBeVisible();
});

it('should include a link to switch back to the parent account if the active user is a proxy user', async () => {
server.use(
http.get('*/profile', () => {
Expand Down
16 changes: 16 additions & 0 deletions packages/manager/src/features/Account/SwitchAccountDrawer.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';

import { StyledLinkButton } from 'src/components/Button/StyledLinkButton';
import { DebouncedSearchTextField } from 'src/components/DebouncedSearchTextField';
import { Drawer } from 'src/components/Drawer';
import { Notice } from 'src/components/Notice/Notice';
import { Typography } from 'src/components/Typography';
Expand Down Expand Up @@ -37,6 +38,11 @@ export const SwitchAccountDrawer = (props: Props) => {
const [isParentTokenError, setIsParentTokenError] = React.useState<
APIError[]
>([]);
const filter = {};
const [query, setQuery] = React.useState<string>('');
if (query) {
filter['company'] = { '+contains': query };
}

const isProxyUser = userType === 'proxy';
const currentParentTokenWithBearer =
Expand Down Expand Up @@ -154,10 +160,20 @@ export const SwitchAccountDrawer = (props: Props) => {
)}
.
</Typography>
<DebouncedSearchTextField
debounceTime={250}
hideLabel
label="Search"
onSearch={(query) => setQuery(query)}
mjac0bs marked this conversation as resolved.
Show resolved Hide resolved
placeholder="Search"
sx={{ marginBottom: 3 }}
value={query}
/>
<ChildAccountList
currentTokenWithBearer={
isProxyUser ? currentParentTokenWithBearer : currentTokenWithBearer
}
filter={Object.keys(filter).length > 0 ? filter : undefined}
isLoading={isSubmitting}
onClose={onClose}
onSwitchAccount={handleSwitchToChildAccount}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,69 @@ const props = {
userType: undefined,
};

it('should display a list of child accounts', async () => {
beforeAll(() => {
server.use(
http.get('*/profile', () => {
return HttpResponse.json(profileFactory.build({ user_type: 'parent' }));
}),
})
);
});

it('should display a list of child accounts', async () => {
const { findByTestId } = renderWithTheme(<ChildAccountList {...props} />);
server.use(
http.get('*/account/child-accounts', () => {
return HttpResponse.json(
makeResourcePage(accountFactory.buildList(5, { company: 'Child Co.' }))
);
})
);

const { findByTestId } = renderWithTheme(<ChildAccountList {...props} />);

await waitFor(async () => {
expect(await findByTestId('child-account-list')).not.toBeNull();
});

const childAccounts = await findByTestId('child-account-list');

// Confirm that all child accounts are listed.
expect(
within(childAccounts).getAllByText('Child Co.', { exact: false })
).toHaveLength(5);
});

it('should display the list of child accounts in alphabetical order', async () => {
const mockChildAccounts = [
accountFactory.build({ company: 'Z Child Co.' }),
accountFactory.build({ company: '42 Child Co.' }),
accountFactory.build({ company: 'Z Child Co. 2' }),
accountFactory.build({ company: 'A Child Co.' }),
];
const mockAlphabeticalCompanyNames = [
'42 Child Co.',
'A Child Co.',
'Z Child Co.',
'Z Child Co. 2',
];

server.use(
http.get('*/account/child-accounts', () => {
return HttpResponse.json(makeResourcePage(mockChildAccounts));
})
);

const { findByTestId, getAllByRole } = renderWithTheme(
<ChildAccountList {...props} />
);

await waitFor(async () => {
expect(await findByTestId('child-account-list')).not.toBeNull();
});

const childAccounts = await findByTestId('child-account-list');
const childAccountCompanyNames = getAllByRole('button').map(
(account) => account.textContent
);

expect(childAccounts).toBeInTheDocument();
// Confirm that child accounts are listed in alphabetical order by company name.
expect(childAccountCompanyNames).toEqual(mockAlphabeticalCompanyNames);
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ import { Stack } from 'src/components/Stack';
import { Typography } from 'src/components/Typography';
import { useChildAccountsInfiniteQuery } from 'src/queries/account/account';

import type { UserType } from '@linode/api-v4';
import type { Filter, UserType } from '@linode/api-v4';

interface ChildAccountListProps {
currentTokenWithBearer: string;
filter?: Filter;
isLoading?: boolean;
onClose: () => void;
onSwitchAccount: (props: {
Expand All @@ -30,6 +31,7 @@ interface ChildAccountListProps {
export const ChildAccountList = React.memo(
({
currentTokenWithBearer,
filter,
isLoading,
onClose,
onSwitchAccount,
Expand All @@ -46,18 +48,28 @@ export const ChildAccountList = React.memo(
isError,
isFetchingNextPage,
isInitialLoading,
isRefetching,
refetch: refetchChildAccounts,
} = useChildAccountsInfiniteQuery({
filter,
headers:
userType === 'proxy'
? {
Authorization: currentTokenWithBearer,
}
: undefined,
});
const childAccounts = data?.pages.flatMap((page) => page.data);
// Sort the list of child accounts alphabetically.
mjac0bs marked this conversation as resolved.
Show resolved Hide resolved
const childAccounts = data?.pages
.flatMap((page) => page.data)
.sort((a, b) => a.company.localeCompare(b.company));

mjac0bs marked this conversation as resolved.
Show resolved Hide resolved
if (isInitialLoading) {
if (
isInitialLoading ||
isLoading ||
isSwitchingChildAccounts ||
isRefetching
) {
return (
<Box display="flex" justifyContent="center">
<CircleProgress mini size={70} />
Expand All @@ -67,7 +79,10 @@ export const ChildAccountList = React.memo(

if (childAccounts?.length === 0) {
return (
<Notice variant="info">There are no indirect customer accounts.</Notice>
<Notice variant="info">
There are no child accounts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not using "indirect customer", so updated the terminology here.

{filter ? ' that match this query' : undefined}.
</Notice>
);
}

Expand Down Expand Up @@ -119,11 +134,6 @@ export const ChildAccountList = React.memo(

return (
<Stack alignItems={'flex-start'} data-testid="child-account-list">
{(isSwitchingChildAccounts || isLoading) && (
<Box display="flex" justifyContent="center" width={'100%'}>
<CircleProgress mini size={70} />
</Box>
)}
Comment on lines -122 to -126
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we were setting up loading spinners in two different places, but I removed this and added all conditions to L67-72. The addition of isRefetching handles loading state for the search queries. Reviewers should check that they're not seeing any unexpected behavior in our handling of loading state.

{!isSwitchingChildAccounts && !isLoading && renderChildAccounts}
{hasNextPage && <Waypoint onEnter={() => fetchNextPage()} />}
{isFetchingNextPage && <CircleProgress mini />}
Expand Down
6 changes: 5 additions & 1 deletion packages/manager/src/mocks/serverHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1354,7 +1354,11 @@ export const handlers = [
const url = new URL(request.url);
const page = Number(url.searchParams.get('page') || 1);
const pageSize = Number(url.searchParams.get('page_size') || 25);
const childAccounts = accountFactory.buildList(100);
const childAccounts = [
accountFactory.build({ company: 'z-company-2' }),
accountFactory.build({ company: 'z-company' }),
...accountFactory.buildList(100),
];
return HttpResponse.json({
data: childAccounts.slice(
(page - 1) * pageSize,
Expand Down
Loading