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

Consistent system paid message #36298

Merged
merged 4 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
8 changes: 6 additions & 2 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2232,6 +2232,7 @@ function getReportPreviewMessage(
isPreviewMessageForParentChatReport = false,
policy: OnyxEntry<Policy> = null,
isForListPreview = false,
shouldHidePayer = false,
): string {
const reportActionMessage = reportAction?.message?.[0].html ?? '';

Expand Down Expand Up @@ -2296,15 +2297,18 @@ function getReportPreviewMessage(
if (isSettled(report.reportID) || (report.isWaitingOnBankAccount && isPreviewMessageForParentChatReport)) {
// A settled report preview message can come in three formats "paid ... elsewhere" or "paid ... with Expensify"
let translatePhraseKey: TranslationPaths = 'iou.paidElsewhereWithAmount';
if (

Copy link
Contributor

Choose a reason for hiding this comment

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

remove this empty line

Copy link
Contributor

Choose a reason for hiding this comment

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

if (isPreviewMessageForParentChatReport) {
translatePhraseKey = 'iou.payerPaidAmount';
} else if (
[CONST.IOU.PAYMENT_TYPE.VBBA, CONST.IOU.PAYMENT_TYPE.EXPENSIFY].some((paymentType) => paymentType === originalMessage?.paymentType) ||
!!reportActionMessage.match(/ (with Expensify|using Expensify)$/) ||
report.isWaitingOnBankAccount
) {
translatePhraseKey = 'iou.paidWithExpensifyWithAmount';
}

let actualPayerName = report.managerID === currentUserAccountID ? '' : getDisplayNameForParticipant(report.managerID, true);
let actualPayerName = report.managerID === currentUserAccountID || shouldHidePayer ? '' : getDisplayNameForParticipant(report.managerID, true);
actualPayerName = actualPayerName && isForListPreview && !isPreviewMessageForParentChatReport ? `${actualPayerName}:` : actualPayerName;
const payerDisplayName = isPreviewMessageForParentChatReport ? payerName : actualPayerName;

Expand Down
10 changes: 9 additions & 1 deletion src/pages/home/report/ContextMenu/ContextMenuActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,15 @@ const ContextMenuActions: ContextMenuAction[] = [
const modifyExpenseMessage = ModifiedExpenseMessage.getForReportAction(reportID, reportAction);
Clipboard.setString(modifyExpenseMessage);
} else if (ReportActionsUtils.isMoneyRequestAction(reportAction)) {
const displayMessage = ReportUtils.getIOUReportActionDisplayMessage(reportAction);
const displayMessage = ReportUtils.getReportPreviewMessage(
Copy link
Contributor

Choose a reason for hiding this comment

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

I see there are many differences between getIOUReportActionDisplayMessage and getReportPreviewMessage functions. Using getReportPreviewMessage will change other behaviors (not relevant to this issue)
This is an example

Screen.Recording.2024-02-15.at.11.10.56.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DylanDylann For this case I think we should ask internal team to confirm the expected for this case again, because it's displayed Scan in progress but the message is copied now is split $0.00

Copy link
Contributor

Choose a reason for hiding this comment

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

@dukenv0307 Also remove getIOUReportActionDisplayMessage function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DylanDylann I updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

What happened here? getReportPreviewMessage retrieves the message for the report preview. We've been displaying the wrong message for IOU actions for a while, but the messages displayed by getIOUReportActionDisplayMessage are the right ones. If that causes other bugs, we need to address them, but not revert back to the wrong messages. Was there a discussion about this on Slack?

Copy link
Contributor Author

@dukenv0307 dukenv0307 Feb 19, 2024

Choose a reason for hiding this comment

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

What's wrong here? I saw that all cases in getIOUReportActionDisplayMessage already have in getReportPreviewMessage.

ReportUtils.getReport(ReportUtils.getOriginalReportID(reportID, reportAction)),
reportAction,
false,
false,
null,
false,
true,
);
Clipboard.setString(displayMessage);
} else if (ReportActionsUtils.isCreatedTaskReportAction(reportAction)) {
const taskPreviewMessage = TaskUtils.getTaskCreatedMessage(reportAction);
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionItemMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function ReportActionItemMessage({action, displayAsGroup, reportID, style, isHid
const originalMessage = action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? action.originalMessage : null;
const iouReportID = originalMessage?.IOUReportID;
if (iouReportID) {
iouMessage = ReportUtils.getReportPreviewMessage(ReportUtils.getReport(iouReportID), action);
iouMessage = ReportUtils.getReportPreviewMessage(ReportUtils.getReport(iouReportID), action, false, false, null, false, true);
}
}

Expand Down
Loading