Repository navigation
Fix change workspace condition #62062
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.
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
Changes from all commits
e63e19c
8a4de6a
d0ea15e
b928bd5
98f492d
77a9d7a
437272c
75d1454
a5ead71
7af8600
8188854
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,7 +72,6 @@ import type {Comment, TransactionChanges, WaypointCollection} from '@src/types/o | |
| import {isEmptyObject} from '@src/types/utils/EmptyObject'; | ||
| import type IconAsset from '@src/types/utils/IconAsset'; | ||
| import {createDraftTransaction, getIOUReportActionToApproveOrPay, setMoneyRequestParticipants, unholdRequest} from './actions/IOU'; | ||
| import {isApprover as isApproverMember} from './actions/Policy/Member'; | ||
| import {createDraftWorkspace} from './actions/Policy/Policy'; | ||
| import {autoSwitchToFocusMode} from './actions/PriorityMode'; | ||
| import {hasCreditBankAccount} from './actions/ReimbursementAccount/store'; | ||
|
|
@@ -129,6 +128,7 @@ import { | |
| isPaidGroupPolicy as isPaidGroupPolicyPolicyUtils, | ||
| isPolicyAdmin as isPolicyAdminPolicyUtils, | ||
| isPolicyAuditor, | ||
| isPolicyMember, | ||
| isPolicyOwner, | ||
| isSubmitAndClose, | ||
| shouldShowPolicy, | ||
|
|
@@ -3823,6 +3823,40 @@ function canEditMoneyRequest(reportAction: OnyxInputOrEntry<ReportAction<typeof | |
| return !isReportApproved({report: moneyRequestReport}) && !isSettled(moneyRequestReport?.reportID) && !isClosedReport(moneyRequestReport) && isRequestor; | ||
| } | ||
|
|
||
| function canEditReportPolicy(report: OnyxEntry<Report>, reportPolicy: OnyxEntry<Policy>): boolean { | ||
| const isAdmin = isPolicyAdminPolicyUtils(reportPolicy); | ||
| const isManager = isReportManager(report); | ||
| const isSubmitter = isReportOwner(report); | ||
| const isReportAuditor = isAuditor(report); | ||
| const isIOUType = isIOUReport(report); | ||
| const isInvoiceType = isInvoiceReport(report); | ||
| const isExpenseType = isExpenseReport(report); | ||
| const isOpen = isOpenReport(report); | ||
| const isSubmitted = isProcessingReport(report); | ||
|
|
||
| if (isIOUType) { | ||
| return isOpen || isSubmitted; | ||
| } | ||
|
|
||
| if (isInvoiceType) { | ||
| return isOpen && !isReportAuditor; | ||
| } | ||
|
|
||
| if (isExpenseType) { | ||
| if (isOpen) { | ||
| return isSubmitter || isAdmin; | ||
| } | ||
|
|
||
| if (isSubmitted) { | ||
| return (isSubmitter && isAwaitingFirstLevelApproval(report)) || isManager || isAdmin; | ||
| } | ||
|
|
||
| return isManager || isAdmin; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the current user can edit the provided property of an expense | ||
| * | ||
|
|
@@ -10446,78 +10480,12 @@ function hasExportError(reportActions: OnyxEntry<ReportActions> | ReportAction[] | |
| return Object.values(reportActions).some((action) => isIntegrationMessageAction(action)); | ||
| } | ||
|
|
||
| function verifyState(report: OnyxEntry<Report>, validStates: Array<ValueOf<typeof CONST.REPORT.STATE_NUM>>): boolean { | ||
| if (report?.stateNum === undefined || report?.stateNum === null) { | ||
| return false; | ||
| } | ||
| return validStates.includes(report?.stateNum); | ||
| } | ||
|
|
||
| function verifyStatus(report: OnyxEntry<Report>, validStatuses: Array<ValueOf<typeof CONST.REPORT.STATUS_NUM>>): boolean { | ||
| if (report?.statusNum === undefined || report?.statusNum === null) { | ||
| return false; | ||
| } | ||
| return validStatuses.includes(report?.statusNum); | ||
| } | ||
|
|
||
| /** | ||
| * Determines whether the report can be moved to the workspace. | ||
| */ | ||
| function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry<Policy>, report: OnyxEntry<Report>, session: OnyxEntry<Session>, currentPolicy?: OnyxEntry<Policy>): boolean { | ||
| if (!session?.accountID) { | ||
| return false; | ||
| } | ||
|
|
||
| const isIOU = isIOUReport(report); | ||
| const submitterLogin = report?.ownerAccountID && getLoginByAccountID(report?.ownerAccountID); | ||
| const isSubmitterMember = !!submitterLogin && !!newPolicy?.employeeList?.[submitterLogin]; | ||
| const managerLogin = report?.managerID && getLoginByAccountID(report?.managerID); | ||
| const isManagerMember = !!managerLogin && !!newPolicy?.employeeList?.[managerLogin]; | ||
| const isCurrentUserAdmin = isPolicyAdminPolicyUtils(newPolicy, session?.email); | ||
| const isAdminOfCurrentPolicy = isPolicyAdminPolicyUtils(currentPolicy, session?.email); | ||
| const isPaidGroupPolicyType = isPaidGroupPolicyPolicyUtils(newPolicy); | ||
| const isReportOpenOrSubmitted = verifyState(report, [CONST.REPORT.STATE_NUM.OPEN, CONST.REPORT.STATE_NUM.SUBMITTED]); | ||
|
|
||
| // For IOUs, the sender and receiver can only change the workspace if: | ||
| // 1. The sender AND receiver are both members of the new policy OR | ||
| // 2. The sender OR receiver is an admin of the new policy. In this case, changing the policy also invites the non-member to the policy | ||
| if (isIOU && isReportOpenOrSubmitted && isPaidGroupPolicyType && ((isSubmitterMember && isManagerMember) || isCurrentUserAdmin)) { | ||
| return true; | ||
| } | ||
|
|
||
| // From this point on, reports must be of type Expense, the policy must be a paid type. | ||
| // The submitter and manager must also be policy members OR the current user is an admin so they can invite the non-members to the policy. | ||
| // Additionally, if the report is not open or submitted, the current user must be an admin. | ||
| // We're temporarily disabling moving reports to a workspace if the submitter is not a member of the new policy because this flow requires additional API changes. | ||
| const isExpenseReportType = isExpenseReport(report); | ||
| if (!isExpenseReportType || !isPaidGroupPolicyType || !isSubmitterMember || (!isReportOpenOrSubmitted && !isAdminOfCurrentPolicy)) { | ||
| return false; | ||
| } | ||
|
|
||
| const isCurrentUserReportSubmitter = session.accountID === report?.ownerAccountID; | ||
| if (isCurrentUserReportSubmitter) { | ||
| return true; | ||
| } | ||
|
|
||
| const isCurrentUserReportApprover = isApproverMember(newPolicy, session.accountID); | ||
| if (isCurrentUserReportApprover && verifyState(report, [CONST.REPORT.STATE_NUM.SUBMITTED])) { | ||
| return true; | ||
| } | ||
|
|
||
| const isCurrentUserReportPayer = isPayer(session, report, false, newPolicy); | ||
| if (isCurrentUserReportPayer && verifyState(report, [CONST.REPORT.STATE_NUM.APPROVED])) { | ||
| return true; | ||
| } | ||
|
|
||
| if ( | ||
| isCurrentUserAdmin && | ||
| verifyState(report, [CONST.REPORT.STATE_NUM.APPROVED]) && | ||
| verifyStatus(report, [CONST.REPORT.STATUS_NUM.APPROVED, CONST.REPORT.STATUS_NUM.REIMBURSED, CONST.REPORT.STATUS_NUM.CLOSED]) | ||
| ) { | ||
| return true; | ||
| } | ||
|
Comment on lines
-10513
to
-10518
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Coming from the #76421 checklist: during the refactor and cleanup of conditions, we missed this case. As a result, the available workspace is shown, but the backend prevents the change |
||
|
|
||
| return false; | ||
| function isWorkspaceEligibleForReportChange(newPolicy: OnyxEntry<Policy>, report: OnyxEntry<Report>, policies: OnyxCollection<Policy>): boolean { | ||
| const submitterEmail = getLoginByAccountID(report?.ownerAccountID ?? CONST.DEFAULT_NUMBER_ID); | ||
| return isPaidGroupPolicyPolicyUtils(newPolicy) && (isPolicyMember(submitterEmail, newPolicy?.id) || isPolicyAdmin(newPolicy?.id, policies)); | ||
|
Comment on lines
+10486
to
+10488
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NAB: Currently, the solution section lacks a description of this change, so we can't verify this logic.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, I need to update the doc with this logic.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this change returns incorrect available workspaces Screen.Recording.2025-05-21.at.17.25.16.mp4
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah the workspaces are correct, but this request is failing with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BE PR is still WIP. @suneox let's continue review here and not block on that.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got it. Overall, the change set is look good. I'm just a bit confused about this part, so I'll continue the checklist based on the current state.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Coming from #79075. We should also check here for |
||
| } | ||
|
|
||
| function getApprovalChain(policy: OnyxEntry<Policy>, expenseReport: OnyxEntry<Report>): string[] { | ||
|
|
@@ -10792,6 +10760,7 @@ export { | |
| canCurrentUserOpenReport, | ||
| canDeleteReportAction, | ||
| canHoldUnholdReportAction, | ||
| canEditReportPolicy, | ||
| canEditFieldOfMoneyRequest, | ||
| canEditMoneyRequest, | ||
| canEditPolicyDescription, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
This behavior is now considered incorrect and was removed in #88530