Skip to content
22 changes: 17 additions & 5 deletions src/hooks/usePersonalDetailOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type {LocaleContextProps} from '@components/LocaleContextProvider';
import {usePersonalDetails} from '@components/OnyxListItemProvider';

import memoize, {equivalentArgsComparator} from '@libs/memoize';
import {createOptionList, filterPersonalDetailsByLogins} from '@libs/PersonalDetailOptionsListUtils';
import {createOptionList, filterAgentsPersonalDetailsByAccountIDs, filterPersonalDetailsByLogins} from '@libs/PersonalDetailOptionsListUtils';
import type {OptionData, PrivateIsArchivedMap} from '@libs/PersonalDetailOptionsListUtils/types';
import {isOneOnOneChat, isSelfDM} from '@libs/ReportUtils';
import {registerSessionCleanupCallback} from '@libs/SessionCleanup';
Expand All @@ -23,6 +23,10 @@ type UseFilteredOptionsConfig = {
enabled?: boolean;
/** When set, only the personal details whose login is in this set are turned into options */
includeLoginsOnly?: Set<string>;
/**
* When set, only the agents in this set are turned into options.
*/
agentsWhitelist?: Set<number>;
/* Whether to include report errors in the option data (default: false) */
shouldStoreReportErrors?: boolean;
/* Whether to include brick road indicator status in the option data (default: false) */
Expand Down Expand Up @@ -108,7 +112,7 @@ const createReportAttributesSelector =
/**
* Building an option per personal details entry is the expensive step of this hook and depends only on Onyx values.
* Arguments are compared one level deep because the maps are rebuilt each render from those same unchanged values.
* A single entry is held, so a consumer narrowing the list with `includeLoginsOnly` evicts the whole-list one.
* A single entry is held, so a consumer narrowing the list with `includeLoginsOnly` or `agentsWhitelist` evicts the whole-list one.
*/
const memoizedCreateOptionList = memoize(createOptionList, {
maxSize: 1,
Expand All @@ -128,6 +132,7 @@ type BuildOptionsDataParams = {
accountID: number;
allPersonalDetails: OnyxEntry<PersonalDetailsList>;
includeLoginsOnly: Set<string> | undefined;
agentsWhitelist: Set<number> | undefined;
reports: OnyxCollection<Report>;
reportAttributes: FilteredReportAttributes | undefined;
reportNameValuePairs: PrivateIsArchivedMap | undefined;
Expand All @@ -142,6 +147,7 @@ function buildOptionsData({
accountID,
allPersonalDetails,
includeLoginsOnly,
agentsWhitelist,
reports,
reportAttributes,
reportNameValuePairs,
Expand All @@ -150,9 +156,14 @@ function buildOptionsData({
shouldStoreReportErrors,
shouldShowBrickRoadIndicator,
}: BuildOptionsDataParams) {
const personalDetails = includeLoginsOnly ? filterPersonalDetailsByLogins(allPersonalDetails, includeLoginsOnly) : allPersonalDetails;
let personalDetails = allPersonalDetails;
if (includeLoginsOnly) {
personalDetails = filterPersonalDetailsByLogins(personalDetails, includeLoginsOnly);
}
if (agentsWhitelist) {
personalDetails = filterAgentsPersonalDetailsByAccountIDs(personalDetails, agentsWhitelist);
}
const accountIDToReportIDMap = generateAccountIDToReportIDMap(reports, accountID);

return memoizedCreateOptionList(accountID, personalDetails, accountIDToReportIDMap, reports, reportAttributes, reportNameValuePairs ?? {}, formatPhoneNumber, translate, {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NAB: memoizedCreateOptionList has maxSize: 1, and the invite page now always requests a narrowed list, so switching between this page and any full-list consumer rebuilds the whole option list each time. Probably fine, but the doc comment above memoizedCreateOptionList only mentions includeLoginsOnly evicting the whole-list entry. Can we mention includeAgentsOnly too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated comment

shouldStoreReportErrors,
shouldShowBrickRoadIndicator,
Expand All @@ -179,7 +190,7 @@ function buildOptionsData({
* />
*/
function usePersonalDetailOptions(config: UseFilteredOptionsConfig = {}): UseFilteredOptionsResult {
const {enabled = true, includeLoginsOnly, shouldStoreReportErrors = false, shouldShowBrickRoadIndicator = false} = config;
const {enabled = true, includeLoginsOnly, agentsWhitelist, shouldStoreReportErrors = false, shouldShowBrickRoadIndicator = false} = config;

const reportAttributesSelector = createReportAttributesSelector(shouldStoreReportErrors, shouldShowBrickRoadIndicator);

Expand All @@ -200,6 +211,7 @@ function usePersonalDetailOptions(config: UseFilteredOptionsConfig = {}): UseFil
accountID,
allPersonalDetails,
includeLoginsOnly,
agentsWhitelist,
reports,
reportAttributes,
reportNameValuePairs,
Expand Down
8 changes: 7 additions & 1 deletion src/hooks/usePersonalDetailSearchSelector/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ type UseSearchSelectorConfig = {
/** When set, only the logins in this set are turned into options */
includeLoginsOnly?: Set<string>;

/**
* When set, only the agents in this set are turned into options.
*/
agentsWhitelist?: Set<number>;

includeRecentReports?: boolean;
includeCurrentUser?: boolean;
includeDomainEmail?: boolean;
Expand Down Expand Up @@ -187,6 +192,7 @@ function usePersonalDetailSearchSelectorBase({
excludeLogins = CONST.EMPTY_OBJECT,
excludeFromSuggestionsOnly = CONST.EMPTY_OBJECT,
includeLoginsOnly,
agentsWhitelist,
includeRecentReports = true,
onSelectionChange,
onSingleSelect,
Expand All @@ -203,7 +209,7 @@ function usePersonalDetailSearchSelectorBase({
}: UseSearchSelectorConfig): UseSearchSelectorReturn {
const {translate, formatPhoneNumber} = useLocalize();
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
const {options: defaultOptions, currentOption, isLoading: isPersonalDetailsOptionsLoading} = usePersonalDetailOptions({enabled: shouldInitialize, includeLoginsOnly});
const {options: defaultOptions, currentOption, isLoading: isPersonalDetailsOptionsLoading} = usePersonalDetailOptions({enabled: shouldInitialize, includeLoginsOnly, agentsWhitelist});
const [countryCode = CONST.DEFAULT_COUNTRY_CODE] = useOnyx(ONYXKEYS.COUNTRY_CODE);
const [loginList] = useOnyx(ONYXKEYS.LOGINS, {selector: expensifyLoginsSelector});

Expand Down
16 changes: 16 additions & 0 deletions src/libs/PersonalDetailOptionsListUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,21 @@ function filterPersonalDetailsByLogins(personalDetails: OnyxEntry<PersonalDetail
return filteredPersonalDetails;
}

/**
* Excludes the personal details of agents whom account ID is not in the whitelist `accountIDs`
* Meant to be applied before createOptionList so that agents you do not own are not listed (e.g. when inviting members to workspace)
*/
function filterAgentsPersonalDetailsByAccountIDs(personalDetails: OnyxEntry<PersonalDetailsList>, accountIDs: Set<number>): PersonalDetailsList {
const filteredPersonalDetails: PersonalDetailsList = {};
for (const [accountID, personalDetail] of Object.entries(personalDetails ?? {})) {
if (personalDetail?.isCustomAgent && !accountIDs.has(Number(accountID))) {
continue;
}
filteredPersonalDetails[accountID] = personalDetail;
}
return filteredPersonalDetails;
}

/**
* Helper method that returns the text to be used for the header's message and title (if any)
*/
Expand Down Expand Up @@ -565,6 +580,7 @@ export {
getValidOptions,
createOptionList,
filterPersonalDetailsByLogins,
filterAgentsPersonalDetailsByAccountIDs,
getHeaderMessage,
getUserToInviteOption,
getFilteredRecentAttendees,
Expand Down
5 changes: 5 additions & 0 deletions src/pages/workspace/DynamicWorkspaceInvitePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import SelectionListWithSections from '@components/SelectionList/SelectionListWi
import type {Section} from '@components/SelectionList/SelectionListWithSections/types';
import withNavigationTransitionEnd from '@components/withNavigationTransitionEnd';

import useAgents from '@hooks/useAgents';
import useDynamicBackPath from '@hooks/useDynamicBackPath';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
Expand Down Expand Up @@ -66,6 +67,9 @@ function DynamicWorkspaceInvitePageContent({route, policy, invitedEmailsToAccoun
const [personalDetails] = useAllPersonalDetails();
const [account] = useOnyx(ONYXKEYS.ACCOUNT);
const employeePersonalDetails = usePersonalDetailsByLogins(Object.keys(policy?.employeeList ?? {}));
const {agents} = useAgents();
const agentsAccountIDs = new Set(agents.map(({accountID}) => accountID));

const openWorkspaceInvitePage = () => {
const policyMemberEmailsToAccountIDs = getMemberAccountIDsForWorkspace(policy?.employeeList, employeePersonalDetails);
policyOpenWorkspaceInvitePage(route.params.policyID, Object.keys(policyMemberEmailsToAccountIDs));
Expand Down Expand Up @@ -139,6 +143,7 @@ function DynamicWorkspaceInvitePageContent({route, policy, invitedEmailsToAccoun
initialSelected,
initialExtraOptions,
shouldKeepSelectedInAvailableOptions: true,
agentsWhitelist: agentsAccountIDs,
});

const sections: Array<Section<OptionData>> = useMemo(() => {
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspaceInitialPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, route}: Workspac
highlighted={!!item?.highlighted}
focused={getItemScreenNames(item).some((screenName) => activeRoute?.startsWith(screenName))}
role={CONST.ROLE.TAB}
badgeText={item.badgeText}
badgeText={item.badgeTranslationKey ? translate(item.badgeTranslationKey) : item.badgeText}
shouldIconUseAutoWidthStyle
sentryLabel={item.sentryLabel}
shouldGreyOutWhenDisabled={hasPolicyCreationError}
Expand Down
2 changes: 2 additions & 0 deletions src/pages/workspace/getWorkspaceMenuItems.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ type WorkspaceMenuItem = WithSentryLabel & {
getRoute: () => Route;
brickRoadIndicator?: ValueOf<typeof CONST.BRICK_ROAD_INDICATOR_STATUS>;
screenName: WorkspaceTopLevelScreens;
badgeTranslationKey?: TranslationPaths;
badgeText?: string;
highlighted?: boolean;

Expand Down Expand Up @@ -265,6 +266,7 @@ function getWorkspaceMenuItems({
getRoute: () => ROUTES.WORKSPACE_AGENTS.getRoute(policyID),
screenName: SCREENS.WORKSPACE.AGENTS,
sentryLabel: CONST.SENTRY_LABEL.WORKSPACE.INITIAL.AGENTS,
badgeTranslationKey: 'common.beta',
});
}

Expand Down
27 changes: 24 additions & 3 deletions tests/unit/usePersonalDetailOptionsTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ const PERSONAL_DETAILS: PersonalDetailsList = Object.fromEntries(
{accountID: 2, login: '[email protected]', displayName: 'Member'},
{accountID: 3, login: '[email protected]', displayName: 'Outsider'},
{accountID: 4, login: '[email protected]', displayName: 'Other'},
{accountID: 5, login: '[email protected]', displayName: 'Agent A', isCustomAgent: true},
{accountID: 6, login: '[email protected]', displayName: 'Agent B', isCustomAgent: true},
].map((personalDetail) => [personalDetail.accountID, personalDetail]),
);

Expand All @@ -38,8 +40,8 @@ function Wrapper({children}: {children: React.ReactNode}) {
);
}

const renderOptions = async (includeLoginsOnly?: Set<string>) => {
const {result} = renderHook(() => usePersonalDetailOptions({includeLoginsOnly}), {wrapper: Wrapper});
const renderOptions = async (includeLoginsOnly?: Set<string>, agentsWhitelist?: Set<number>) => {
const {result} = renderHook(() => usePersonalDetailOptions({includeLoginsOnly, agentsWhitelist}), {wrapper: Wrapper});
await waitForBatchedUpdatesWithAct();
return result;
};
Expand Down Expand Up @@ -73,7 +75,14 @@ describe('usePersonalDetailOptions', () => {
it('builds an option for every personal detail when no allowlist is given', async () => {
const result = await renderOptions();

expect(loginsOf(result.current.options)).toEqual(['[email protected]', '[email protected]', '[email protected]', '[email protected]']);
expect(loginsOf(result.current.options)).toEqual([
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
]);
expect(result.current.currentOption?.login).toBe(CURRENT_USER_EMAIL);
expect(result.current.isLoading).toBe(false);
});
Expand Down Expand Up @@ -103,4 +112,16 @@ describe('usePersonalDetailOptions', () => {
expect(result.current.options).toEqual([]);
expect(result.current.isLoading).toBe(false);
});

it('excludes agents not in agents allow list', async () => {
const result = await renderOptions(undefined, new Set([5]));

expect(loginsOf(result.current.options)).toEqual(['[email protected]', '[email protected]', '[email protected]', '[email protected]', '[email protected]']);
});

it('excludes all agents if allow list is empty', async () => {
const result = await renderOptions(undefined, new Set());

expect(loginsOf(result.current.options)).toEqual(['[email protected]', '[email protected]', '[email protected]', '[email protected]']);
});
});
Loading