Render role-restricted card links as plain text when viewer lacks access #98612
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; | |
| import {setIssueNewCardStepAndData} from '@libs/actions/Card'; | ||
| import {getDefaultExpensifyCardLimitType} from '@libs/CardUtils'; | ||
| import {convertToBackendAmount, convertToFrontendAmountAsString} from '@libs/CurrencyUtils'; | ||
| import {canMemberRead, getApprovalWorkflow, isPolicyFeatureEnabled} from '@libs/PolicyUtils'; | ||
| import {getFieldRequiredErrors} from '@libs/ValidationUtils'; | ||
|
|
||
| import CONST from '@src/CONST'; | ||
|
|
@@ -31,6 +31,7 @@ import KeyboardUtils from '@src/utils/keyboard'; | |
|
|
||
| import type {OnyxEntry} from 'react-native-onyx'; | ||
|
|
||
| import {emailSelector} from '@selectors/Session'; | ||
| import React, {useCallback, useMemo, useRef, useState} from 'react'; | ||
| import {View} from 'react-native'; | ||
|
|
||
|
|
@@ -56,6 +57,11 @@ function LimitTypeStep({policy, stepNames, startStepIndex}: LimitTypeStepProps) | |
| const formRef = useRef<FormRef | null>(null); | ||
| const [issueNewCard] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_ISSUE_NEW_EXPENSIFY_CARD}${policyID}`); | ||
| const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`); | ||
| const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: emailSelector}); | ||
|
Contributor
There was a problem hiding this comment. @MelvinBot Same login source as
Contributor
Author
There was a problem hiding this comment. Good catch — in the loaded (steady) state the gate won't drift, but matching the source is the more airtight choice, and I'd lean toward it.
Where they can differ, and why it's low-risk either wayThe provider builds the object as
So Since the goal is "mirror the destination page's access check exactly," switching both sites ( |
||
|
|
||
| // Only link to the Workflows page when the current user can actually read it. Card admins without Workflows | ||
| // access would otherwise be dropped onto the Not Found page. When they lack access, render plain (non-linked) text. | ||
| const canReadWorkflows = canMemberRead(policy, currentUserLogin ?? '', CONST.POLICY.POLICY_FEATURE.WORKFLOWS); | ||
|
|
||
| const areApprovalsConfigured = getApprovalWorkflow(policy) !== CONST.POLICY.APPROVAL_MODE.OPTIONAL; | ||
| const defaultType = getDefaultExpensifyCardLimitType(policy); | ||
|
|
@@ -102,7 +108,7 @@ function LimitTypeStep({policy, stepNames, startStepIndex}: LimitTypeStepProps) | |
| setIssueNewCardStepAndData({step: CONST.EXPENSIFY_CARD.STEP.CARD_TYPE, policyID}); | ||
| }, [isEditing, policyID]); | ||
|
|
||
| const workspaceWorkflowsPageURL = canReadWorkflows ? `${environmentURL}/${ROUTES.WORKSPACE_WORKFLOWS.getRoute(policyID)}` : undefined; | ||
|
|
||
| const data = useMemo(() => { | ||
| const options = []; | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MelvinBot Same login source as
WorkspaceWorkflowsPage? That page feedscanMemberReadfromuseCurrentUserPersonalDetails().login, whereas this usessession.email— usually identical, but would matching the source keep the gate exactly in sync?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch — in the loaded (steady) state the gate won't drift, but matching the source is the more airtight choice, and I'd lean toward it.
canMemberReadultimately does an exact key lookup:policy.employeeList[login].role. For the current user,session.emailanduseCurrentUserPersonalDetails().loginresolve to the same primary-login string, so once Onyx is populated both feedcanMemberReadthe identical value and the two pages agree.Where they can differ, and why it's low-risk either way
The provider builds the object as
{...personalDetails[accountID], accountID, email: session.email}— noteemailis overridden tosession.email, butloginis not; it comes straight fromPERSONAL_DETAILS_LIST. So the only realistic divergence is a timing one:session.emailis available immediately, whereaspersonalDetails[accountID].loginis''until that Onyx key loads.WorkspaceWorkflowsPagepasseslogin=''→canReadWorkflowsis briefly false → it renders its Not Found gate. Our page, usingsession.email, would already show the link.So
session.emailis arguably more robust at first paint, butuseCurrentUserPersonalDetails().logingives exact parity with the gate the destination page enforces, which is the whole intent of this PR.Since the goal is "mirror the destination page's access check exactly," switching both sites (
LimitTypeStep.tsx:60andDynamicExpensifyCardLimitTypePage.tsx:68) touseCurrentUserPersonalDetails().loginremoves even the theoretical drift for a trivial cost. Want me to push that change?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MelvinBot Yes, apply it