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

[$250] Pay with business BA - System message is "paid x", while thread header is "paid x elsewhere" #48560

Closed
6 tasks done
IuliiaHerets opened this issue Sep 4, 2024 · 48 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Sep 4, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 9.0.29-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4921007
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  • Admin and member are in the same workspace.
  • Workspace has bank account set up.
  • Member has personal bank account.
  1. Go to staging.new.expensify.com
  2. [Member] Go to workspace chat and submit an expense.
  3. [Admin] Approve the expense.
  4. [Admin] Go offline (because there is an error when paying with business bank account).
  5. [Admin] Click Pay with business bank account.
  6. [Admin] Right click on the paid system message > Reply in thread.

Expected Result:

The thread header for paid system message should be the same as the system message.

Actual Result:

The system message is "paid x", while the thread header shows "paid x elsewhere".

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6592705_1725443475664.20240904_174052.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021831454888955451181
  • Upwork Job ID: 1831454888955451181
  • Last Price Increase: 2024-10-09
Issue OwnerCurrent Issue Owner: @
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

Copy link

melvin-bot bot commented Sep 4, 2024

Triggered auto assignment to @AndrewGable (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@melvin-bot melvin-bot bot added the Daily KSv2 label Sep 4, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Sep 4, 2024
Copy link
Contributor

github-actions bot commented Sep 4, 2024

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@AndrewGable
Copy link
Contributor

Trying to figure out which PR introduced this since it doesn't happen on prod

@AndrewGable
Copy link
Contributor

@bondydaa / @MariaHCD - I am curious if you think this revert revert PR could possibly be related? #48421

@AndrewGable
Copy link
Contributor

After discussion in Slack we think this is probably a regression from the linked PR, we are going to work with @bondydaa @brandonhenry @allroundexperts to fix this regression!

@AndrewGable AndrewGable added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Sep 4, 2024
@bondydaa bondydaa added the External Added to denote the issue can be worked on by a contributor label Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021831454888955451181

@melvin-bot melvin-bot bot changed the title Pay with business BA - System message is "paid x", while thread header is "paid x elsewhere" [$250] Pay with business BA - System message is "paid x", while thread header is "paid x elsewhere" Sep 4, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

Current assignee @allroundexperts is eligible for the External assigner, not assigning anyone new.

@bondydaa
Copy link
Contributor

bondydaa commented Sep 4, 2024

also throwing the external label on here, ideally @brandonhenry can fix but if not no need to block for that either.

@jaydamani
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Thread title and orignal message are incosistent for payment action message

What is the root cause of that problem?

Both strings come from different functions that are incosistent.

The thread title comes from message field in reportAction which is added as part of optimistic data for payMoneyRequest. The optimistic data gets this message from getIOUReportActionMessage. The getIOUReportActionMessage function returns Paid x elsewhere by default.

The message content comes from getIOUReportActionDisplayMessage. The getIOUReportActionDisplayMessage returns Paid x by default.

This causes inconsistency between thread title and the message content.

getIOUReportActionMessage:

App/src/libs/ReportUtils.ts

Lines 4467 to 4475 in 8a35256

switch (paymentType) {
case CONST.IOU.PAYMENT_TYPE.VBBA:
case CONST.IOU.PAYMENT_TYPE.EXPENSIFY:
paymentMethodMessage = ' with Expensify';
break;
default:
paymentMethodMessage = ` elsewhere`;
break;
}

getIOUReportActionDisplayMessage:

App/src/libs/ReportUtils.ts

Lines 6933 to 6944 in 8a35256

switch (originalMessage.paymentType) {
case CONST.IOU.PAYMENT_TYPE.ELSEWHERE:
translationKey = hasMissingInvoiceBankAccount(IOUReportID ?? '-1') ? 'iou.payerSettledWithMissingBankAccount' : 'iou.paidElsewhereWithAmount';
break;
case CONST.IOU.PAYMENT_TYPE.EXPENSIFY:
case CONST.IOU.PAYMENT_TYPE.VBBA:
translationKey = 'iou.paidWithExpensifyWithAmount';
break;
default:
translationKey = 'iou.payerPaidAmount';
break;
}

What changes do you think we should make in order to solve the problem?

Paid x feels more right so, we can update getIOUReportActionMessage to return Paid x by default. This function also has some strings hardcoded in english so maybe we should also localize this.

What alternative solutions did you explore? (Optional)

Option 1

Update getIOUReportActionDisplayMessage to return Paid x elsewhere.

Option 2

getIOUReportActionMessage is only used for building optmistic reports and for showing last message so maybe we can replace it with getIOUReportActionDisplayMessage.

@bondydaa
Copy link
Contributor

bondydaa commented Sep 6, 2024

i'll be OOO next week so if we get proposals ready please ask for another engineer to review

@isabelastisser
Copy link
Contributor

isabelastisser commented Sep 6, 2024

@allroundexperts, can you please review the proposal above? Thanks!

@melvin-bot melvin-bot bot added the Overdue label Sep 23, 2024
@isabelastisser
Copy link
Contributor

@allroundexperts, can you please provide an update? Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Sep 23, 2024
@allroundexperts
Copy link
Contributor

Looking at this now.

@allroundexperts
Copy link
Contributor

@jaydamani can you please update your proposal to match the expected behaviour we decided above?

Copy link

melvin-bot bot commented Sep 25, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@jaydamani
Copy link
Contributor

jaydamani commented Sep 26, 2024

Trying to replicate this again and I am not able to get the option for "Pay with business bank account". If I try using Pay with Expensify then the text is for thread title and system message.
image

@melvin-bot melvin-bot bot added the Overdue label Sep 27, 2024
Copy link

melvin-bot bot commented Sep 30, 2024

@allroundexperts, @isabelastisser Eep! 4 days overdue now. Issues have feelings too...

@allroundexperts
Copy link
Contributor

Trying to replicate this again and I am not able to get the option for "Pay with business bank account"

@jaydamani Have you tried toggling the following line:
https://github.com/rojiphil/app/blob/20cc41438132b17f6345a18f3afbff5b9fb04d1a/src/components/AddPaymentMethodMenu.tsx#L71

@melvin-bot melvin-bot bot removed the Overdue label Sep 30, 2024
Copy link

melvin-bot bot commented Oct 2, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Oct 2, 2024

@allroundexperts @isabelastisser this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

@jaydamani
Copy link
Contributor

@allroundexperts After further investigating, the "Pay with Business account" option was added by #48421 (originally #37174) which was reverted #48639. So, now we only have "Pay with Expensify" and "Pay Elsewhere" for expenses.

In main, the business account option is only for invoices and even for that the payment type is elsewhere so this issue is technically no longer be reproduced. However, it can happen again if new payment types are added but not updated here and here.
So, should we close this as it does not really affect anything or update the default messages to be consistent?

Payment methods in 9.0.29-0:

const paymentMethods = {
[CONST.IOU.PAYMENT_TYPE.VBBA]: {
text: translate('iou.settleExpensify', {formattedAmount}),
icon: Expensicons.Wallet,
value: CONST.IOU.PAYMENT_TYPE.VBBA,
},
[CONST.PAYMENT_METHODS.PERSONAL_BANK_ACCOUNT]: {
text: translate('iou.settlePersonalBank', {formattedAmount}),
icon: Expensicons.Bank,
value: CONST.PAYMENT_METHODS.PERSONAL_BANK_ACCOUNT,
},
[CONST.PAYMENT_METHODS.BUSINESS_BANK_ACCOUNT]: {
text: translate('iou.settleBusinessBank', {formattedAmount}),
icon: Expensicons.Bank,
value: CONST.PAYMENT_METHODS.BUSINESS_BANK_ACCOUNT,
},
[CONST.PAYMENT_METHODS.DEBIT_CARD]: {
text: translate('iou.settleDebitCard', {formattedAmount}),
icon: Expensicons.CreditCard,
value: CONST.PAYMENT_METHODS.DEBIT_CARD,
},
[CONST.IOU.PAYMENT_TYPE.ELSEWHERE]: {
text: translate('iou.payElsewhere', {formattedAmount}),
icon: Expensicons.Cash,
value: CONST.IOU.PAYMENT_TYPE.ELSEWHERE,
},
};

Payment methods in main:

const paymentMethods = {
[CONST.IOU.PAYMENT_TYPE.EXPENSIFY]: {
text: translate('iou.settleExpensify', {formattedAmount}),
icon: Expensicons.Wallet,
value: CONST.IOU.PAYMENT_TYPE.EXPENSIFY,
},
[CONST.IOU.PAYMENT_TYPE.VBBA]: {
text: translate('iou.settleExpensify', {formattedAmount}),
icon: Expensicons.Wallet,
value: CONST.IOU.PAYMENT_TYPE.VBBA,
},
[CONST.IOU.PAYMENT_TYPE.ELSEWHERE]: {
text: translate('iou.payElsewhere', {formattedAmount}),
icon: Expensicons.Cash,
value: CONST.IOU.PAYMENT_TYPE.ELSEWHERE,
},
};

@melvin-bot melvin-bot bot added the Overdue label Oct 3, 2024
@isabelastisser
Copy link
Contributor

@allroundexperts, can you please provide an update? Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Oct 3, 2024
@allroundexperts
Copy link
Contributor

Since this is no longer reproducible, I think we can close this issue.

@trjExpensify
Copy link
Contributor

Trying to replicate this again and I am not able to get the option for "Pay with business bank account". If I try using Pay with Expensify then the text is for thread title and system message.

I'm not following this. Can you confirm these steps and include a video to confirm it's not reproducible:

  1. Create a workspace
  2. Enable Workflows > Make or Track Payments > Connect bank account
  3. Submit an expense to the workspace
  4. Click "Pay with Expensify" to pay it via the connected bank account
  5. Click reply on the "paid $x.xx" system message to open a thread
  6. What does the thread header say?

Copy link

melvin-bot bot commented Oct 9, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Oct 9, 2024
@jaydamani
Copy link
Contributor

@trjExpensify
It shows "Paid x with expensify" for both header and message which seems correct.

1.New.Expensify.-.Google.Chrome.2024-10-10.18-39-25.mp4

Also unrelated to this issue but, while checking I noticed that an expense is greyed out even when no changes were done after going offline Here are some rough steps for it:

  1. Follow steps from original issue
  2. Turn off offline mode
  3. Clear the error
  4. Check that payment button is visible
  5. Turn on offline mode
  6. Validate that the expense is not greyed out

Can you confirm if this is something that needs to be changed?

@isabelastisser
Copy link
Contributor

@allroundexperts can you follow up on the comments above? Thanks!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 10, 2024
@isabelastisser
Copy link
Contributor

@allroundexperts, please provide an update! I will DM you for visibility, thanks!

@melvin-bot melvin-bot bot removed the Overdue label Oct 15, 2024
@trjExpensify
Copy link
Contributor

It shows "Paid x with expensify" for both header and message which seems correct.

Great, so this bug is resolved and we can close this.

@github-project-automation github-project-automation bot moved this from Release 1: Spring 2024 (May) to Done in [#whatsnext] #wave-collect Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
No open projects
Status: Done
Development

No branches or pull requests

10 participants