Add checks for editing APPROVED reports #33633
There are no files selected for viewing
This file contains 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 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 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 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 |
|---|---|---|
|
|
@@ -541,7 +541,8 @@ function isExpenseReport(report: OnyxEntry<Report> | EmptyObject): boolean { | |
| /** | ||
| * Checks if a report is an IOU report. | ||
| */ | ||
| function isIOUReport(reportOrID: OnyxEntry<Report> | string | EmptyObject): boolean { | ||
| const report = typeof reportOrID === 'string' ? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportOrID}`] ?? null : reportOrID; | ||
| return report?.type === CONST.REPORT.TYPE.IOU; | ||
| } | ||
|
|
||
|
|
@@ -597,7 +598,8 @@ function isReportManager(report: OnyxEntry<Report>): boolean { | |
| /** | ||
| * Checks if the supplied report has been approved | ||
| */ | ||
| function isReportApproved(reportOrID: OnyxEntry<Report> | string | EmptyObject): boolean { | ||
| const report = typeof reportOrID === 'string' ? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportOrID}`] ?? null : reportOrID; | ||
| return report?.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && report?.statusNum === CONST.REPORT.STATUS.APPROVED; | ||
| } | ||
|
|
||
|
|
@@ -814,10 +816,17 @@ function isConciergeChatReport(report: OnyxEntry<Report>): boolean { | |
| /** | ||
| * Returns true if report is still being processed | ||
| */ | ||
| function isProcessingReport(report: OnyxEntry<Report> | EmptyObject): boolean { | ||
| return report?.stateNum === CONST.REPORT.STATE_NUM.PROCESSING && report?.statusNum === CONST.REPORT.STATUS.SUBMITTED; | ||
| } | ||
|
|
||
| /** | ||
| * Returns true if report is still open | ||
| */ | ||
| function isOpenReport(report: OnyxEntry<Report> | EmptyObject): boolean { | ||
| return report?.stateNum === CONST.REPORT.STATE_NUM.OPEN && report?.statusNum === CONST.REPORT.STATUS.OPEN; | ||
| } | ||
|
|
||
mountiny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** | ||
| * Check if the report is a single chat report that isn't a thread | ||
| * and personal detail of participant is optimistic data | ||
|
|
@@ -1845,9 +1854,13 @@ function getTransactionDetails(transaction: OnyxEntry<Transaction>, createdDateF | |
| * - the current user is the requestor and is not settled yet | ||
| * - in case of expense report | ||
| * - the current user is the requestor and is not settled yet | ||
| * - the current user is the manager of the report | ||
| * - or the current user is an admin on the policy the expense report is tied to | ||
| * | ||
| * This is used in conjuction with canEditRestrictedField to control editing of specific fields like amount, currency, created, and receipt. | ||
| * On its own, it only controls allowing/disallowing navigating to the editing pages or showing/hiding the 'Edit' icon on report actions | ||
| */ | ||
| function canEditMoneyRequest(reportAction: OnyxEntry<ReportAction>): boolean { | ||
| const isDeleted = ReportActionsUtils.isDeletedAction(reportAction); | ||
|
|
||
| if (isDeleted) { | ||
|
|
@@ -1870,52 +1883,59 @@ function canEditMoneyRequest(reportAction: OnyxEntry<ReportAction>, fieldToEdit | |
| } | ||
|
|
||
| const moneyRequestReport = getReport(String(moneyRequestReportID)); | ||
| const policy = getPolicy(moneyRequestReport?.policyID ?? ''); | ||
| const isAdmin = isExpenseReport(moneyRequestReport) && policy.role === CONST.POLICY.ROLE.ADMIN; | ||
| const isManager = isExpenseReport(moneyRequestReport) && currentUserAccountID === moneyRequestReport?.managerID; | ||
| const isRequestor = currentUserAccountID === reportAction?.actorAccountID; | ||
|
|
||
|
There was a problem hiding this comment. This logic is moved to |
||
| // Admin & managers can always edit coding fields such as tag, category, billable, etc. As long as the report has a state higher than OPEN. | ||
| if ((isAdmin || isManager) && !isOpenReport(moneyRequestReport)) { | ||
| return true; | ||
| } | ||
|
|
||
| return !isReportApproved(moneyRequestReport) && !isSettled(moneyRequestReport?.reportID) && isRequestor; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the current user can edit the provided property of a money request | ||
| * | ||
| */ | ||
| function canEditFieldOfMoneyRequest(reportAction: OnyxEntry<ReportAction>, fieldToEdit: ValueOf<typeof CONST.EDIT_REQUEST_FIELD>): boolean { | ||
| // A list of fields that cannot be edited by anyone, once a money request has been settled | ||
| const restrictedFields: string[] = [ | ||
| CONST.EDIT_REQUEST_FIELD.AMOUNT, | ||
| CONST.EDIT_REQUEST_FIELD.CURRENCY, | ||
| CONST.EDIT_REQUEST_FIELD.MERCHANT, | ||
| CONST.EDIT_REQUEST_FIELD.DATE, | ||
| CONST.EDIT_REQUEST_FIELD.RECEIPT, | ||
| CONST.EDIT_REQUEST_FIELD.DISTANCE, | ||
| ]; | ||
|
|
||
| if (!canEditMoneyRequest(reportAction)) { | ||
| return false; | ||
| } | ||
|
|
||
| // If we're editing fields such as category, tag, description, etc. the check above should be enough for handling the permission | ||
| if (!restrictedFields.includes(fieldToEdit)) { | ||
| return true; | ||
| } | ||
|
|
||
| const iouMessage = reportAction?.originalMessage as IOUMessage; | ||
| if (isSettled(String(iouMessage.IOUReportID)) || isReportApproved(String(iouMessage.IOUReportID))) { | ||
| return false; | ||
| } | ||
|
|
||
| const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${iouMessage?.IOUTransactionID}`] ?? ({} as Transaction); | ||
| if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.AMOUNT || fieldToEdit === CONST.EDIT_REQUEST_FIELD.CURRENCY) { | ||
| return !TransactionUtils.isCardTransaction(transaction); | ||
| } | ||
|
|
||
| if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.RECEIPT && TransactionUtils.hasReceipt(transaction)) { | ||
| const isRequestor = currentUserAccountID === reportAction?.actorAccountID; | ||
| return !TransactionUtils.isReceiptBeingScanned(transaction) && !TransactionUtils.isDistanceRequest(transaction) && isRequestor; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -4307,6 +4327,7 @@ export { | |
| formatReportLastMessageText, | ||
| chatIncludesConcierge, | ||
| isPolicyExpenseChat, | ||
| isGroupPolicy, | ||
| isControlPolicyExpenseChat, | ||
| isControlPolicyExpenseReport, | ||
| isGroupPolicyExpenseChat, | ||
|
|
||
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.
@mountiny woops this doesn't take into account Free policies, I'll revert this back to testing the policy is not personal
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.
Can you also rename it to isPaidGroupPolicy?
And create a new one name isGroupPolicy checking for free too? it can be done in a follow up too
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.
yeah that sounds good
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.
done, though I just found out we have the same method in
PolicyUtilsThere 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.
I think we're creating a lot of redundant helper methods we should probably some day clean things up :D