From 6261e47e46b58d8a1ff7a9771dee83e6a68a4e0a Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Mon, 9 Oct 2023 18:10:15 +0200 Subject: [PATCH 01/16] improve canEditMoneyRequest --- src/libs/ReportUtils.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index a8e80101c07f..ee1eaeeea3fe 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1377,28 +1377,36 @@ function getTransactionDetails(transaction) { * Can only edit if: * * - in case of IOU report - * - the current user is the requestor + * - the current user is the requestor and is not settled yet * - in case of expense report - * - the current user is the requestor + * - the current user is the requestor and is not settled yet * - or the user is an admin on the policy the expense report is tied to * * @param {Object} reportAction * @returns {Boolean} */ function canEditMoneyRequest(reportAction) { - // If the report action i snot IOU type, return true early + // If the report action is not IOU type, return true early if (reportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.IOU) { return true; } + const moneyRequestReportID = lodashGet(reportAction, 'originalMessage.IOUReportID', 0); + if (!moneyRequestReportID) { return false; } + const moneyRequestReport = getReport(moneyRequestReportID); const isReportSettled = isSettled(moneyRequestReport.reportID); const isAdmin = isExpenseReport(moneyRequestReport) && lodashGet(getPolicy(moneyRequestReport.policyID), 'role', '') === CONST.POLICY.ROLE.ADMIN; const isRequestor = currentUserAccountID === reportAction.actorAccountID; - return !isReportSettled && (isAdmin || isRequestor); + + if (isAdmin) { + return true; + } + + return !isReportSettled && isRequestor; } /** From 10dce757ec23ae03bf00242e086b5290ee36cfb3 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Mon, 9 Oct 2023 18:10:28 +0200 Subject: [PATCH 02/16] improve condition to edit fields --- .../ReportActionItem/MoneyRequestView.js | 8 +++--- src/pages/EditRequestPage.js | 13 ++++++---- src/pages/iou/IOUCurrencySelection.js | 25 ++++++++++++++++++- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.js b/src/components/ReportActionItem/MoneyRequestView.js index 079bc64d96bf..3c034ef6b593 100644 --- a/src/components/ReportActionItem/MoneyRequestView.js +++ b/src/components/ReportActionItem/MoneyRequestView.js @@ -158,8 +158,8 @@ function MoneyRequestView({report, betas, parentReport, policyCategories, should titleIcon={Expensicons.Checkmark} description={description} titleStyle={styles.newKansasLarge} - interactive={canEdit} - shouldShowRightIcon={canEdit} + interactive={canEdit && !isSettled} + shouldShowRightIcon={canEdit && !isSettled} onPress={() => Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.AMOUNT))} brickRoadIndicator={hasErrors && transactionAmount === 0 ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''} error={hasErrors && transactionAmount === 0 ? translate('common.error.enterAmount') : ''} @@ -182,8 +182,8 @@ function MoneyRequestView({report, betas, parentReport, policyCategories, should Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.DATE))} brickRoadIndicator={hasErrors && transactionDate === '' ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''} diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 28e70dc1a47e..b5497daf83b2 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -110,9 +110,7 @@ function EditRequestPage({betas, report, route, parentReport, policy, session, p const isDeleted = ReportActionsUtils.isDeletedAction(parentReportAction); const isSettled = ReportUtils.isSettled(parentReport.reportID); - const isAdmin = Policy.isAdminOfFreePolicy([policy]) && ReportUtils.isExpenseReport(parentReport); - const isRequestor = ReportUtils.isMoneyRequestReport(parentReport) && lodashGet(session, 'accountID', null) === parentReportAction.actorAccountID; - const canEdit = !isSettled && !isDeleted && (isAdmin || isRequestor); + const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction); // For now, it always defaults to the first tag of the policy const policyTag = PolicyUtils.getTag(policyTags); @@ -130,13 +128,18 @@ function EditRequestPage({betas, report, route, parentReport, policy, session, p // Dismiss the modal when the request is paid or deleted useEffect(() => { - if (canEdit) { + const isAmountToEdit = fieldToEdit === CONST.EDIT_REQUEST_FIELD.AMOUNT; + const isCreatedDateToEdit = fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE; + const isNonEditableFieldWhenSettled = isAmountToEdit || isCreatedDateToEdit; + + if (canEdit && !isDeleted && (!isNonEditableFieldWhenSettled || !isSettled)) { return; } + Navigation.isNavigationReady().then(() => { Navigation.dismissModal(); }); - }, [canEdit]); + }, [canEdit, isDeleted, isSettled, fieldToEdit]); // Update the transaction object and close the modal function editMoneyRequest(transactionChanges) { diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js index cd14dcd25f11..6ed99030349c 100644 --- a/src/pages/iou/IOUCurrencySelection.js +++ b/src/pages/iou/IOUCurrencySelection.js @@ -1,4 +1,4 @@ -import React, {useState, useMemo, useCallback, useRef} from 'react'; +import React, {useState, useMemo, useCallback, useRef, useEffect} from 'react'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; @@ -14,6 +14,8 @@ import compose from '../../libs/compose'; import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize'; import {withNetwork} from '../../components/OnyxProvider'; import * as CurrencyUtils from '../../libs/CurrencyUtils'; +import * as ReportActionsUtils from '../../libs/ReportActionsUtils'; +import * as ReportUtils from '../../libs/ReportUtils'; import ROUTES from '../../ROUTES'; import themeColors from '../../styles/themes/default'; import * as Expensicons from '../../components/Icon/Expensicons'; @@ -74,6 +76,27 @@ function IOUCurrencySelection(props) { const selectedCurrencyCode = (lodashGet(props.route, 'params.currency', props.iou.currency) || CONST.CURRENCY.USD).toUpperCase(); const iouType = lodashGet(props.route, 'params.iouType', CONST.IOU.MONEY_REQUEST_TYPE.REQUEST); const reportID = lodashGet(props.route, 'params.reportID', ''); + const threadReportID = lodashGet(props.route, 'params.threadReportID', ''); + // Dismiss the modal when the request is paid or deleted + useEffect(() => { + if (!threadReportID) { + return; + } + + const report = ReportUtils.getReport(threadReportID); + const parentReportAction = ReportActionsUtils.getReportAction(report.parentReportID, report.parentReportActionID); + const isDeleted = ReportActionsUtils.isDeletedAction(parentReportAction); + const isSettled = ReportUtils.isSettled(report.parentReportID); + const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction); + + if (canEdit && !isDeleted && !isSettled) { + return; + } + + Navigation.isNavigationReady().then(() => { + Navigation.dismissModal(); + }); + }, [threadReportID]); const confirmCurrencySelection = useCallback( (option) => { From c0b085484bc8c7e96b9f0069d2375c596694a85c Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Tue, 10 Oct 2023 11:13:34 +0200 Subject: [PATCH 03/16] fix props --- src/pages/EditRequestPage.js | 37 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index b5497daf83b2..8ccb4bb64827 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -11,12 +11,10 @@ import * as ReportActionsUtils from '../libs/ReportActionsUtils'; import * as ReportUtils from '../libs/ReportUtils'; import * as PolicyUtils from '../libs/PolicyUtils'; import * as TransactionUtils from '../libs/TransactionUtils'; -import * as Policy from '../libs/actions/Policy'; import * as IOU from '../libs/actions/IOU'; import * as CurrencyUtils from '../libs/CurrencyUtils'; import * as OptionsListUtils from '../libs/OptionsListUtils'; import Permissions from '../libs/Permissions'; -import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsPropTypes} from '../components/withCurrentUserPersonalDetails'; import tagPropTypes from '../components/tagPropTypes'; import FullPageNotFoundView from '../components/BlockingViews/FullPageNotFoundView'; import EditRequestDescriptionPage from './EditRequestDescriptionPage'; @@ -30,6 +28,8 @@ import EditRequestCategoryPage from './EditRequestCategoryPage'; import EditRequestTagPage from './EditRequestTagPage'; import categoryPropTypes from '../components/categoryPropTypes'; import ScreenWrapper from '../components/ScreenWrapper'; +import reportActionPropTypes from './home/report/reportActionPropTypes'; +import transactionPropTypes from '../components/transactionPropTypes'; const propTypes = { /** Route from navigation */ @@ -54,43 +54,30 @@ const propTypes = { /** The parent report object for the thread report */ parentReport: reportPropTypes, - /** The policy object for the current route */ - policy: PropTypes.shape({ - /** The name of the policy */ - name: PropTypes.string, - - /** The URL for the policy avatar */ - avatar: PropTypes.string, - }), - - /** Session info for the currently logged in user. */ - session: PropTypes.shape({ - /** Currently logged in user email */ - email: PropTypes.string, - }), - /** Collection of categories attached to a policy */ policyCategories: PropTypes.objectOf(categoryPropTypes), /** Collection of tags attached to a policy */ policyTags: tagPropTypes, - ...withCurrentUserPersonalDetailsPropTypes, + /** The actions from the parent report */ + parentReportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)), + + /** Transaction that stores the request data */ + transaction: transactionPropTypes, }; const defaultProps = { betas: [], report: {}, parentReport: {}, - policy: null, - session: { - email: null, - }, policyCategories: {}, policyTags: {}, + parentReportActions: {}, + transaction: {}, }; -function EditRequestPage({betas, report, route, parentReport, policy, session, policyCategories, policyTags, parentReportActions, transaction}) { +function EditRequestPage({betas, report, route, parentReport, policyCategories, policyTags, parentReportActions, transaction}) { const parentReportAction = parentReportActions[report.parentReportActionID]; const { amount: transactionAmount, @@ -292,7 +279,6 @@ EditRequestPage.displayName = 'EditRequestPage'; EditRequestPage.propTypes = propTypes; EditRequestPage.defaultProps = defaultProps; export default compose( - withCurrentUserPersonalDetails, withOnyx({ betas: { key: ONYXKEYS.BETAS, @@ -303,9 +289,6 @@ export default compose( }), // eslint-disable-next-line rulesdir/no-multiple-onyx-in-file withOnyx({ - policy: { - key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY}${report ? report.policyID : '0'}`, - }, policyCategories: { key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${report ? report.policyID : '0'}`, }, From f0a5d470dc5a76910165addcec87d889670e3db1 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Tue, 10 Oct 2023 16:33:15 +0200 Subject: [PATCH 04/16] add a line --- src/pages/iou/IOUCurrencySelection.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js index 6ed99030349c..75e1de502722 100644 --- a/src/pages/iou/IOUCurrencySelection.js +++ b/src/pages/iou/IOUCurrencySelection.js @@ -77,6 +77,7 @@ function IOUCurrencySelection(props) { const iouType = lodashGet(props.route, 'params.iouType', CONST.IOU.MONEY_REQUEST_TYPE.REQUEST); const reportID = lodashGet(props.route, 'params.reportID', ''); const threadReportID = lodashGet(props.route, 'params.threadReportID', ''); + // Dismiss the modal when the request is paid or deleted useEffect(() => { if (!threadReportID) { From 14e8cb550bdcaa807ea6d6e3684c24f5dccb13ab Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 11 Oct 2023 12:32:53 +0200 Subject: [PATCH 05/16] rename vars --- src/pages/EditRequestPage.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 8ccb4bb64827..066f94b3268b 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -115,9 +115,9 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, // Dismiss the modal when the request is paid or deleted useEffect(() => { - const isAmountToEdit = fieldToEdit === CONST.EDIT_REQUEST_FIELD.AMOUNT; - const isCreatedDateToEdit = fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE; - const isNonEditableFieldWhenSettled = isAmountToEdit || isCreatedDateToEdit; + const isEditingAmount = fieldToEdit === CONST.EDIT_REQUEST_FIELD.AMOUNT; + const isEditingCreatedDate = fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE; + const isNonEditableFieldWhenSettled = isEditingAmount || isEditingCreatedDate; if (canEdit && !isDeleted && (!isNonEditableFieldWhenSettled || !isSettled)) { return; From 3574dfd45c63abe3ea87d3d3cd63970e6512f74d Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 11 Oct 2023 12:42:48 +0200 Subject: [PATCH 06/16] use is delete into canEditMoneyRequest --- src/libs/ReportUtils.js | 6 ++++++ src/pages/EditRequestPage.js | 7 ++----- src/pages/iou/IOUCurrencySelection.js | 3 +-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 6834a8c7953f..50b69a5765bf 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1386,6 +1386,12 @@ function getTransactionDetails(transaction) { * @returns {Boolean} */ function canEditMoneyRequest(reportAction) { + const isDeleted = ReportActionsUtils.isDeletedAction(reportAction); + + if (isDeleted) { + return false; + } + // If the report action is not IOU type, return true early if (reportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.IOU) { return true; diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 066f94b3268b..ff5a747de338 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -7,7 +7,6 @@ import CONST from '../CONST'; import ONYXKEYS from '../ONYXKEYS'; import compose from '../libs/compose'; import Navigation from '../libs/Navigation/Navigation'; -import * as ReportActionsUtils from '../libs/ReportActionsUtils'; import * as ReportUtils from '../libs/ReportUtils'; import * as PolicyUtils from '../libs/PolicyUtils'; import * as TransactionUtils from '../libs/TransactionUtils'; @@ -94,9 +93,7 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, const transactionCreated = TransactionUtils.getCreated(transaction); const fieldToEdit = lodashGet(route, ['params', 'field'], ''); - const isDeleted = ReportActionsUtils.isDeletedAction(parentReportAction); const isSettled = ReportUtils.isSettled(parentReport.reportID); - const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction); // For now, it always defaults to the first tag of the policy @@ -119,14 +116,14 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, const isEditingCreatedDate = fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE; const isNonEditableFieldWhenSettled = isEditingAmount || isEditingCreatedDate; - if (canEdit && !isDeleted && (!isNonEditableFieldWhenSettled || !isSettled)) { + if (canEdit && (!isNonEditableFieldWhenSettled || !isSettled)) { return; } Navigation.isNavigationReady().then(() => { Navigation.dismissModal(); }); - }, [canEdit, isDeleted, isSettled, fieldToEdit]); + }, [canEdit, isSettled, fieldToEdit]); // Update the transaction object and close the modal function editMoneyRequest(transactionChanges) { diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js index 75e1de502722..1fea61172c80 100644 --- a/src/pages/iou/IOUCurrencySelection.js +++ b/src/pages/iou/IOUCurrencySelection.js @@ -86,11 +86,10 @@ function IOUCurrencySelection(props) { const report = ReportUtils.getReport(threadReportID); const parentReportAction = ReportActionsUtils.getReportAction(report.parentReportID, report.parentReportActionID); - const isDeleted = ReportActionsUtils.isDeletedAction(parentReportAction); const isSettled = ReportUtils.isSettled(report.parentReportID); const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction); - if (canEdit && !isDeleted && !isSettled) { + if (canEdit && !isSettled) { return; } From a544459bb41a219edd5c8e3d015e26ee7b9decb1 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 11 Oct 2023 13:07:17 +0200 Subject: [PATCH 07/16] clarify comments --- src/components/ReportActionItem/MoneyRequestView.js | 2 ++ src/pages/EditRequestPage.js | 9 ++++++++- src/pages/iou/IOUCurrencySelection.js | 10 +++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.js b/src/components/ReportActionItem/MoneyRequestView.js index 3c034ef6b593..bc3f5b4e6467 100644 --- a/src/components/ReportActionItem/MoneyRequestView.js +++ b/src/components/ReportActionItem/MoneyRequestView.js @@ -95,8 +95,10 @@ function MoneyRequestView({report, betas, parentReport, policyCategories, should transactionMerchant === '' || transactionMerchant === CONST.TRANSACTION.UNKNOWN_MERCHANT || transactionMerchant === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT; const formattedTransactionAmount = transactionAmount && transactionCurrency && CurrencyUtils.convertToDisplayString(transactionAmount, transactionCurrency); + // Flags for allowing or disallowing editing a money request const isSettled = ReportUtils.isSettled(moneyRequestReport.reportID); const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction); + // A flag for verifying that the current report is a sub-report of a workspace chat const isPolicyExpenseChat = useMemo(() => ReportUtils.isPolicyExpenseChat(ReportUtils.getRootParentReport(report)), [report]); diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index ff5a747de338..195d3ca948bd 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -110,16 +110,23 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, // A flag for showing the tags page const shouldShowTags = isPolicyExpenseChat && Permissions.canUseTags(betas) && (transactionTag || OptionsListUtils.hasEnabledOptions(lodashValues(policyTagList))); - // Dismiss the modal when the request is paid or deleted + // Decides whether to allow or disallow editing a money request useEffect(() => { const isEditingAmount = fieldToEdit === CONST.EDIT_REQUEST_FIELD.AMOUNT; const isEditingCreatedDate = fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE; const isNonEditableFieldWhenSettled = isEditingAmount || isEditingCreatedDate; + /** + * Do not dismiss the modal, when a current user can edit a money request. + * To satisfy the condition below: + * 1. "canEdit" must be "true". It checks common rules like if the user is a requestor or admin, etc. + * 2. When the current edit field is the "amount" or "date" (created date), the money request shouldn't be settled yet. + * */ if (canEdit && (!isNonEditableFieldWhenSettled || !isSettled)) { return; } + // Dismiss the modal when a current user cannot edit a money request. Navigation.isNavigationReady().then(() => { Navigation.dismissModal(); }); diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js index 1fea61172c80..3b54752dd64e 100644 --- a/src/pages/iou/IOUCurrencySelection.js +++ b/src/pages/iou/IOUCurrencySelection.js @@ -78,8 +78,9 @@ function IOUCurrencySelection(props) { const reportID = lodashGet(props.route, 'params.reportID', ''); const threadReportID = lodashGet(props.route, 'params.threadReportID', ''); - // Dismiss the modal when the request is paid or deleted + // Decides whether to allow or disallow editing a money request useEffect(() => { + // Do not dismiss the modal, when it is not the edit flow. if (!threadReportID) { return; } @@ -89,10 +90,17 @@ function IOUCurrencySelection(props) { const isSettled = ReportUtils.isSettled(report.parentReportID); const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction); + /** + * Do not dismiss the modal, when a current user can edit a money request. + * To satisfy the condition below: + * 1. "canEdit" must be "true". It checks common rules like if the user is a requestor or admin, etc. + * 2. When the current edit field is the "currency", the money request shouldn't be settled yet. + * */ if (canEdit && !isSettled) { return; } + // Dismiss the modal when a current user cannot edit a money request. Navigation.isNavigationReady().then(() => { Navigation.dismissModal(); }); From 73adbc580a603f7b53708142aaa5c40e4b5fd860 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Thu, 12 Oct 2023 14:04:33 +0200 Subject: [PATCH 08/16] simplify condition --- src/pages/EditRequestPage.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 1e15e56e86b5..63aa63d9d486 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -1,4 +1,5 @@ import React, {useEffect, useMemo} from 'react'; +import _ from 'underscore'; import PropTypes from 'prop-types'; import lodashGet from 'lodash/get'; import lodashValues from 'lodash/values'; @@ -114,9 +115,7 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, // Decides whether to allow or disallow editing a money request useEffect(() => { - const isEditingAmount = fieldToEdit === CONST.EDIT_REQUEST_FIELD.AMOUNT; - const isEditingCreatedDate = fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE; - const isNonEditableFieldWhenSettled = isEditingAmount || isEditingCreatedDate; + const isNonEditableFieldWhenSettled = _.includes([CONST.EDIT_REQUEST_FIELD.AMOUNT, CONST.EDIT_REQUEST_FIELD.DATE], fieldToEdit); /** * Do not dismiss the modal, when a current user can edit a money request. From 1962613b3630330845f9e7aa3d05242f32dcb63f Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Thu, 12 Oct 2023 16:12:03 +0200 Subject: [PATCH 09/16] add a default value --- src/pages/EditRequestPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 63aa63d9d486..40a81c544682 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -80,7 +80,7 @@ const defaultProps = { function EditRequestPage({betas, report, route, parentReport, policyCategories, policyTags, parentReportActions, transaction}) { const parentReportActionID = lodashGet(report, 'parentReportActionID', '0'); - const parentReportAction = lodashGet(parentReportActions, parentReportActionID); + const parentReportAction = lodashGet(parentReportActions, parentReportActionID, {}); const { amount: transactionAmount, currency: transactionCurrency, From 6634bf6cbdd3a3e44bf56d562607b7f393a1887f Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Thu, 12 Oct 2023 16:22:50 +0200 Subject: [PATCH 10/16] restricted distance and receipt --- src/components/ReportActionItem/MoneyRequestView.js | 4 ++-- src/pages/EditRequestPage.js | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.js b/src/components/ReportActionItem/MoneyRequestView.js index bc3f5b4e6467..bd0346dc88aa 100644 --- a/src/components/ReportActionItem/MoneyRequestView.js +++ b/src/components/ReportActionItem/MoneyRequestView.js @@ -197,8 +197,8 @@ function MoneyRequestView({report, betas, parentReport, policyCategories, should Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.DISTANCE))} /> diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 40a81c544682..84b632c5773f 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -115,7 +115,10 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, // Decides whether to allow or disallow editing a money request useEffect(() => { - const isNonEditableFieldWhenSettled = _.includes([CONST.EDIT_REQUEST_FIELD.AMOUNT, CONST.EDIT_REQUEST_FIELD.DATE], fieldToEdit); + const isNonEditableFieldWhenSettled = _.includes( + [CONST.EDIT_REQUEST_FIELD.AMOUNT, CONST.EDIT_REQUEST_FIELD.DATE, CONST.EDIT_REQUEST_FIELD.RECEIPT, CONST.EDIT_REQUEST_FIELD.DISTANCE], + fieldToEdit, + ); /** * Do not dismiss the modal, when a current user can edit a money request. From e83af7c79bf3a2c40ca03ea13af9c35dc129ce8e Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Mon, 16 Oct 2023 18:15:28 +0200 Subject: [PATCH 11/16] clarify comment --- src/pages/iou/IOUCurrencySelection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js index 3b54752dd64e..d35c7bf5b45d 100644 --- a/src/pages/iou/IOUCurrencySelection.js +++ b/src/pages/iou/IOUCurrencySelection.js @@ -94,7 +94,7 @@ function IOUCurrencySelection(props) { * Do not dismiss the modal, when a current user can edit a money request. * To satisfy the condition below: * 1. "canEdit" must be "true". It checks common rules like if the user is a requestor or admin, etc. - * 2. When the current edit field is the "currency", the money request shouldn't be settled yet. + * 2. Settled requests cannot have their amount/currency edited. * */ if (canEdit && !isSettled) { return; From 32e32c416d9e395fac592abe5f261fd0d84fe45a Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Tue, 17 Oct 2023 16:15:20 +0200 Subject: [PATCH 12/16] unify logic to check edit permission --- src/libs/ReportUtils.js | 38 +++++++++++++++++++++++++++ src/pages/EditRequestPage.js | 20 +++----------- src/pages/iou/IOUCurrencySelection.js | 14 +++------- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index c903054bde4e..7f3d6f322874 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1473,6 +1473,43 @@ function canEditMoneyRequest(reportAction) { return !isReportSettled && isRequestor; } +/** + * Checks is the current user can edit the current field of a money request + * + * @param {Object} reportAction + * @param {String} reportID + * @param {String} fieldToEdit + * @returns {Boolean} + */ +function canEditFieldOfMoneyRequest(reportAction, reportID, fieldToEdit) { + // A list of fields that cannot be edited, once a money request has been settled + const nonEditableFieldsWhenSettled = [ + CONST.EDIT_REQUEST_FIELD.AMOUNT, + CONST.EDIT_REQUEST_FIELD.CURRENCY, + CONST.EDIT_REQUEST_FIELD.DATE, + CONST.EDIT_REQUEST_FIELD.RECEIPT, + CONST.EDIT_REQUEST_FIELD.DISTANCE, + ]; + // Checks if the current field is a restricted one + const isNonEditableFieldWhenSettled = _.includes(nonEditableFieldsWhenSettled, fieldToEdit); + // Checks if the current report is settled one + const isSettledMoneyRequest = isSettled(reportID); + // Checks common rules like if the user is a requestor or admin, etc + const canEdit = canEditMoneyRequest(reportAction); + + /** + * To satisfy the condition below: + * 1. "canEdit" must be "true". It checks if the current user can, in general, edit a money request. + * 2. When the current edit field is one restricted, the money request shouldn't be settled yet. + * */ + if (canEdit && (!isNonEditableFieldWhenSettled || !isSettledMoneyRequest)) { + return true; + } + + // A current user cannot edit the current field of the money request + return false; +} + /** * Can only edit if: * @@ -4070,6 +4107,7 @@ export { getTaskAssigneeChatOnyxData, getParticipantsIDs, canEditMoneyRequest, + canEditFieldOfMoneyRequest, buildTransactionThread, areAllRequestsBeingSmartScanned, getReportPreviewDisplayTransactions, diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 84b632c5773f..92ba2f8bd31f 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -1,5 +1,4 @@ import React, {useEffect, useMemo} from 'react'; -import _ from 'underscore'; import PropTypes from 'prop-types'; import lodashGet from 'lodash/get'; import lodashValues from 'lodash/values'; @@ -96,9 +95,6 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, const transactionCreated = TransactionUtils.getCreated(transaction); const fieldToEdit = lodashGet(route, ['params', 'field'], ''); - const isSettled = ReportUtils.isSettled(parentReport.reportID); - const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction); - // For now, it always defaults to the first tag of the policy const policyTag = PolicyUtils.getTag(policyTags); const policyTagList = lodashGet(policyTag, 'tags', {}); @@ -115,18 +111,10 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, // Decides whether to allow or disallow editing a money request useEffect(() => { - const isNonEditableFieldWhenSettled = _.includes( - [CONST.EDIT_REQUEST_FIELD.AMOUNT, CONST.EDIT_REQUEST_FIELD.DATE, CONST.EDIT_REQUEST_FIELD.RECEIPT, CONST.EDIT_REQUEST_FIELD.DISTANCE], - fieldToEdit, - ); + const canEdit = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, parentReport.reportID, fieldToEdit); - /** - * Do not dismiss the modal, when a current user can edit a money request. - * To satisfy the condition below: - * 1. "canEdit" must be "true". It checks common rules like if the user is a requestor or admin, etc. - * 2. When the current edit field is the "amount" or "date" (created date), the money request shouldn't be settled yet. - * */ - if (canEdit && (!isNonEditableFieldWhenSettled || !isSettled)) { + // Do not dismiss the modal, when a current user can edit a money request. + if (canEdit) { return; } @@ -134,7 +122,7 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, Navigation.isNavigationReady().then(() => { Navigation.dismissModal(); }); - }, [canEdit, isSettled, fieldToEdit]); + }, [parentReportAction, parentReport.reportID, fieldToEdit]); // Update the transaction object and close the modal function editMoneyRequest(transactionChanges) { diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js index d35c7bf5b45d..ebefb4fe5e39 100644 --- a/src/pages/iou/IOUCurrencySelection.js +++ b/src/pages/iou/IOUCurrencySelection.js @@ -87,16 +87,10 @@ function IOUCurrencySelection(props) { const report = ReportUtils.getReport(threadReportID); const parentReportAction = ReportActionsUtils.getReportAction(report.parentReportID, report.parentReportActionID); - const isSettled = ReportUtils.isSettled(report.parentReportID); - const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction); - - /** - * Do not dismiss the modal, when a current user can edit a money request. - * To satisfy the condition below: - * 1. "canEdit" must be "true". It checks common rules like if the user is a requestor or admin, etc. - * 2. Settled requests cannot have their amount/currency edited. - * */ - if (canEdit && !isSettled) { + const canEdit = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, report.parentReportID, CONST.EDIT_REQUEST_FIELD.CURRENCY); + + // Do not dismiss the modal, when a current user can edit a money request. + if (canEdit) { return; } From c2bfd64090ecc0bad696ac044450b8e916f4af3d Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Tue, 17 Oct 2023 16:47:26 +0200 Subject: [PATCH 13/16] remove a var --- src/pages/EditRequestPage.js | 4 +--- src/pages/iou/IOUCurrencySelection.js | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 92ba2f8bd31f..63ae4e37efe5 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -111,10 +111,8 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, // Decides whether to allow or disallow editing a money request useEffect(() => { - const canEdit = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, parentReport.reportID, fieldToEdit); - // Do not dismiss the modal, when a current user can edit a money request. - if (canEdit) { + if (ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, parentReport.reportID, fieldToEdit)) { return; } diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js index ebefb4fe5e39..569eb90e08e9 100644 --- a/src/pages/iou/IOUCurrencySelection.js +++ b/src/pages/iou/IOUCurrencySelection.js @@ -87,10 +87,9 @@ function IOUCurrencySelection(props) { const report = ReportUtils.getReport(threadReportID); const parentReportAction = ReportActionsUtils.getReportAction(report.parentReportID, report.parentReportActionID); - const canEdit = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, report.parentReportID, CONST.EDIT_REQUEST_FIELD.CURRENCY); // Do not dismiss the modal, when a current user can edit a money request. - if (canEdit) { + if (ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, report.parentReportID, CONST.EDIT_REQUEST_FIELD.CURRENCY)) { return; } From 6a8be94670b7836b82563682da105008cb4afe07 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 18 Oct 2023 12:58:49 +0200 Subject: [PATCH 14/16] clarify comments --- src/libs/ReportUtils.js | 20 +++++++++----------- src/pages/EditRequestPage.js | 2 +- src/pages/iou/IOUCurrencySelection.js | 2 +- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index c9c18fe44841..2412ea86f9b7 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1514,7 +1514,7 @@ function canEditMoneyRequest(reportAction) { } /** - * Checks is the current user can edit the current field of a money request + * Checks if the current user can edit the provided property of a money request * * @param {Object} reportAction * @param {String} reportID @@ -1522,7 +1522,7 @@ function canEditMoneyRequest(reportAction) { * @returns {Boolean} */ function canEditFieldOfMoneyRequest(reportAction, reportID, fieldToEdit) { - // A list of fields that cannot be edited, once a money request has been settled + // A list of fields that cannot be edited by anyone, once a money request has been settled const nonEditableFieldsWhenSettled = [ CONST.EDIT_REQUEST_FIELD.AMOUNT, CONST.EDIT_REQUEST_FIELD.CURRENCY, @@ -1530,23 +1530,21 @@ function canEditFieldOfMoneyRequest(reportAction, reportID, fieldToEdit) { CONST.EDIT_REQUEST_FIELD.RECEIPT, CONST.EDIT_REQUEST_FIELD.DISTANCE, ]; - // Checks if the current field is a restricted one + + // Checks if the provided property is a restricted one const isNonEditableFieldWhenSettled = _.includes(nonEditableFieldsWhenSettled, fieldToEdit); - // Checks if the current report is settled one + + // Checks if the report is settled const isSettledMoneyRequest = isSettled(reportID); - // Checks common rules like if the user is a requestor or admin, etc + + // Checks if this user has permissions to edit this money request const canEdit = canEditMoneyRequest(reportAction); - /** - * To satisfy the condition below: - * 1. "canEdit" must be "true". It checks if the current user can, in general, edit a money request. - * 2. When the current edit field is one restricted, the money request shouldn't be settled yet. - * */ if (canEdit && (!isNonEditableFieldWhenSettled || !isSettledMoneyRequest)) { return true; } - // A current user cannot edit the current field of the money request + // Current user cannot edit the provided property of the money request return false; } diff --git a/src/pages/EditRequestPage.js b/src/pages/EditRequestPage.js index 63ae4e37efe5..f039afeb085f 100644 --- a/src/pages/EditRequestPage.js +++ b/src/pages/EditRequestPage.js @@ -111,7 +111,7 @@ function EditRequestPage({betas, report, route, parentReport, policyCategories, // Decides whether to allow or disallow editing a money request useEffect(() => { - // Do not dismiss the modal, when a current user can edit a money request. + // Do not dismiss the modal, when a current user can edit this property of the money request. if (ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, parentReport.reportID, fieldToEdit)) { return; } diff --git a/src/pages/iou/IOUCurrencySelection.js b/src/pages/iou/IOUCurrencySelection.js index 265df8c5e0af..eef0baacd379 100644 --- a/src/pages/iou/IOUCurrencySelection.js +++ b/src/pages/iou/IOUCurrencySelection.js @@ -89,7 +89,7 @@ function IOUCurrencySelection(props) { const report = ReportUtils.getReport(threadReportID); const parentReportAction = ReportActionsUtils.getReportAction(report.parentReportID, report.parentReportActionID); - // Do not dismiss the modal, when a current user can edit a money request. + // Do not dismiss the modal, when a current user can edit this currency of this money request. if (ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, report.parentReportID, CONST.EDIT_REQUEST_FIELD.CURRENCY)) { return; } From f9872692f6440e442bbfd5cf2638f1b238094925 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 18 Oct 2023 15:58:43 +0200 Subject: [PATCH 15/16] simplified function --- src/libs/ReportUtils.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 2412ea86f9b7..34de2e63cd3b 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1531,17 +1531,15 @@ function canEditFieldOfMoneyRequest(reportAction, reportID, fieldToEdit) { CONST.EDIT_REQUEST_FIELD.DISTANCE, ]; - // Checks if the provided property is a restricted one - const isNonEditableFieldWhenSettled = _.includes(nonEditableFieldsWhenSettled, fieldToEdit); - - // Checks if the report is settled - const isSettledMoneyRequest = isSettled(reportID); - // Checks if this user has permissions to edit this money request - const canEdit = canEditMoneyRequest(reportAction); + if (!canEditMoneyRequest(reportAction)) { + return false; // User doesn't have permission to edit + } - if (canEdit && (!isNonEditableFieldWhenSettled || !isSettledMoneyRequest)) { - return true; + // Checks if the report is settled + // Checks if the provided property is a restricted one + if (!isSettled(reportID) || !nonEditableFieldsWhenSettled.includes(fieldToEdit)) { + return true; // User can edit } // Current user cannot edit the provided property of the money request From 8f48a9046020b29bd70d2faf2bc5ccf0f323e7bb Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 18 Oct 2023 16:40:21 +0200 Subject: [PATCH 16/16] simplified function --- src/libs/ReportUtils.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 34de2e63cd3b..c22c680806cf 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1538,12 +1538,7 @@ function canEditFieldOfMoneyRequest(reportAction, reportID, fieldToEdit) { // Checks if the report is settled // Checks if the provided property is a restricted one - if (!isSettled(reportID) || !nonEditableFieldsWhenSettled.includes(fieldToEdit)) { - return true; // User can edit - } - - // Current user cannot edit the provided property of the money request - return false; + return !isSettled(reportID) || !nonEditableFieldsWhenSettled.includes(fieldToEdit); } /**