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

Jump to bottom

Fix - An employee can access in a paid IOU the tag selection menu via a URL request #37176

Merged
merged 4 commits into from
Mar 5, 2024
Merged

Fix - An employee can access in a paid IOU the tag selection menu via a URL request #37176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
46cb964
display not found page on deeplink unauthorized access for tag page
FitseTLT Feb 24, 2024
ca95fb3
Merge branch 'main' into fix-deeplink-tag-page-bug
FitseTLT Feb 28, 2024
cb31a15
moved not found page under screen wrapper
FitseTLT Feb 28, 2024
1b084e5
Merge branch 'main' into fix-deeplink-tag-page-bug
FitseTLT Mar 3, 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
17 changes: 16 additions & 1 deletion src/pages/iou/request/step/IOURequestStepTag.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import compose from '@libs/compose';
import * as IOUUtils from '@libs/IOUUtils';
import Navigation from '@libs/Navigation/Navigation';
import * as PolicyUtils from '@libs/PolicyUtils';
import {canEditMoneyRequest} from '@libs/ReportUtils';
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: I think we generally prefer import * in our style, but I can't actually find a rule about that.

import * as TransactionUtils from '@libs/TransactionUtils';
import reportActionPropTypes from '@pages/home/report/reportActionPropTypes';
import reportPropTypes from '@pages/reportPropTypes';
import {policyPropTypes} from '@pages/workspace/withPolicy';
import * as IOU from '@userActions/IOU';
Expand Down Expand Up @@ -42,6 +44,9 @@ const propTypes = {

/** Collection of tags attached to a policy */
policyTags: tagPropTypes,

/** The actions from the parent report */
parentReportActions: PropTypes.shape(reportActionPropTypes),
};

const defaultProps = {
Expand All @@ -50,6 +55,7 @@ const defaultProps = {
policyTags: null,
policyCategories: null,
transaction: {},
parentReportActions: {},
};

function IOURequestStepTag({
Expand All @@ -61,6 +67,7 @@ function IOURequestStepTag({
params: {action, tagIndex: rawTagIndex, transactionID, backTo, iouType},
},
transaction,
parentReportActions,
}) {
const styles = useThemeStyles();
const {translate} = useLocalize();
Expand All @@ -71,6 +78,10 @@ function IOURequestStepTag({
const tag = TransactionUtils.getTag(transaction, tagIndex);
const isEditing = action === CONST.IOU.ACTION.EDIT;
const isSplitBill = iouType === CONST.IOU.TYPE.SPLIT;
const parentReportAction = parentReportActions[report.parentReportActionID];

// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = isEditing && !canEditMoneyRequest(parentReportAction);
Copy link
Contributor

Choose a reason for hiding this comment

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

While updating not found logic here, the case of P2P request could also have been considered as it doesn't make sense to show tags page in P2P.
Issue: #37207

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't know how this pr is considered as the offending PR to that issue as it wasn't merged when that issue was created and this pr is still younger than yours 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no specific offending PR for #37207.
I also didn't say that this one is the offending PR but just missing case.


const navigateBack = () => {
Navigation.goBack(backTo);
Expand Down Expand Up @@ -103,11 +114,11 @@ function IOURequestStepTag({
onBackButtonPress={navigateBack}
shouldShowWrapper
testID={IOURequestStepTag.displayName}
shouldShowNotFoundPage={shouldShowNotFoundPage}
>
{({insets}) => (
<>
<Text style={[styles.ph5, styles.pv3]}>{translate('iou.tagSelection', {tagName: policyTagListName})}</Text>

<TagPicker
policyID={report.policyID}
tag={policyTagListName}
Expand Down Expand Up @@ -139,5 +150,9 @@ export default compose(
policyTags: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${report ? report.policyID : '0'}`,
},
parentReportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '0'}`,
canEvict: false,
},
}),
)(IOURequestStepTag);
40 changes: 19 additions & 21 deletions src/pages/iou/request/step/StepScreenWrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,8 @@ const defaultProps = {
function StepScreenWrapper({testID, headerTitle, onBackButtonPress, onEntryTransitionEnd, children, shouldShowWrapper, shouldShowNotFoundPage, includeSafeAreaPaddingBottom}) {
const styles = useThemeStyles();

if (shouldShowNotFoundPage) {
return <FullPageNotFoundView shouldShow />;
}

if (!shouldShowWrapper) {
return children;
return <FullPageNotFoundView shouldShow={shouldShowNotFoundPage}>{children}</FullPageNotFoundView>;
}

return (
Expand All @@ -59,22 +55,24 @@ function StepScreenWrapper({testID, headerTitle, onBackButtonPress, onEntryTrans
shouldEnableMaxHeight={DeviceCapabilities.canUseTouchScreen()}
>
{({insets, safeAreaPaddingBottomStyle, didScreenTransitionEnd}) => (
<View style={[styles.flex1]}>
<HeaderWithBackButton
title={headerTitle}
onBackButtonPress={onBackButtonPress}
/>
{
// If props.children is a function, call it to provide the insets to the children.
_.isFunction(children)
? children({
insets,
safeAreaPaddingBottomStyle,
didScreenTransitionEnd,
})
: children
}
</View>
<FullPageNotFoundView shouldShow={shouldShowNotFoundPage}>
<View style={[styles.flex1]}>
<HeaderWithBackButton
title={headerTitle}
onBackButtonPress={onBackButtonPress}
/>
{
// If props.children is a function, call it to provide the insets to the children.
_.isFunction(children)
? children({
insets,
safeAreaPaddingBottomStyle,
didScreenTransitionEnd,
})
: children
}
</View>
</FullPageNotFoundView>
)}
</ScreenWrapper>
);
Expand Down
Loading