Skip to content

Commit

Permalink
Merge pull request #36388 from Expensify/youssef_instant_submit_collect
Browse files Browse the repository at this point in the history 
Improve creating expenses on Collect with Instant Submit
  • Loading branch information
@youssef-lr
youssef-lr authored Mar 7, 2024
2 parents 15cf6ad + 3fcd191 commit 9bbe923
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 73 deletions.
16 changes: 5 additions & 11 deletions src/components/MoneyRequestHeader.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import * as HeaderUtils from '@libs/HeaderUtils';
import Navigation from '@libs/Navigation/Navigation';
import * as PolicyUtils from '@libs/PolicyUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import * as TransactionUtils from '@libs/TransactionUtils';
Expand Down Expand Up @@ -79,16 +78,11 @@ function MoneyRequestHeader({session, parentReport, report, parentReportAction,
const isScanning = TransactionUtils.hasReceipt(transaction) && TransactionUtils.isReceiptBeingScanned(transaction);
const isPending = TransactionUtils.isExpensifyCardTransaction(transaction) && TransactionUtils.isPending(transaction);

const isRequestModifiable = !isSettled && !isApproved && !ReportActionsUtils.isDeletedAction(parentReportAction);
const canModifyRequest = isActionOwner && !isSettled && !isApproved && !ReportActionsUtils.isDeletedAction(parentReportAction);
let canDeleteRequest = canModifyRequest;
const isDeletedParentAction = ReportActionsUtils.isDeletedAction(parentReportAction);
const canHoldOrUnholdRequest = !isSettled && !isApproved && !isDeletedParentAction;

if (ReportUtils.isPaidGroupPolicyExpenseReport(moneyRequestReport)) {
// If it's a paid policy expense report, only allow deleting the request if it's in draft state or instantly submitted state or the user is the policy admin
canDeleteRequest =
canDeleteRequest &&
(ReportUtils.isDraftExpenseReport(moneyRequestReport) || ReportUtils.isExpenseReportWithInstantSubmittedState(moneyRequestReport) || PolicyUtils.isPolicyAdmin(policy));
}
// If the report supports adding transactions to it, then it also supports deleting transactions from it.
const canDeleteRequest = isActionOwner && ReportUtils.canAddOrDeleteTransactions(moneyRequestReport) && !isDeletedParentAction;

const changeMoneyRequestStatus = () => {
if (isOnHold) {
Expand All @@ -108,7 +102,7 @@ function MoneyRequestHeader({session, parentReport, report, parentReportAction,
}, [canDeleteRequest]);

const threeDotsMenuItems = [HeaderUtils.getPinMenuItem(report)];
if (isRequestModifiable) {
if (canHoldOrUnholdRequest) {
const isRequestIOU = parentReport?.type === 'iou';
const isHoldCreator = ReportUtils.isHoldCreator(transaction, report?.reportID) && isRequestIOU;
const canModifyStatus = isPolicyAdmin || isActionOwner || isApprover;
Expand Down
2 changes: 1 addition & 1 deletion src/libs/PolicyUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ function isPaidGroupPolicy(policy: OnyxEntry<Policy> | EmptyObject): boolean {
* Checks if policy's scheduled submit / auto reporting frequency is "instant".
* Note: Free policies have "instant" submit always enabled.
*/
function isInstantSubmitEnabled(policy: OnyxEntry<Policy>): boolean {
function isInstantSubmitEnabled(policy: OnyxEntry<Policy> | EmptyObject): boolean {
return policy?.autoReportingFrequency === CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT || policy?.type === CONST.POLICY.TYPE.FREE;
}

Expand Down
68 changes: 44 additions & 24 deletions src/libs/ReportUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -959,14 +959,6 @@ function isProcessingReport(report: OnyxEntry<Report> | EmptyObject): boolean {
return report?.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && report?.statusNum === CONST.REPORT.STATUS_NUM.SUBMITTED;
}

/**
* Returns true if the policy has `instant` reporting frequency and if the report is still being processed (i.e. submitted state)
*/
function isExpenseReportWithInstantSubmittedState(report: OnyxEntry<Report> | EmptyObject): boolean {
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`] ?? null;
return isExpenseReport(report) && isProcessingReport(report) && PolicyUtils.isInstantSubmitEnabled(policy);
}

/**
* Check if the report is a single chat report that isn't a thread
* and personal detail of participant is optimistic data
Expand Down Expand Up @@ -1279,6 +1271,29 @@ function getChildReportNotificationPreference(reportAction: OnyxEntry<ReportActi
return isActionCreator(reportAction) ? CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS : CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;
}

/**
* Checks whether the supplied report supports adding more transactions to it.
* Return true if:
* - report is a non-settled IOU
* - report is a draft
* - report is a processing expense report and its policy has Instant reporting frequency
*/
function canAddOrDeleteTransactions(moneyRequestReport: OnyxEntry<Report>): boolean {
if (!isMoneyRequestReport(moneyRequestReport)) {
return false;
}

if (isReportApproved(moneyRequestReport) || isSettled(moneyRequestReport?.reportID)) {
return false;
}

if (isGroupPolicy(moneyRequestReport) && isProcessingReport(moneyRequestReport) && !PolicyUtils.isInstantSubmitEnabled(getPolicy(moneyRequestReport?.policyID))) {
return false;
}

return true;
}

/**
* Can only delete if the author is this user and the action is an ADDCOMMENT action or an IOU action in an unsettled report, or if the user is a
* policy admin
Expand All @@ -1293,14 +1308,13 @@ function canDeleteReportAction(reportAction: OnyxEntry<ReportAction>, reportID:
// For now, users cannot delete split actions
const isSplitAction = reportAction?.originalMessage?.type === CONST.IOU.REPORT_ACTION_TYPE.SPLIT;

if (isSplitAction || isSettled(String(reportAction?.originalMessage?.IOUReportID)) || (!isEmptyObject(report) && isReportApproved(report))) {
if (isSplitAction) {
return false;
}

if (isActionOwner) {
if (!isEmptyObject(report) && isPaidGroupPolicyExpenseReport(report)) {
// If it's a paid policy expense report, only allow deleting the request if it's a draft or is instantly submitted or the user is the policy admin
return isDraftExpenseReport(report) || isExpenseReportWithInstantSubmittedState(report) || PolicyUtils.isPolicyAdmin(policy);
if (!isEmptyObject(report) && isMoneyRequestReport(report)) {
return canAddOrDeleteTransactions(report);
}
return true;
}
Expand Down Expand Up @@ -2983,11 +2997,10 @@ function buildOptimisticExpenseReport(chatReportID: string, policyID: string, pa
const formattedTotal = CurrencyUtils.convertToDisplayString(storedTotal, currency);
const policy = getPolicy(policyID);

const isFree = policy?.type === CONST.POLICY.TYPE.FREE;
const isInstantSubmitEnabled = PolicyUtils.isInstantSubmitEnabled(policy);

// Define the state and status of the report based on whether the policy is free or paid
const stateNum = isFree ? CONST.REPORT.STATE_NUM.SUBMITTED : CONST.REPORT.STATE_NUM.OPEN;
const statusNum = isFree ? CONST.REPORT.STATUS_NUM.SUBMITTED : CONST.REPORT.STATUS_NUM.OPEN;
const stateNum = isInstantSubmitEnabled ? CONST.REPORT.STATE_NUM.SUBMITTED : CONST.REPORT.STATE_NUM.OPEN;
const statusNum = isInstantSubmitEnabled ? CONST.REPORT.STATUS_NUM.SUBMITTED : CONST.REPORT.STATUS_NUM.OPEN;

const expenseReport: OptimisticExpenseReport = {
reportID: generateReportID(),
Expand Down Expand Up @@ -4319,7 +4332,6 @@ function canRequestMoney(report: OnyxEntry<Report>, policy: OnyxEntry<Policy>, o
return false;
}

// In case of expense reports, we have to look at the parent workspace chat to get the isOwnPolicyExpenseChat property
let isOwnPolicyExpenseChat = report?.isOwnPolicyExpenseChat ?? false;
if (isExpenseReport(report) && getParentReport(report)) {
isOwnPolicyExpenseChat = Boolean(getParentReport(report)?.isOwnPolicyExpenseChat);
Expand All @@ -4333,12 +4345,8 @@ function canRequestMoney(report: OnyxEntry<Report>, policy: OnyxEntry<Policy>, o
// User can request money in any IOU report, unless paid, but user can only request money in an expense report
// which is tied to their workspace chat.
if (isMoneyRequestReport(report)) {
const isOwnExpenseReport = isExpenseReport(report) && isOwnPolicyExpenseChat;
if (isOwnExpenseReport && PolicyUtils.isPaidGroupPolicy(policy)) {
return isDraftExpenseReport(report) || isExpenseReportWithInstantSubmittedState(report);
}

return (isOwnExpenseReport || isIOUReport(report)) && !isReportApproved(report) && !isSettled(report?.reportID);
const canAddTransactions = canAddOrDeleteTransactions(report);
return isGroupPolicy(report) ? isOwnPolicyExpenseChat && canAddTransactions : canAddTransactions;
}

// In case of policy expense chat, users can only request money from their own policy expense chat
Expand Down Expand Up @@ -5094,6 +5102,17 @@ function canBeAutoReimbursed(report: OnyxEntry<Report>, policy: OnyxEntry<Policy
return isAutoReimbursable;
}

/**
* Used from money request actions to decide if we need to build an optimistic money request report.
Create a new report if:
- we don't have an iouReport set in the chatReport
- we have one, but it's waiting on the payee adding a bank account
- we have one but we can't add more transactions to it due to: report is approved or settled, or report is processing and policy isn't on Instant submit reporting frequency
*/
function shouldCreateNewMoneyRequestReport(existingIOUReport: OnyxEntry<Report> | undefined | null, chatReport: OnyxEntry<Report> | null): boolean {
return !existingIOUReport || hasIOUWaitingOnCurrentUserBankAccount(chatReport) || !canAddOrDeleteTransactions(existingIOUReport);
}

export {
getReportParticipantsTitle,
isReportMessageAttachment,
Expand Down Expand Up @@ -5125,7 +5144,6 @@ export {
isPublicAnnounceRoom,
isConciergeChatReport,
isProcessingReport,
isExpenseReportWithInstantSubmittedState,
isCurrentUserTheOnlyParticipant,
hasAutomatedExpensifyAccountIDs,
hasExpensifyGuidesEmails,
Expand Down Expand Up @@ -5297,6 +5315,8 @@ export {
canEditRoomVisibility,
canEditPolicyDescription,
getPolicyDescriptionText,
canAddOrDeleteTransactions,
shouldCreateNewMoneyRequestReport,
};

export type {
Expand Down
40 changes: 13 additions & 27 deletions src/libs/actions/IOU.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -845,37 +845,26 @@ function getMoneyRequestInformation(
// STEP 2: Get the money request report. If the moneyRequestReportID has been provided, we want to add the transaction to this specific report.
// If no such reportID has been provided, let's use the chatReport.iouReportID property. In case that is not present, build a new optimistic money request report.
let iouReport: OnyxEntry<OnyxTypes.Report> = null;
const shouldCreateNewMoneyRequestReport = !moneyRequestReportID && (!chatReport.iouReportID || ReportUtils.hasIOUWaitingOnCurrentUserBankAccount(chatReport));
if (moneyRequestReportID) {
iouReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${moneyRequestReportID}`] ?? null;
} else if (!shouldCreateNewMoneyRequestReport) {
} else {
iouReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${chatReport.iouReportID}`] ?? null;
}

let isFromPaidPolicy = false;
if (isPolicyExpenseChat) {
isFromPaidPolicy = PolicyUtils.isPaidGroupPolicy(policy ?? null);
const shouldCreateNewMoneyRequestReport = ReportUtils.shouldCreateNewMoneyRequestReport(iouReport, chatReport);

// If the linked expense report on paid policy is not draft and not instantly submitted, we need to create a new draft expense report
if (iouReport && isFromPaidPolicy && !ReportUtils.isDraftExpenseReport(iouReport) && !ReportUtils.isExpenseReportWithInstantSubmittedState(iouReport)) {
iouReport = null;
}
}

if (iouReport) {
if (isPolicyExpenseChat) {
iouReport = {...iouReport};
if (iouReport?.currency === currency && typeof iouReport.total === 'number') {
// Because of the Expense reports are stored as negative values, we subtract the total from the amount
iouReport.total -= amount;
}
} else {
iouReport = IOUUtils.updateIOUOwnerAndTotal(iouReport, payeeAccountID, amount, currency);
}
} else {
if (!iouReport || shouldCreateNewMoneyRequestReport) {
iouReport = isPolicyExpenseChat
? ReportUtils.buildOptimisticExpenseReport(chatReport.reportID, chatReport.policyID ?? '', payeeAccountID, amount, currency)
: ReportUtils.buildOptimisticIOUReport(payeeAccountID, payerAccountID, amount, chatReport.reportID, currency);
} else if (isPolicyExpenseChat) {
iouReport = {...iouReport};
if (iouReport?.currency === currency && typeof iouReport.total === 'number') {
// Because of the Expense reports are stored as negative values, we subtract the total from the amount
iouReport.total -= amount;
}
} else {
iouReport = IOUUtils.updateIOUOwnerAndTotal(iouReport, payeeAccountID, amount, currency);
}

// STEP 3: Build optimistic receipt and transaction
Expand Down Expand Up @@ -1860,10 +1849,8 @@ function createSplitsAndOnyxData(
}

// STEP 2: Get existing IOU/Expense report and update its total OR build a new optimistic one
// For Control policy expense chats, if the report is already approved, create a new expense report
let oneOnOneIOUReport: OneOnOneIOUReport = oneOnOneChatReport.iouReportID ? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${oneOnOneChatReport.iouReportID}`] : null;
const shouldCreateNewOneOnOneIOUReport =
!oneOnOneIOUReport || (isOwnPolicyExpenseChat && ReportUtils.isControlPolicyExpenseReport(oneOnOneIOUReport) && ReportUtils.isReportApproved(oneOnOneIOUReport));
const shouldCreateNewOneOnOneIOUReport = ReportUtils.shouldCreateNewMoneyRequestReport(oneOnOneIOUReport, oneOnOneChatReport);

if (!oneOnOneIOUReport || shouldCreateNewOneOnOneIOUReport) {
oneOnOneIOUReport = isOwnPolicyExpenseChat
Expand Down Expand Up @@ -2501,8 +2488,7 @@ function completeSplitBill(chatReportID: string, reportAction: OnyxTypes.ReportA
}

let oneOnOneIOUReport: OneOnOneIOUReport = oneOnOneChatReport?.iouReportID ? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${oneOnOneChatReport.iouReportID}`] : null;
const shouldCreateNewOneOnOneIOUReport =
!oneOnOneIOUReport || (isPolicyExpenseChat && ReportUtils.isControlPolicyExpenseReport(oneOnOneIOUReport) && ReportUtils.isReportApproved(oneOnOneIOUReport));
const shouldCreateNewOneOnOneIOUReport = ReportUtils.shouldCreateNewMoneyRequestReport(oneOnOneIOUReport, oneOnOneChatReport);

if (!oneOnOneIOUReport || shouldCreateNewOneOnOneIOUReport) {
oneOnOneIOUReport = isPolicyExpenseChat
Expand Down
52 changes: 42 additions & 10 deletions tests/unit/ReportUtilsTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,21 +410,26 @@ describe('ReportUtils', () => {
});
});

it("it is a non-open expense report tied to user's own paid policy expense chat", () => {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}101`, {
reportID: '101',
chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT,
isOwnPolicyExpenseChat: true,
}).then(() => {
it("it is a submitted report tied to user's own policy expense chat and the policy does not have Instant Submit frequency", () => {
const paidPolicy = {
id: '3f54cca8',
type: CONST.POLICY.TYPE.TEAM,
};
Promise.all([
Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${paidPolicy.id}`, paidPolicy),
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}101`, {
reportID: '101',
chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT,
isOwnPolicyExpenseChat: true,
}),
]).then(() => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.EXPENSE,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
parentReportID: '101',
};
const paidPolicy = {
type: CONST.POLICY.TYPE.TEAM,
policyID: paidPolicy.id,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, paidPolicy, [currentUserAccountID, participantsAccountIDs[0]]);
expect(moneyRequestOptions.length).toBe(0);
Expand Down Expand Up @@ -498,7 +503,7 @@ describe('ReportUtils', () => {
});
});

it("it is an open expense report tied to user's own paid policy expense chat", () => {
it("it is an open expense report tied to user's own policy expense chat", () => {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}103`, {
reportID: '103',
chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT,
Expand Down Expand Up @@ -542,6 +547,33 @@ describe('ReportUtils', () => {
expect(moneyRequestOptions.length).toBe(1);
expect(moneyRequestOptions.includes(CONST.IOU.TYPE.REQUEST)).toBe(true);
});

it("it is a submitted expense report in user's own policyExpenseChat and the policy has Instant Submit frequency", () => {
const paidPolicy = {
id: 'ef72dfeb',
type: CONST.POLICY.TYPE.TEAM,
autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT,
};
Promise.all([
Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${paidPolicy.id}`, paidPolicy),
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}101`, {
reportID: '101',
chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT,
isOwnPolicyExpenseChat: true,
}),
]).then(() => {
const report = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.EXPENSE,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
parentReportID: '101',
policyID: paidPolicy.id,
};
const moneyRequestOptions = ReportUtils.getMoneyRequestOptions(report, paidPolicy, [currentUserAccountID, participantsAccountIDs[0]]);
expect(moneyRequestOptions.length).toBe(1);
});
});
});

describe('return multiple money request option if', () => {
Expand Down

0 comments on commit 9bbe923

Please sign in to comment.