Skip to content
Open
Show file tree
Hide file tree
Changes from 10 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
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ type WorkspaceCompanyCardsTableHeaderButtonsProps = {
/** Whether to show the table controls */
showTableControls: boolean;

/** Whether the current member can edit company cards */
canWriteCompanyCards: boolean;

/** Card feed icon */
CardFeedIcon: React.ReactNode;
};

function WorkspaceCompanyCardsTableHeaderButtons({policyID, feedName, isLoading, showTableControls, CardFeedIcon}: WorkspaceCompanyCardsTableHeaderButtonsProps) {
function WorkspaceCompanyCardsTableHeaderButtons({policyID, feedName, isLoading, showTableControls, canWriteCompanyCards, CardFeedIcon}: WorkspaceCompanyCardsTableHeaderButtonsProps) {
const styles = useThemeStyles();

const {shouldUseNarrowLayout, isMediumScreenWidth} = useResponsiveLayout();
Expand Down Expand Up @@ -162,22 +165,24 @@ function WorkspaceCompanyCardsTableHeaderButtons({policyID, feedName, isLoading,
{!isLoading && (
<>
{showTableControls && <Table.FilterButtons style={shouldShowNarrowLayout && [styles.flex1]} />}
<ButtonWithDropdownMenu
success={false}
onPress={() => {}}
shouldUseOptionIcon
customText={translate('common.more')}
options={secondaryActions}
isSplitButton={false}
wrapperStyle={shouldShowNarrowLayout ? styles.flex1 : styles.flexGrow0}
sentryLabel={CONST.SENTRY_LABEL.WORKSPACE.COMPANY_CARDS.MORE_DROPDOWN}
/>
{canWriteCompanyCards && (
<ButtonWithDropdownMenu
success={false}
onPress={() => {}}
shouldUseOptionIcon
customText={translate('common.more')}
options={secondaryActions}
isSplitButton={false}
wrapperStyle={shouldShowNarrowLayout ? styles.flex1 : styles.flexGrow0}
sentryLabel={CONST.SENTRY_LABEL.WORKSPACE.COMPANY_CARDS.MORE_DROPDOWN}
/>
)}
</>
)}
</View>
</View>
</View>
{!isLoading && (isFeedConnectionBroken || hasFeedErrors) && (
{!isLoading && canWriteCompanyCards && (isFeedConnectionBroken || hasFeedErrors) && (
<View style={[styles.flexRow, styles.ph5, styles.alignItemsCenter]}>
<Icon
src={icons.DotIndicator}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
/** Whether to disable assign card button */
isAssigningCardDisabled?: boolean;

/** Whether the current member can edit company cards */
canWriteCompanyCards: boolean;

/** Whether to use narrow table row layout */
shouldUseNarrowTableLayout: boolean;

Expand All @@ -63,7 +66,16 @@
onAssignCard: (cardName: string, cardID: string) => void;
};

function WorkspaceCompanyCardTableRow({item, policyID, CardFeedIcon, shouldUseNarrowTableLayout, rowIndex, isAssigningCardDisabled, onAssignCard}: WorkspaceCompanyCardTableRowProps) {
function WorkspaceCompanyCardTableRow({
item,
policyID,
CardFeedIcon,
shouldUseNarrowTableLayout,
rowIndex,
isAssigningCardDisabled,
canWriteCompanyCards,
onAssignCard,
}: WorkspaceCompanyCardTableRowProps) {
const theme = useTheme();
const styles = useThemeStyles();
const {isOffline} = useNetwork();
Expand Down Expand Up @@ -94,18 +106,25 @@
? {width: variables.cardAvatarWidth, height: variables.cardAvatarHeight}
: {width: variables.cardAvatarWidthSmall, height: variables.cardAvatarHeightSmall};

const canOpenCardDetails = !!assignedCard?.accountID && !!assignedCard?.fundID && assignedCard?.cardID !== undefined;
const canAssignCard = !isAssigned && canWriteCompanyCards && !isAssigningCardDisabled;
const canPressRow = canOpenCardDetails || canAssignCard;

const handleRowPress = () => {
if (!assignedCard) {
if (!canAssignCard) {
return;
}
onAssignCard(cardName, encryptedCardNumber);

return;
}

if (!assignedCard?.accountID || !assignedCard?.fundID) {
if (!canOpenCardDetails || assignedCard.cardID === undefined) {
return;
}

const feedName = getCardFeedWithDomainID(assignedCard?.bank as CompanyCardFeed, assignedCard.fundID);

Check failure on line 127 in src/components/Tables/WorkspaceCompanyCardsTable/WorkspaceCompanyCardsTableRow.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Argument of type 'string | undefined' is not assignable to parameter of type 'string | number'.

return Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARD_DETAILS.getRoute(policyID, feedName as CompanyCardFeedWithDomainID, assignedCard.cardID.toString()));
};
Expand All @@ -115,7 +134,7 @@
interactive
rowIndex={rowIndex}
isLoading={isDeleting}
disabled={isCardDeleted || isAssigningCardDisabled}
disabled={isCardDeleted || !canPressRow}
skeletonReasonAttributes={reasonAttributes}
sentryLabel={CONST.SENTRY_LABEL.WORKSPACE.COMPANY_CARDS.TABLE_ITEM}
LoadingComponent={WorkspaceCompanyCardsTableSkeleton}
Expand Down Expand Up @@ -175,7 +194,7 @@
)}

<View style={[styles.flexRow, styles.alignItemsCenter, styles.justifyContentEnd, styles.gap3]}>
{!isAssigned && (
{!isAssigned && canWriteCompanyCards && (
<Button
small
success
Expand All @@ -185,13 +204,15 @@
/>
)}

<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
additionalStyles={[styles.alignSelfCenter, !hovered && styles.opacitySemiTransparent]}
width={variables.iconSizeNormal}
height={variables.iconSizeNormal}
/>
{canPressRow && (
<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
additionalStyles={[styles.alignSelfCenter, !hovered && styles.opacitySemiTransparent]}
width={variables.iconSizeNormal}
height={variables.iconSizeNormal}
/>
)}
</View>
</>
)}
Expand Down
7 changes: 7 additions & 0 deletions src/components/Tables/WorkspaceCompanyCardsTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ type WorkspaceCompanyCardsTableProps = {
/** Whether to disable assign card button */
isAssigningCardDisabled: boolean;

/** Whether the current member can edit company cards */
canWriteCompanyCards: boolean;

/** On assign card callback */
onAssignCard: (cardID: string, encryptedCardNumber: string) => void;

Expand All @@ -68,6 +71,7 @@ function WorkspaceCompanyCardsTable({
companyCards,
onAssignCard,
isAssigningCardDisabled,
canWriteCompanyCards,
onReloadPage,
onReloadFeed,
}: WorkspaceCompanyCardsTableProps) {
Expand Down Expand Up @@ -294,6 +298,7 @@ function WorkspaceCompanyCardsTable({
CardFeedIcon={cardFeedIcon}
onAssignCard={onAssignCard}
isAssigningCardDisabled={isAssigningCardDisabled}
canWriteCompanyCards={canWriteCompanyCards}
shouldUseNarrowTableLayout={shouldUseNarrowTableLayout}
/>
);
Expand Down Expand Up @@ -337,6 +342,7 @@ function WorkspaceCompanyCardsTable({
policyID={policyID}
feedName={feedName}
showTableControls={showTableControls}
canWriteCompanyCards={canWriteCompanyCards}
CardFeedIcon={cardFeedIcon}
/>
</View>
Expand Down Expand Up @@ -397,6 +403,7 @@ function WorkspaceCompanyCardsTable({
<WorkspaceCompanyCardPageEmptyState
policyID={policyID}
shouldShowGBDisclaimer={shouldShowGBDisclaimer}
canWriteCompanyCards={canWriteCompanyCards}
/>
</View>
)}
Expand Down
34 changes: 34 additions & 0 deletions src/hooks/usePolicyFeatureWriteAccess.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import {canMemberWrite} from '@libs/PolicyUtils';
import type {PolicyFeature} from '@libs/PolicyUtils';
import type {OnyxInputOrEntry, Policy} from '@src/types/onyx';
import useConfirmModal from './useConfirmModal';
import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails';
import useLocalize from './useLocalize';

function usePolicyFeatureWriteAccess(policy: OnyxInputOrEntry<Policy>, feature: PolicyFeature) {
const {translate} = useLocalize();
const {showConfirmModal} = useConfirmModal();
const {login: currentUserLogin = ''} = useCurrentUserPersonalDetails();
const canWrite = canMemberWrite(policy, currentUserLogin, feature);

const showReadOnlyModal = () => {
showConfirmModal({
title: translate('workspace.common.readOnlyActionTitle'),
prompt: translate('workspace.common.readOnlyActionPrompt'),
confirmText: translate('common.buttonConfirm'),
shouldShowCancelButton: false,
});
};

const getReadOnlyDisabledAction = (disabledAction?: () => void | Promise<void>) => {
if (!canWrite) {
return showReadOnlyModal;
}

return disabledAction;
};

return {canWrite, showReadOnlyModal, getReadOnlyDisabledAction};
}

export default usePolicyFeatureWriteAccess;
2 changes: 2 additions & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4360,6 +4360,8 @@ const translations = {
unavailable: 'Unavailable workspace',
memberNotFound: 'Member not found. To invite a new member to the workspace, please use the invite button above.',
notAuthorized: `You don't have access to this page. If you're trying to join this workspace, just ask the workspace owner to add you as a member. Something else? Reach out to ${CONST.EMAIL.CONCIERGE}.`,
readOnlyActionTitle: 'Not so fast...',
readOnlyActionPrompt: "Your workspace role can view these settings, but can't edit them.",
goToWorkspace: 'Go to workspace',
duplicateWorkspace: 'Duplicate workspace',
duplicateWorkspacePrefix: 'Duplicate',
Expand Down
2 changes: 2 additions & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4162,6 +4162,8 @@ ${amount} para ${merchant} - ${date}`,
unavailable: 'Espacio de trabajo no disponible',
memberNotFound: 'Miembro no encontrado. Para invitar a un nuevo miembro al espacio de trabajo, por favor, utiliza el botón invitar que está arriba.',
notAuthorized: `No tienes acceso a esta página. Si estás intentando unirte a este espacio de trabajo, pide al dueño del espacio de trabajo que te añada como miembro. ¿Necesitas algo más? Comunícate con ${CONST.EMAIL.CONCIERGE}`,
readOnlyActionTitle: 'No tan rápido...',
readOnlyActionPrompt: 'Tu rol en el espacio de trabajo puede ver esta configuración, pero no editarla.',
goToWorkspace: 'Ir al espacio de trabajo',
duplicateWorkspace: 'Duplicar espacio de trabajo',
duplicateWorkspacePrefix: 'Duplicar',
Expand Down
12 changes: 11 additions & 1 deletion src/libs/PolicyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,18 @@ const ROLE_PERMISSION_BUNDLES: Record<string, Partial<Record<PolicyFeature, Poli
},
};

const CONTROL_POLICY_ONLY_ROLES = [CONST.POLICY.ROLE.AUDITOR, CONST.POLICY.ROLE.CARD_ADMIN, CONST.POLICY.ROLE.PEOPLE_ADMIN, CONST.POLICY.ROLE.PAYMENTS_ADMIN];

function isControlPolicyOnlyRole(role: string | undefined): boolean {
return CONTROL_POLICY_ONLY_ROLES.some((controlPolicyOnlyRole) => controlPolicyOnlyRole === role);
}

function hasPolicyFeaturePermission(policy: OnyxInputOrEntry<Policy>, login: string, feature: PolicyFeature, requiredAccess: PolicyFeatureAccess): boolean {
const role = getPolicyRole(policy, login, false);
const role = getPolicyRole(policy, login, !login);
if (isControlPolicyOnlyRole(role) && (!policy || !isControlPolicy(policy))) {
return false;
}

const access = role ? ROLE_PERMISSION_BUNDLES[role]?.[feature] : undefined;

if (requiredAccess === CONST.POLICY.POLICY_FEATURE_ACCESS.READ) {
Expand Down
3 changes: 2 additions & 1 deletion src/pages/workspace/AccessOrNotFoundWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ function AccessOrNotFoundWrapper({
const {isOffline} = useNetwork();

const isReportArchived = useReportIsArchived(report?.reportID);
const isPageAccessible = accessVariants.reduce((acc, variant) => {
const accessVariantsToCheck = policyFeature ? accessVariants.filter((variant) => variant !== CONST.POLICY.ACCESS_VARIANTS.ADMIN) : accessVariants;
const isPageAccessible = accessVariantsToCheck.reduce((acc, variant) => {
Comment on lines +190 to +191
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.

wondering why do we need this change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When policyFeature is provided, admin-only access variants must be skipped, otherwise non-admin roles with read access would still not have access. What do you think?

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.

Yes, makes sense. Also AccessOrNotFoundWrapper component is used across the app and not only for workspace pages, just adding policy feature prop compulsarily to all workspace pages won't solve the issue for non-workspace pages.

const accessFunction = ACCESS_VARIANTS[variant];
if (variant === CONST.IOU.ACCESS_VARIANTS.CREATE) {
return acc && accessFunction(policy, login, report, allPolicies ?? null, betas, iouType, isReportArchived, isRestrictedToPreferredPolicy);
Expand Down
Loading
Loading