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.

Sign up for GitHub

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

Fix workspace list doesn't get updated unless user opens workspace chat #47958

Merged
merged 2 commits into from
Sep 2, 2024
Merged

Fix workspace list doesn't get updated unless user opens workspace chat #47958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee12dfd
check user role from employeeList
bernhardoj Aug 24, 2024
5bdd016
lint
bernhardoj Aug 24, 2024
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: 3 additions & 2 deletions src/components/MoneyRequestConfirmationListFooter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ function MoneyRequestConfirmationListFooter({
const {translate, toLocaleDigit} = useLocalize();
const {isOffline} = useNetwork();
const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email});

// A flag and a toggler for showing the rest of the form fields
const [shouldExpandFields, toggleShouldExpandFields] = useReducer((state) => !state, false);
Expand All @@ -232,8 +233,8 @@ function MoneyRequestConfirmationListFooter({
const canUpdateSenderWorkspace = useMemo(() => {
const isInvoiceRoomParticipant = selectedParticipants.some((participant) => participant.isInvoiceRoom);

return PolicyUtils.canSendInvoice(allPolicies) && !!transaction?.isFromGlobalCreate && !isInvoiceRoomParticipant;
}, [allPolicies, selectedParticipants, transaction?.isFromGlobalCreate]);
return PolicyUtils.canSendInvoice(allPolicies, currentUserLogin) && !!transaction?.isFromGlobalCreate && !isInvoiceRoomParticipant;
}, [allPolicies, currentUserLogin, selectedParticipants, transaction?.isFromGlobalCreate]);

