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

Create Draft reports optimistically #32157

Merged
merged 12 commits into from
Nov 29, 2023
Merged

Create Draft reports optimistically #32157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
23bfbf9
Revert "Merge pull request #29744 from software-mansion-labs/ts-migra…
mountiny Nov 28, 2023
545059a
Check if the linked report is draft on paid policy
mountiny Nov 28, 2023
5f2b55f
Fix style
mountiny Nov 29, 2023
c2030a2
Revert "Revert "Merge pull request #29744 from software-mansion-labs/…
mountiny Nov 29, 2023
52f52d7
Merge branch 'main' into vit-createDraftReports
mountiny Nov 29, 2023
b66b1de
Clean up and address feedback
mountiny Nov 29, 2023
6d7f9c8
Merge main
mountiny Nov 29, 2023
c87ad6a
Access the last message test and html safely
mountiny Nov 29, 2023
4f18257
Make draft expense report name say owed instead of paid
mountiny Nov 29, 2023
a56c5cb
Rename the isReportDraft method to more descriptive isDraftExpenseReport
mountiny Nov 29, 2023
ffcbf2b
Address PR feedback
mountiny Nov 29, 2023
b057c95
Update the state open const
mountiny Nov 29, 2023
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
1 change: 1 addition & 0 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,7 @@ const CONST = {
ADMINS: '#admins',
},
STATE: {
OPEN: 'OPEN',
SUBMITTED: 'SUBMITTED',
PROCESSING: 'PROCESSING',
},
Expand Down
5 changes: 3 additions & 2 deletions src/components/MoneyReportHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function MoneyReportHeader({session, personalDetails, policy, chatReport, nextSt
const isPolicyAdmin = policyType !== CONST.POLICY.TYPE.PERSONAL && lodashGet(policy, 'role') === CONST.POLICY.ROLE.ADMIN;
const isManager = ReportUtils.isMoneyRequestReport(moneyRequestReport) && lodashGet(session, 'accountID', null) === moneyRequestReport.managerID;
const isPayer = policyType === CONST.POLICY.TYPE.CORPORATE ? isPolicyAdmin && isApproved : isPolicyAdmin || (ReportUtils.isMoneyRequestReport(moneyRequestReport) && isManager);
const isDraft = ReportUtils.isReportDraft(moneyRequestReport);
const isDraft = ReportUtils.isDraftExpenseReport(moneyRequestReport);
const shouldShowSettlementButton = useMemo(
() => isPayer && !isDraft && !isSettled && !moneyRequestReport.isWaitingOnBankAccount && reimbursableTotal !== 0 && !ReportUtils.isArchivedRoom(chatReport),
[isPayer, isDraft, isSettled, moneyRequestReport, reimbursableTotal, chatReport],
Expand All @@ -89,7 +89,8 @@ function MoneyReportHeader({session, personalDetails, policy, chatReport, nextSt
return isManager && !isDraft && !isApproved && !isSettled;
}, [policyType, isManager, isDraft, isApproved, isSettled]);
const shouldShowSubmitButton = isDraft && reimbursableTotal !== 0;
const shouldShowNextSteps = isDraft && nextStep && !_.isEmpty(nextStep.message);
const isFromPaidPolicy = policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.CORPORATE;
const shouldShowNextSteps = isFromPaidPolicy && nextStep && !_.isEmpty(nextStep.message);
Comment on lines +92 to +93
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to ensure we show the next steps on any Paid policy expense report in any state/status not only as draft

const shouldShowAnyButton = shouldShowSettlementButton || shouldShowApproveButton || shouldShowSubmitButton || shouldShowNextSteps;
const bankAccountRoute = ReportUtils.getBankAccountRoute(chatReport);
const formattedAmount = CurrencyUtils.convertToDisplayString(reimbursableTotal, moneyRequestReport.currency);
Expand Down
6 changes: 3 additions & 3 deletions src/components/ReportActionItem/ReportPreview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function ReportPreview(props) {
const numberOfRequests = ReportActionUtils.getNumberOfMoneyRequests(props.action);
const moneyRequestComment = lodashGet(props.action, 'childLastMoneyRequestComment', '');
const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(props.chatReport);
const isReportDraft = isPolicyExpenseChat && ReportUtils.isReportDraft(props.iouReport);
const isDraftExpenseReport = isPolicyExpenseChat && ReportUtils.isDraftExpenseReport(props.iouReport);

const transactionsWithReceipts = ReportUtils.getTransactionsWithReceipts(props.iouReportID);
const numberOfScanningReceipts = _.filter(transactionsWithReceipts, (transaction) => TransactionUtils.isReceiptBeingScanned(transaction)).length;
Expand All @@ -143,7 +143,7 @@ function ReportPreview(props) {
scanningReceipts: numberOfScanningReceipts,
});

const shouldShowSubmitButton = isReportDraft && reimbursableSpend !== 0;
const shouldShowSubmitButton = isDraftExpenseReport && reimbursableSpend !== 0;

const getDisplayAmount = () => {
if (hasPendingWaypoints) {
Expand Down Expand Up @@ -189,7 +189,7 @@ function ReportPreview(props) {
const bankAccountRoute = ReportUtils.getBankAccountRoute(props.chatReport);
const shouldShowSettlementButton = ReportUtils.isControlPolicyExpenseChat(props.chatReport)
? props.policy.role === CONST.POLICY.ROLE.ADMIN && ReportUtils.isReportApproved(props.iouReport) && !iouSettled && !iouCanceled
: !_.isEmpty(props.iouReport) && isCurrentUserManager && !isReportDraft && !iouSettled && !iouCanceled && !props.iouReport.isWaitingOnBankAccount && reimbursableSpend !== 0;
: !_.isEmpty(props.iouReport) && isCurrentUserManager && !isDraftExpenseReport && !iouSettled && !iouCanceled && !props.iouReport.isWaitingOnBankAccount && reimbursableSpend !== 0;

return (
<View style={[styles.chatItemMessage, ...props.containerStyles]}>
Expand Down
31 changes: 21 additions & 10 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ type OptimisticExpenseReport = Pick<
| 'reportName'
| 'state'
| 'stateNum'
| 'statusNum'
| 'total'
| 'notificationPreference'
| 'parentReportID'
Expand Down Expand Up @@ -545,6 +546,13 @@ function isReportApproved(report: OnyxEntry<Report>): boolean {
return report?.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && report?.statusNum === CONST.REPORT.STATUS.APPROVED;
}

/**
* Checks if the supplied report is an expense report in Open state and status.
*/
function isDraftExpenseReport(report: OnyxEntry<Report>): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just saw this method... why, oh, why did we introduce a new term draft for what in every other place we call open? 😢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed the terminology used in the App so its clearer for external engineers who dont have context of the Open reports, however, I agree in the hindsight it would be better to follow the backend terminology and keep Drafts as the marketing word only 👍

return isExpenseReport(report) && report?.stateNum === CONST.REPORT.STATE_NUM.OPEN && report?.statusNum === CONST.REPORT.STATUS.OPEN;
}

/**
* Given a collection of reports returns them sorted by last read
*/
Expand Down Expand Up @@ -1669,7 +1677,6 @@ function getPolicyExpenseChatName(report: OnyxEntry<Report>, policy: OnyxEntry<P

/**
* Get the title for an IOU or expense chat which will be showing the payer and the amount
*
*/
function getMoneyRequestReportName(report: OnyxEntry<Report>, policy: OnyxEntry<Policy> | undefined = undefined): string {
const moneyRequestTotal = getMoneyRequestReimbursableTotal(report);
Expand All @@ -1688,7 +1695,7 @@ function getMoneyRequestReportName(report: OnyxEntry<Report>, policy: OnyxEntry<
return Localize.translateLocal('iou.payerSpentAmount', {payer: payerName, amount: formattedAmount});
}

if (!!report?.hasOutstandingIOU || moneyRequestTotal === 0) {
if (!!report?.hasOutstandingIOU || isDraftExpenseReport(report) || moneyRequestTotal === 0) {
return Localize.translateLocal('iou.payerOwesAmount', {payer: payerName, amount: formattedAmount});
}

Expand Down Expand Up @@ -2545,9 +2552,16 @@ function buildOptimisticExpenseReport(chatReportID: string, policyID: string, pa
const storedTotal = total * -1;
const policyName = getPolicyName(allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${chatReportID}`]);
const formattedTotal = CurrencyUtils.convertToDisplayString(storedTotal, currency);
const policy = getPolicy(policyID);

// The expense report is always created with the policy's output currency
const outputCurrency = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`]?.outputCurrency ?? CONST.CURRENCY.USD;
const outputCurrency = policy?.outputCurrency ?? CONST.CURRENCY.USD;
const isFree = policy?.type === CONST.POLICY.TYPE.FREE;

// Define the state and status of the report based on whether the policy is free or paid
const state = isFree ? CONST.REPORT.STATE.SUBMITTED : CONST.REPORT.STATE.OPEN;
const stateNum = isFree ? CONST.REPORT.STATE_NUM.PROCESSING : CONST.REPORT.STATE_NUM.OPEN;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed elsewhere but I will make a follow up PR to clean up the naming mess of processing/ submitted states/ status, to make it all aligned

const statusNum = isFree ? CONST.REPORT.STATUS.SUBMITTED : CONST.REPORT.STATUS.OPEN;

return {
reportID: generateReportID(),
Expand All @@ -2560,8 +2574,9 @@ function buildOptimisticExpenseReport(chatReportID: string, policyID: string, pa

// We don't translate reportName because the server response is always in English
reportName: `${policyName} owes ${formattedTotal}`,
state: CONST.REPORT.STATE.SUBMITTED,
stateNum: CONST.REPORT.STATE_NUM.PROCESSING,
state,
stateNum,
statusNum,
Comment on lines +2578 to +2579
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding statusNum too because Draft report is defined by having both state and status set to 0

total: storedTotal,
notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
parentReportID: chatReportID,
Expand Down Expand Up @@ -4134,10 +4149,6 @@ function getIOUReportActionDisplayMessage(reportAction: OnyxEntry<ReportAction>)
});
}

function isReportDraft(report: OnyxEntry<Report>): boolean {
return isExpenseReport(report) && report?.stateNum === CONST.REPORT.STATE_NUM.OPEN && report?.statusNum === CONST.REPORT.STATUS.OPEN;
}

/**
* Return room channel log display message
*/
Expand Down Expand Up @@ -4372,7 +4383,7 @@ export {
getIOUReportActionDisplayMessage,
isWaitingForAssigneeToCompleteTask,
isGroupChat,
isReportDraft,
isDraftExpenseReport,
shouldUseFullTitleToDisplay,
parseReportRouteParams,
getReimbursementQueuedActionMessage,
Expand Down
78 changes: 58 additions & 20 deletions src/libs/actions/IOU.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,15 @@ function getMoneyRequestInformation(
const isNewIOUReport = !chatReport.iouReportID || ReportUtils.hasIOUWaitingOnCurrentUserBankAccount(chatReport);
let iouReport = isNewIOUReport ? null : allReports[`${ONYXKEYS.COLLECTION.REPORT}${chatReport.iouReportID}`];

// If the linked expense report on paid policy is not draft, we need to create a new draft expense report
if (isPolicyExpenseChat && iouReport) {
const policyType = ReportUtils.getPolicy(iouReport.policyID).type || '';
const isFromPaidPolicy = policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.CORPORATE;
if (isFromPaidPolicy && !ReportUtils.isDraftExpenseReport(iouReport)) {
iouReport = null;
}
}

if (iouReport) {
if (isPolicyExpenseChat) {
iouReport = {...iouReport};
Expand Down Expand Up @@ -2659,30 +2668,46 @@ function approveMoneyRequest(expenseReport) {
*/
function submitReport(expenseReport) {
const optimisticSubmittedReportAction = ReportUtils.buildOptimisticSubmittedReportAction(expenseReport.total, expenseReport.currency, expenseReport.reportID);
const parentReport = ReportUtils.getReport(expenseReport.parentReportID);

const optimisticReportActionsData = {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`,
value: {
[optimisticSubmittedReportAction.reportActionID]: {
...optimisticSubmittedReportAction,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
const optimisticData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`,
value: {
[optimisticSubmittedReportAction.reportActionID]: {
...optimisticSubmittedReportAction,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
},
},
},
};
const optimisticIOUReportData = {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`,
value: {
...expenseReport,
lastMessageText: optimisticSubmittedReportAction.message[0].text,
lastMessageHtml: optimisticSubmittedReportAction.message[0].html,
state: CONST.REPORT.STATE.SUBMITTED,
stateNum: CONST.REPORT.STATE_NUM.PROCESSING,
statusNum: CONST.REPORT.STATUS.SUBMITTED,
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`,
value: {
...expenseReport,
lastMessageText: lodashGet(optimisticSubmittedReportAction, 'message.0.text', ''),
lastMessageHtml: lodashGet(optimisticSubmittedReportAction, 'message.0.html', ''),
state: CONST.REPORT.STATE.SUBMITTED,
stateNum: CONST.REPORT.STATE_NUM.PROCESSING,
statusNum: CONST.REPORT.STATUS.SUBMITTED,
},
},
};
const optimisticData = [optimisticIOUReportData, optimisticReportActionsData];
...(parentReport.reportID
? [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${parentReport.reportID}`,
value: {
...parentReport,
hasOutstandingIOU: false,
hasOutstandingChildRequest: false,
iouReportID: null,
Comment on lines +2703 to +2705
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The submitter does not have an outstanding child request anymore on the workspace chat even if they did not have one before.

removing the iouReportID key ensure we wont try to create new transaction on processing report

},
},
]
: []),
];

const successData = [
{
Expand Down Expand Up @@ -2714,6 +2739,19 @@ function submitReport(expenseReport) {
stateNum: CONST.REPORT.STATE_NUM.OPEN,
},
},
...(parentReport.reportID
? [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${parentReport.reportID}`,
value: {
hasOutstandingIOU: parentReport.hasOutstandingIOU,
hasOutstandingChildRequest: parentReport.hasOutstandingChildRequest,
iouReportID: expenseReport.reportID,
},
},
]
: []),
];

API.write(
Expand Down
Loading