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.

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

[CP Stag] Add GBR to pending wallet reports & update perview for 1:1 reports #30090

Merged
merged 14 commits into from
Oct 23, 2023
Merged
6 changes: 5 additions & 1 deletion src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,11 @@ function getLastMessageTextForReport(report) {
reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE &&
ReportActionUtils.isMoneyRequestAction(reportAction),
);
lastMessageTextFromReport = ReportUtils.getReportPreviewMessage(iouReport, lastIOUMoneyReport, true);
// Whether parent report is a 1:1 report
b4s36t4 marked this conversation as resolved.
Show resolved Hide resolved
const isChatReport = ReportUtils.isChatReport(report);
techievivek marked this conversation as resolved.
Show resolved Hide resolved
b4s36t4 marked this conversation as resolved.
Show resolved Hide resolved
lastMessageTextFromReport = ReportUtils.getReportPreviewMessage(iouReport, lastIOUMoneyReport, true, isChatReport);
} else if (ReportActionUtils.isReimbursementQueuedAction(lastReportAction)) {
lastMessageTextFromReport = ReportUtils.getReimbursementQueuedActionMessage(lastReportAction, report);
} else if (ReportActionUtils.isDeletedParentAction(lastReportAction) && ReportUtils.isChatReport(report)) {
lastMessageTextFromReport = ReportUtils.getDeletedParentActionMessageForChatReport(lastReportAction);
} else if (ReportActionUtils.isModifiedExpenseAction(lastReportAction)) {
Expand Down
5 changes: 5 additions & 0 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ function isWhisperAction(reportAction: OnyxEntry<ReportAction>): boolean {
return (reportAction?.whisperedToAccountIDs ?? []).length > 0;
}

function isReimbursementQueuedAction(reportAction: OnyxEntry<ReportAction>) {
return reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.REIMBURSEMENTQUEUED;
}

/**
* Returns whether the comment is a thread parent message/the first message in a thread
*/
Expand Down Expand Up @@ -636,6 +640,7 @@ export {
isThreadParentMessage,
isTransactionThread,
isWhisperAction,
isReimbursementQueuedAction,
shouldReportActionBeVisible,
shouldReportActionBeVisibleAsLastAction,
};
35 changes: 30 additions & 5 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,12 @@ function isSettled(reportID) {
return false;
}
const report = allReports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] || {};
if ((typeof report === 'object' && Object.keys(report).length === 0) || report.isWaitingOnBankAccount) {
if (typeof report === 'object' && Object.keys(report).length === 0) {
techievivek marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

return report.statusNum === CONST.REPORT.STATUS.REIMBURSED;
// APPROVED status is for Personal policies
techievivek marked this conversation as resolved.
Show resolved Hide resolved
return report.statusNum === CONST.REPORT.STATUS.REIMBURSED || report.statusNum === CONST.REPORT.STATUS.APPROVED;
}

/**
Expand Down Expand Up @@ -1240,6 +1241,26 @@ function getDeletedParentActionMessageForChatReport(reportAction) {
return deletedMessageText;
}

/**
* Returns the preview message for `REIMBURSEMENTQUEUED` action
*
* @param {Object} reportAction
* @param {Object} report
* @returns {String}
*/

b4s36t4 marked this conversation as resolved.
Show resolved Hide resolved
function getReimbursementQueuedActionMessage(reportAction, report) {
const submitterDisplayName = getDisplayNameForParticipant(report.ownerAccountID, true);
let messageKey;
if (lodashGet(reportAction, 'originalMessage.paymentType', '') === CONST.IOU.PAYMENT_TYPE.EXPENSIFY) {
messageKey = 'iou.waitingOnEnabledWallet';
} else {
messageKey = 'iou.waitingOnBankAccount';
}

return Localize.translateLocal(messageKey, {submitterDisplayName});
}

/**
* Returns the last visible message for a given report after considering the given optimistic actions
*
Expand Down Expand Up @@ -1295,7 +1316,8 @@ function isWaitingForIOUActionFromCurrentUser(report) {
}

// Money request waiting for current user to add their credit bank account
if (report.hasOutstandingIOU && report.ownerAccountID === currentUserAccountID && report.isWaitingOnBankAccount) {
// hasOutstandingIOU will be false if the user paid, but isWaitingOnBankAccount will be true if user don't have a wallet or bank account setup
if ((report.hasOutstandingIOU || report.isWaitingOnBankAccount) && report.ownerAccountID === currentUserAccountID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition is still wrong. It causes regression above ^

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 don't think so, I removed it and tested it still shows.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not seeing GBR after reverting this condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@situchan I think that dot is intentional, it's not showing in main (tested) because the condition is wrong (the RCA for GBR bug). Maybe you're trying in a new account.

// Money request waiting for current user to add their credit bank account

This feels like intentional.

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 am not seeing GBR after reverting this condition

Means report.hasOutstandingIOU && report.ownerAccountID === currentUserAccountID this would be the conditon right?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kapture.2023-10-23.at.18.21.43.mp4

this is OP for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

you haven't fully reverted code.
report.isWaitingOnBankAccount is missing

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'm fixing it as per the comment in slack. Only show GBR when payee Paid it (no outstanding)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@situchan Fixed, Can you please check?

return true;
}

Expand Down Expand Up @@ -1615,9 +1637,10 @@ function getTransactionReportName(reportAction) {
* @param {Object} report
* @param {Object} [reportAction={}] This can be either a report preview action or the IOU action
* @param {Boolean} [shouldConsiderReceiptBeingScanned=false]
* @param {Boolean} isPreviewMessageForParentChatReport If the report is from 1:1 chat
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Also involves workspace chat

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

* @returns {String}
*/
function getReportPreviewMessage(report, reportAction = {}, shouldConsiderReceiptBeingScanned = false) {
function getReportPreviewMessage(report, reportAction = {}, shouldConsiderReceiptBeingScanned = false, isPreviewMessageForParentChatReport = false) {
const reportActionMessage = lodashGet(reportAction, 'message[0].html', '');

if (_.isEmpty(report) || !report.reportID) {
Expand Down Expand Up @@ -1656,7 +1679,8 @@ function getReportPreviewMessage(report, reportAction = {}, shouldConsiderReceip
}
}

if (isSettled(report.reportID)) {
// Show Paid preview message if it's settled or if the amount is paid & stuck at receivers end for only chat reports.
if (isSettled(report.reportID) || (report.isWaitingOnBankAccount && !isPreviewMessageForParentChatReport)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@situchan @techievivek review this please. Updated to resolve issue with isSettled function update.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it not show as settled for the iou report now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a or condition so it would show paid for iou as well.

Copy link
Contributor

@techievivek techievivek Oct 23, 2023

Choose a reason for hiding this comment

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

That's what I am saying then how would it show system message in case of iou report if isWaitingOnBankAccount is set on ioureport as isSettled(report.reportID) would always return as true.

// A settled report preview message can come in three formats "paid ... elsewhere" or "paid ... with Expensify"
let translatePhraseKey = 'iou.paidElsewhereWithAmount';
if (
Expand Down Expand Up @@ -4122,4 +4146,5 @@ export {
isReportDraft,
shouldUseFullTitleToDisplay,
parseReportRouteParams,
getReimbursementQueuedActionMessage,
};
6 changes: 3 additions & 3 deletions tests/unit/ReportUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,16 +271,16 @@ describe('ReportUtils', () => {
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
});
});
it('returns false when the report has no oustanding IOU but is waiting for a bank account and the logged user is the report owner', () => {
it('returns true when the report has no outstanding IOU but is waiting for a bank account and the logged user is the report owner', () => {
const report = {
...LHNTestUtils.getFakeReport(),
hasOutstandingIOU: false,
ownerAccountID: currentUserAccountID,
isWaitingOnBankAccount: true,
};
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false);
expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true);
});
it('returns true when the report has oustanding IOU and is waiting for a bank account and the logged user is the report owner', () => {
it('returns true when the report has outstanding IOU and is waiting for a bank account and the logged user is the report owner', () => {
const report = {
...LHNTestUtils.getFakeReport(),
hasOutstandingIOU: true,
Expand Down
Loading