const isTypeSend = iouType === CONST.IOU.TYPE.PAY;
const taxRates = policy?.taxRates ?? null;
Expand Down
5 changes: 2 additions & 3 deletions src/components/SettlementButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,9 @@ function SettlementButton({
const {translate} = useLocalize();
const {isOffline} = useNetwork();
const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID);

const primaryPolicy = useMemo(() => PolicyActions.getPrimaryPolicy(activePolicyID), [activePolicyID]);

const session = useSession();
const primaryPolicy = useMemo(() => PolicyActions.getPrimaryPolicy(activePolicyID, session?.email), [activePolicyID, session?.email]);

// The app would crash due to subscribing to the entire report collection if chatReportID is an empty string. So we should have a fallback ID here.
const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${chatReportID || -1}`);
const isInvoiceReport = (!isEmptyObject(iouReport) && ReportUtils.isInvoiceReport(iouReport)) || false;
Expand Down
22 changes: 12 additions & 10 deletions src/libs/PolicyUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,19 +159,23 @@ function getPolicyBrickRoadIndicatorStatus(policy: OnyxEntry<Policy>): ValueOf<t
return undefined;
}

function getPolicyRole(policy: OnyxInputOrEntry<Policy>, currentUserLogin: string | undefined) {
return policy?.role ?? policy?.employeeList?.[currentUserLogin ?? '-1']?.role;
}

/**
* Check if the policy can be displayed
* If offline, always show the policy pending deletion.
* If online, show the policy pending deletion only if there is an error.
* Note: Using a local ONYXKEYS.NETWORK subscription will cause a delay in
* updating the screen. Passing the offline status from the component.
*/
function shouldShowPolicy(policy: OnyxEntry<Policy>, isOffline: boolean): boolean {
function shouldShowPolicy(policy: OnyxEntry<Policy>, isOffline: boolean, currentUserLogin: string | undefined): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's subscribe in src/libs/PolicyUtils.ts to ONYXKEYS.SESSION instead of using a currentUserLogin argument and having to pass it in shouldShowPolicy references

let currentUserLogin: string | undefined;
Onyx.connect({
    key: ONYXKEYS.SESSION,
    callback: (value) => {
        currentUserLogin = value?.email;
    },
});

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 initially wanna do that, but then I remembered that the user email could be changed (from the contact method page, from another client), so to keep everything reactive, I pass it as a param instead. If it's an account ID, then I will connect it to the lib file.

return (
!!policy &&
(policy?.type !== CONST.POLICY.TYPE.PERSONAL || !!policy?.isJoinRequestPending) &&
(isOffline || policy?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || Object.keys(policy.errors ?? {}).length > 0) &&
!!policy?.role
!!getPolicyRole(policy, currentUserLogin)
);
}

Expand All @@ -183,14 +187,12 @@ function isExpensifyTeam(email: string | undefined): boolean {
/**
* Checks if the current user is an admin of the policy.
*/
const isPolicyAdmin = (policy: OnyxInputOrEntry<Policy>, currentUserLogin?: string): boolean =>
(policy?.role ?? (currentUserLogin && policy?.employeeList?.[currentUserLogin]?.role)) === CONST.POLICY.ROLE.ADMIN;
const isPolicyAdmin = (policy: OnyxInputOrEntry<Policy>, currentUserLogin?: string): boolean => getPolicyRole(policy, currentUserLogin) === CONST.POLICY.ROLE.ADMIN;

/**
* Checks if the current user is of the role "user" on the policy.
*/
const isPolicyUser = (policy: OnyxInputOrEntry<Policy>, currentUserLogin?: string): boolean =>
(policy?.role ?? (currentUserLogin && policy?.employeeList?.[currentUserLogin]?.role)) === CONST.POLICY.ROLE.USER;
const isPolicyUser = (policy: OnyxInputOrEntry<Policy>, currentUserLogin?: string): boolean => getPolicyRole(policy, currentUserLogin) === CONST.POLICY.ROLE.USER;

/**
* Checks if the policy is a free group policy.
Expand Down Expand Up @@ -557,9 +559,9 @@ function getPolicy(policyID: string | undefined): OnyxEntry<Policy> {
}

/** Return active policies where current user is an admin */
function getActiveAdminWorkspaces(policies: OnyxCollection<Policy> | null): Policy[] {
function getActiveAdminWorkspaces(policies: OnyxCollection<Policy> | null, currentUserLogin: string | undefined): Policy[] {
const activePolicies = getActivePolicies(policies);
return activePolicies.filter((policy) => shouldShowPolicy(policy, NetworkStore.isOffline()) && isPolicyAdmin(policy));
return activePolicies.filter((policy) => shouldShowPolicy(policy, NetworkStore.isOffline(), currentUserLogin) && isPolicyAdmin(policy, currentUserLogin));
}

/** Whether the user can send invoice from the workspace */
Expand All @@ -569,8 +571,8 @@ function canSendInvoiceFromWorkspace(policyID: string | undefined): boolean {
}

/** Whether the user can send invoice */
function canSendInvoice(policies: OnyxCollection<Policy> | null): boolean {
return getActiveAdminWorkspaces(policies).length > 0;
function canSendInvoice(policies: OnyxCollection<Policy> | null, currentUserLogin: string | undefined): boolean {
return getActiveAdminWorkspaces(policies, currentUserLogin).length > 0;
// TODO: Uncomment the following line when the invoices screen is ready - https://github.com/Expensify/App/issues/45175.
// return getActiveAdminWorkspaces(policies).some((policy) => canSendInvoiceFromWorkspace(policy.id));
}
Expand Down
8 changes: 4 additions & 4 deletions src/libs/actions/Policy/Policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ function getPolicy(policyID: string | undefined): OnyxEntry<Policy> {
* Returns a primary policy for the user
*/
// TODO: Use getInvoicePrimaryWorkspace when the invoices screen is ready - https://github.com/Expensify/App/issues/45175.
function getPrimaryPolicy(activePolicyID?: OnyxEntry<string>): Policy | undefined {
const activeAdminWorkspaces = PolicyUtils.getActiveAdminWorkspaces(allPolicies);
function getPrimaryPolicy(activePolicyID: OnyxEntry<string>, currentUserLogin: string | undefined): Policy | undefined {
const activeAdminWorkspaces = PolicyUtils.getActiveAdminWorkspaces(allPolicies, currentUserLogin);
const primaryPolicy: Policy | null | undefined = activeAdminWorkspaces.find((policy) => policy.id === activePolicyID);
return primaryPolicy ?? activeAdminWorkspaces[0];
}
Expand All @@ -224,11 +224,11 @@ function hasInvoicingDetails(policy: OnyxEntry<Policy>): boolean {
/**
* Returns a primary invoice workspace for the user
*/
function getInvoicePrimaryWorkspace(activePolicyID?: OnyxEntry<string>): Policy | undefined {
function getInvoicePrimaryWorkspace(activePolicyID: OnyxEntry<string>, currentUserLogin: string | undefined): Policy | undefined {
if (PolicyUtils.canSendInvoiceFromWorkspace(activePolicyID)) {
return allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${activePolicyID ?? '-1'}`];
}
const activeAdminWorkspaces = PolicyUtils.getActiveAdminWorkspaces(allPolicies);
const activeAdminWorkspaces = PolicyUtils.getActiveAdminWorkspaces(allPolicies, currentUserLogin);
return activeAdminWorkspaces.find((policy) => PolicyUtils.canSendInvoiceFromWorkspace(policy.id));
}

