Skip to content

Update logic to display pay button in paid policies #35347

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

Merged
marcochavezf merged 6 commits into from
Feb 9, 2024
Merged

Update logic to display pay button in paid policies #35347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b8ecd41
Add comments to reimbursement choices in CONST.ts
marcochavezf Jan 30, 2024
9d1fd95
Add reimburser email field to Policy type
marcochavezf Jan 30, 2024
147dbb3
Update pay button logic in ReportPreview component
marcochavezf Jan 30, 2024
0f2c953
Update pay logic for payer in MoneyReportHeader
marcochavezf Jan 30, 2024
8417107
Merge branch 'main' into marco-displayPayButtonReimburser
marcochavezf Jan 30, 2024
d6d4081
Merge branch 'main' into marco-displayPayButtonReimburser
marcochavezf Feb 6, 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
6 changes: 3 additions & 3 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1336,9 +1336,9 @@ const CONST = {
OWNER_EMAIL_FAKE: '_FAKE_',
OWNER_ACCOUNT_ID_FAKE: 0,
REIMBURSEMENT_CHOICES: {
REIMBURSEMENT_YES: 'reimburseYes',
REIMBURSEMENT_NO: 'reimburseNo',
REIMBURSEMENT_MANUAL: 'reimburseManual',
REIMBURSEMENT_YES: 'reimburseYes', // Direct
REIMBURSEMENT_NO: 'reimburseNo', // None
REIMBURSEMENT_MANUAL: 'reimburseManual', // Indirect
},
ID_FAKE: '_FAKE_',
EMPTY: 'EMPTY',
Expand Down
3 changes: 2 additions & 1 deletion src/components/MoneyReportHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ function MoneyReportHeader({session, policy, chatReport, nextStep, report: money
const isAutoReimbursable = ReportUtils.canBeAutoReimbursed(moneyRequestReport, policy);
const isPaidGroupPolicy = ReportUtils.isPaidGroupPolicy(moneyRequestReport);
const isManager = ReportUtils.isMoneyRequestReport(moneyRequestReport) && session?.accountID === moneyRequestReport.managerID;
const isReimburser = session?.email === policy?.reimburserEmail;

@c3024 c3024 Feb 6, 2024

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.

Might it be better to send the reimburser's accountID and compare it with session.accountID?

I think we standardised all to compare accountIDs. With comparing accountIDs, even if the reimburser changes their primary email, this check works fine. Reimbursers's email address can also remain hidden if we send only reimburser's accountID (though I don't think this is of much use.)

@marcochavezf marcochavezf Feb 6, 2024

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.

Oh good point, the problem is that we're setting the reimburser email in the backend from OldDot. I agree we should compare accountIDs but that means we'd need to refactor also OldDot to save the accountID when the reimburser is selected.

Said that, let's go for now with this change and we can refactor later to send the accountID when it becomes a problem or when the dashboard for policy settings is migrated to NewDot.

@c3024 c3024 Feb 7, 2024 •
edited
Loading

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.

If the reimburser changes their primary email, then we might check to see if any of the reimbursers' validated email ids match with policy reimburser email. This is a rare and unlikely case and I am not sure if extra verbosity added is worth it. @marcochavezf

@grgia grgia Feb 9, 2024

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.

@marcochavezf should we just update BE now to send the accountID? Or do we assume that all policy users should have access to this email info

const isPayer = isPaidGroupPolicy
? // In a group policy, the admin approver can pay the report directly by skipping the approval step
isPolicyAdmin && (isApproved || isManager)
isReimburser && (isApproved || isManager)
: isPolicyAdmin || (ReportUtils.isMoneyRequestReport(moneyRequestReport) && isManager);
const isDraft = ReportUtils.isDraftExpenseReport(moneyRequestReport);
const shouldShowPayButton = useMemo(
Expand Down
3 changes: 2 additions & 1 deletion src/components/ReportActionItem/ReportPreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ function ReportPreview({
const {totalDisplaySpend, reimbursableSpend} = ReportUtils.getMoneyRequestSpendBreakdown(iouReport);
const policyType = policy?.type;
const isAutoReimbursable = ReportUtils.canBeAutoReimbursed(iouReport, policy);
const isReimburser = session?.email === policy?.reimburserEmail;

const iouSettled = ReportUtils.isSettled(iouReportID);
const iouCanceled = ReportUtils.isArchivedRoom(chatReport);
Expand Down Expand Up @@ -217,7 +218,7 @@ function ReportPreview({
const isPolicyAdmin = policyType !== CONST.POLICY.TYPE.PERSONAL && policy?.role === CONST.POLICY.ROLE.ADMIN;
const isPayer = isPaidGroupPolicy
? // In a paid group policy, the admin approver can pay the report directly by skipping the approval step
isPolicyAdmin && (isApproved || isCurrentUserManager)
isReimburser && (isApproved || isCurrentUserManager)
: isPolicyAdmin || (isMoneyRequestReport && isCurrentUserManager);
const shouldShowPayButton = useMemo(
() => isPayer && !isDraftExpenseReport && !iouSettled && !iouReport?.isWaitingOnBankAccount && reimbursableSpend !== 0 && !iouCanceled && !isAutoReimbursable,
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/Policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ type Policy = {
/** When tax tracking is enabled */
isTaxTrackingEnabled?: boolean;

/** The email of the reimburser set when reimbursement is direct */
reimburserEmail?: string;

/** ReportID of the admins room for this workspace */
chatReportIDAdmins?: number;

Expand Down