Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.1k stars 2.6k forks source link

[$250] Send invoices - Date notification not copied when invoice date is changed #43571

Open lanitochka17 opened 2 weeks ago

lanitochka17 commented 2 weeks ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.4.82-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): abebemiherat@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

  1. Navigate to FAB > Send invoice
  2. Add amount, select user and workspace
  3. Add merchant and send the invoice
  4. Go to invoice details and change the date
  5. Notice the notification appears. Attempt to copy it, but it doesn't work

Expected Result:

The date notification should be copied when the invoice date is changed

Actual Result:

After sending the invoice, changing the date triggers a notification. However, attempting to copy the notification fails

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/8b92eb63-e714-47da-a547-21bc0303d970

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0111e39bcf78303fbe
  • Upwork Job ID: 1803213787847433243
  • Last Price Increase: 2024-06-25
  • Automatic offers:
    • c3024 | Contributor | 102904588
Issue OwnerCurrent Issue Owner: @getusha
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @strepanier03 (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.

lanitochka17 commented 2 weeks ago

@strepanier03 FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

melvin-bot[bot] commented 2 weeks ago

@strepanier03 Huh... This is 4 days overdue. Who can take care of this?

strepanier03 commented 1 week ago

Testing now.

strepanier03 commented 1 week ago

Confirmed reproducible on mac Chrome.

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha (External)

truph01 commented 1 week ago

Proposal

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

Attempting to copy the notification fails

What is the root cause of that problem?

When creating an invoice, in https://github.com/Expensify/App/blob/340f2214469694af11c0a97d1cfd5f7e28c1fa8f/src/libs/actions/IOU.ts#L3560, the params createdIOUReportActionID and reportActionID (IOU action id) are not sent to back-end to use as real report action ids.

So the back-end is creating new IOU action id and send back to front-end. This makes the Invoice has 2 IOUs, so the transactionThreadReportID here is undefined.

Because of that, the originalReportID here is incorrect, it will be the iou report id while it must be the transactionThreadReportID

Next, the reportAction here will be empty (we try to get the modified expense report action from the IOU report which is not found, because originalReportID is wrong above)

So the copying doesn't work because reportAction is empty.

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

When creating an invoice, in https://github.com/Expensify/App/blob/340f2214469694af11c0a97d1cfd5f7e28c1fa8f/src/libs/actions/IOU.ts#L3560, the params createdIOUReportActionID and reportActionID (IOU action id) should be sent to back-end.

Those can be returned from the getSendInvoiceInformation https://github.com/Expensify/App/blob/340f2214469694af11c0a97d1cfd5f7e28c1fa8f/src/libs/actions/IOU.ts#L1852

createdIOUReportActionID: optimisticCreatedActionForIOUReport.reportActionID,
reportActionID: iouAction.reportActionID,

The back-end should be updated to make sure to support those params, and use them as the real action ids. If not mistaken, at the moment it's supporting createdIOUReportActionID but not reportActionID

What alternative solutions did you explore? (Optional)

In https://github.com/Expensify/App/blob/340f2214469694af11c0a97d1cfd5f7e28c1fa8f/src/libs/ReportUtils.ts#L6042, use the reportActions after applying getContinuousReportActionChain so that it will get the correct chain of report actions. This is same method as of ReportScreen

melvin-bot[bot] commented 1 week ago

@strepanier03, @getusha Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] commented 6 days ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

getusha commented 6 days ago

@strepanier03 Reassigning this due to low bandwidth @c3024 will takeover

melvin-bot[bot] commented 5 days ago

@strepanier03 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

c3024 commented 5 days ago

Need a little more time to finalize @MelvinBot

melvin-bot[bot] commented 4 days ago

πŸ“£ @c3024 πŸŽ‰ An offer has been automatically sent to your Upwork account for the Contributor role πŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Keep in mind: Code of Conduct | Contributing πŸ“–

c3024 commented 4 days ago

Not overdue @MelvinBot

c3024 commented 3 days ago

The RCA in @truph01's proposal makes sense to me. The context menu actions are displayed correctly, and the report action is successfully copied to the clipboard when the invoice is sent offline. However, when the backend response is received, there are duplicated report actions with different IDs. Therefore, I agree with the proposed solution to send both createdIOUReportActionID and reportActionID to the backend, accompanied by the necessary backend changes.

πŸŽ€ πŸ‘€ πŸŽ€ C+ Reviewed

melvin-bot[bot] commented 3 days ago

Triggered auto assignment to @stitesExpensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot[bot] commented 17 hours ago

@strepanier03, @stitesExpensify, @c3024 Whoops! This issue is 2 days overdue. Let's get this updated quick!

stitesExpensify commented 14 hours ago

No update, I need to look into this more since there are backend changes proposed