Expand Down
5 changes: 3 additions & 2 deletions src/pages/WorkspaceSwitcherPage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ function WorkspaceSwitcherPage() {
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const [reportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS);
const [policies, fetchStatus] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email});

const brickRoadsForPolicies = useMemo(() => getWorkspacesBrickRoads(reports, policies, reportActions), [reports, policies, reportActions]);
const unreadStatusesForPolicies = useMemo(() => getWorkspacesUnreadStatuses(reports), [reports]);
Expand Down Expand Up @@ -104,7 +105,7 @@ function WorkspaceSwitcherPage() {
}

return Object.values(policies)
.filter((policy) => PolicyUtils.shouldShowPolicy(policy, !!isOffline) && !policy?.isJoinRequestPending)
.filter((policy) => PolicyUtils.shouldShowPolicy(policy, !!isOffline, currentUserLogin) && !policy?.isJoinRequestPending)
.map((policy) => ({
text: policy?.name ?? '',
policyID: policy?.id ?? '-1',
Expand All @@ -123,7 +124,7 @@ function WorkspaceSwitcherPage() {
isPolicyAdmin: PolicyUtils.isPolicyAdmin(policy),
isSelected: activeWorkspaceID === policy?.id,
}));
}, [policies, isOffline, getIndicatorTypeForPolicy, hasUnreadData, activeWorkspaceID]);
}, [policies, isOffline, currentUserLogin, getIndicatorTypeForPolicy, hasUnreadData, activeWorkspaceID]);

const filteredAndSortedUserWorkspaces = useMemo<WorkspaceListItem[]>(
() =>
Expand Down
7 changes: 5 additions & 2 deletions src/pages/home/report/SystemChatReportFooterMessage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ type SystemChatReportFooterMessageProps = SystemChatReportFooterMessageOnyxProps
function SystemChatReportFooterMessage({choice, policies, activePolicyID}: SystemChatReportFooterMessageProps) {
const {translate} = useLocalize();
const styles = useThemeStyles();
const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email});

const adminChatReportID = useMemo(() => {
const adminPolicy = activePolicyID
? PolicyUtils.getPolicy(activePolicyID)
: Object.values(policies ?? {}).find((policy) => PolicyUtils.shouldShowPolicy(policy, false) && policy?.role === CONST.POLICY.ROLE.ADMIN && policy?.chatReportIDAdmins);
: Object.values(policies ?? {}).find(
(policy) => PolicyUtils.shouldShowPolicy(policy, false, currentUserLogin) && policy?.role === CONST.POLICY.ROLE.ADMIN && policy?.chatReportIDAdmins,
);

return String(adminPolicy?.chatReportIDAdmins ?? -1);
}, [activePolicyID, policies]);
}, [activePolicyID, policies, currentUserLogin]);

const [adminChatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${adminChatReportID}`);

Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ function FloatingActionButtonAndPopover(
const {isOffline} = useNetwork();

const {canUseSpotnanaTravel} = usePermissions();
const canSendInvoice = useMemo(() => PolicyUtils.canSendInvoice(allPolicies as OnyxCollection<OnyxTypes.Policy>), [allPolicies]);
const canSendInvoice = useMemo(() => PolicyUtils.canSendInvoice(allPolicies as OnyxCollection<OnyxTypes.Policy>, session?.email), [allPolicies, session?.email]);

const quickActionAvatars = useMemo(() => {
if (quickActionReport) {
Expand Down
5 changes: 3 additions & 2 deletions src/pages/iou/request/MoneyRequestParticipantsSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID);
const policy = usePolicy(activePolicyID);
const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false});
const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email});
const {options, areOptionsInitialized} = useOptionsList({
shouldInitialize: didScreenTransitionEnd,
});
Expand Down Expand Up @@ -255,7 +256,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF

if (iouType === CONST.IOU.TYPE.INVOICE) {
// TODO: Use getInvoicePrimaryWorkspace when the invoices screen is ready - https://github.com/Expensify/App/issues/45175.
const policyID = option.item && ReportUtils.isInvoiceRoom(option.item) ? option.policyID : Policy.getPrimaryPolicy(activePolicyID)?.id;
const policyID = option.item && ReportUtils.isInvoiceRoom(option.item) ? option.policyID : Policy.getPrimaryPolicy(activePolicyID, currentUserLogin)?.id;
newParticipants.push({
policyID,
isSender: true,
Expand All @@ -268,7 +269,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
onFinish();
},
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps -- we don't want to trigger this callback when iouType changes
[onFinish, onParticipantsAdded],
[onFinish, onParticipantsAdded, currentUserLogin],
);

/**
Expand Down
7 changes: 4 additions & 3 deletions src/pages/iou/request/step/IOURequestStepSendFrom.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, {useMemo} from 'react';
import {withOnyx} from 'react-native-onyx';
import {useOnyx, withOnyx} from 'react-native-onyx';
import type {OnyxCollection} from 'react-native-onyx';
import * as Expensicons from '@components/Icon/Expensicons';
import SelectionList from '@components/SelectionList';
Expand Down Expand Up @@ -37,11 +37,12 @@ type IOURequestStepSendFromProps = IOURequestStepSendFromOnyxProps &
function IOURequestStepSendFrom({route, transaction, allPolicies}: IOURequestStepSendFromProps) {
const {translate} = useLocalize();
const {transactionID, backTo} = route.params;
const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email});

const selectedWorkspace = useMemo(() => transaction?.participants?.find((participant) => participant.isSender), [transaction]);

const workspaceOptions: WorkspaceListItem[] = useMemo(() => {
const availableWorkspaces = PolicyUtils.getActiveAdminWorkspaces(allPolicies);
const availableWorkspaces = PolicyUtils.getActiveAdminWorkspaces(allPolicies, currentUserLogin);
// TODO: Uncomment the following line when the invoices screen is ready - https://github.com/Expensify/App/issues/45175.
// .filter((policy) => PolicyUtils.canSendInvoiceFromWorkspace(policy.id));

Expand All @@ -62,7 +63,7 @@ function IOURequestStepSendFrom({route, transaction, allPolicies}: IOURequestSte
],
isSelected: selectedWorkspace?.policyID === policy.id,
}));
}, [allPolicies, selectedWorkspace]);
}, [allPolicies, currentUserLogin, selectedWorkspace]);

const navigateBack = () => {
Navigation.goBack(backTo);
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/AccessOrNotFoundWrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const ACCESS_VARIANTS = {
IOUUtils.isValidMoneyRequestType(iouType) &&
// Allow the user to submit the expense if we are submitting the expense in global menu or the report can create the expense
(isEmptyObject(report?.reportID) || ReportUtils.canCreateRequest(report, policy, iouType)) &&
(iouType !== CONST.IOU.TYPE.INVOICE || PolicyUtils.canSendInvoice(allPolicies)),
(iouType !== CONST.IOU.TYPE.INVOICE || PolicyUtils.canSendInvoice(allPolicies, login)),
} as const satisfies Record<
string,
(policy: OnyxTypes.Policy, login: string, report: OnyxTypes.Report, allPolicies: NonNullable<OnyxCollection<OnyxTypes.Policy>> | null, iouType?: IOUType) => boolean
Expand Down
11 changes: 6 additions & 5 deletions src/pages/workspace/WorkspacesListPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import WorkspacesListRow from './WorkspacesListRow';
type WorkspaceItem = Required<Pick<MenuItemProps, 'title' | 'disabled'>> &
Pick<MenuItemProps, 'brickRoadIndicator' | 'iconFill' | 'fallbackIcon'> &
Pick<OfflineWithFeedbackProps, 'errors' | 'pendingAction'> &
Pick<PolicyType, 'role' | 'type' | 'ownerAccountID'> & {
Pick<PolicyType, 'role' | 'type' | 'ownerAccountID' | 'employeeList'> & {
icon: AvatarSource;
action: () => void;
dismissError: () => void;
Expand Down Expand Up @@ -151,7 +151,7 @@ function WorkspacesListPage({policies, reimbursementAccount, reports, session}:
*/
const getMenuItem = useCallback(
({item, index}: GetMenuItem) => {
const isAdmin = item.role === CONST.POLICY.ROLE.ADMIN;
const isAdmin = PolicyUtils.isPolicyAdmin(item as unknown as PolicyType, session?.email);
const isOwner = item.ownerAccountID === session?.accountID;
// Menu options to navigate to the chat report of #admins and #announce room.
// For navigation, the chat report ids may be unavailable due to the missing chat reports in Onyx.
Expand Down Expand Up @@ -231,7 +231,7 @@ function WorkspacesListPage({policies, reimbursementAccount, reports, session}:
</OfflineWithFeedback>
);
},
[isLessThanMediumScreen, styles.mb3, styles.mh5, styles.ph5, styles.hoveredComponentBG, translate, styles.offlineFeedback.deleted, session?.accountID],
[isLessThanMediumScreen, styles.mb3, styles.mh5, styles.ph5, styles.hoveredComponentBG, translate, styles.offlineFeedback.deleted, session?.accountID, session?.email],
);

const listHeaderComponent = useCallback(() => {
Expand Down Expand Up @@ -312,7 +312,7 @@ function WorkspacesListPage({policies, reimbursementAccount, reports, session}:
}

return Object.values(policies)
.filter((policy): policy is PolicyType => PolicyUtils.shouldShowPolicy(policy, !!isOffline))
.filter((policy): policy is PolicyType => PolicyUtils.shouldShowPolicy(policy, !!isOffline, session?.email))
.map((policy): WorkspaceItem => {
if (policy?.isJoinRequestPending && policy?.policyDetailsForNonMembers) {
const policyInfo = Object.values(policy.policyDetailsForNonMembers)[0];
Expand Down Expand Up @@ -352,10 +352,11 @@ function WorkspacesListPage({policies, reimbursementAccount, reports, session}:
ownerAccountID: policy.ownerAccountID,
role: policy.role,
type: policy.type,
employeeList: policy.employeeList,
};
})
.sort((a, b) => localeCompare(a.title, b.title));
}, [reimbursementAccount?.errors, policies, isOffline, theme.textLight, policyRooms]);
}, [reimbursementAccount?.errors, policies, isOffline, theme.textLight, policyRooms, session?.email]);

const getHeaderButton = () => (
<Button
Expand Down
Loading