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.56k stars 2.9k forks source link

[$250] [LOW] [Splits] IOU - Header of IOU transaction thread is not grayed out when created offline #39352

Closed izarutskaya closed 6 months ago

izarutskaya commented 7 months 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.58-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4466053 Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go offline.
  3. Request money from a user with no previous chat.
  4. Go to 1:1 DM, IOU report and transaction thread.
  5. Note the header of the three reports.

Expected Result:

The header of 1:1 DM, IOU report and transaction thread should be all grayed out.

Actual Result:

The header of 1:1 DM and IOU report is grayed out, but not for transaction thread.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6434082_1711970322335!bandicam_2024-04-01_19-15-38-587

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~017d9117dc1c8dc5c7
  • Upwork Job ID: 1777456926421372928
  • Last Price Increase: 2024-04-15
  • Automatic offers:
    • hoangzinh | Reviewer | 0
    • bernhardoj | Contributor | 0
melvin-bot[bot] commented 7 months ago

Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

izarutskaya commented 7 months ago

We think this issue might be related to the #collect project.

bernhardoj commented 7 months ago

Proposal

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

The transaction thread header is not grayed out when created while offline which is inconsistent with the chat and IOU/expense report header.

What is the root cause of that problem?

When we create a new request while offline, we will optimistically create the chat and IOU report (if new) and add a pending fields. https://github.com/Expensify/App/blob/c848cb6bf29b600eecb8b338222fffe8a36d0329/src/libs/actions/IOU.ts#L516 https://github.com/Expensify/App/blob/c848cb6bf29b600eecb8b338222fffe8a36d0329/src/libs/actions/IOU.ts#L529-L531

The pending fields will be used to grayed out the whole report screen, including the header.

However, we never add a pending fields for the optimistic transaction thread. https://github.com/Expensify/App/blob/c848cb6bf29b600eecb8b338222fffe8a36d0329/src/libs/actions/IOU.ts#L573-L575

Thus, both the header and the composer are not grayed out.

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

Add a pending fields the same as chat/IOU report.

value: {
    ...transactionThreadReport,
    pendingFields: {createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD},
},

Then, clear it when fails just like we did for chat/IOU report too. (we already clear it when success)

Couple of notes:

  1. We need to apply the same fix for track expense too (the above code already covers send, request, and split bill).
  2. The code to add the pending fields is currently lives in IOU.buildOnyxDataForMoneyRequest. I think it's better to add it in ReportUtils.buildOptimisticChatReport though, but since it's used in many places and the current pattern is to add it in IOU.buildOnyxDataForMoneyRequest, I do the same.
MitchExpensify commented 7 months ago

Seeing as this is IOU related I'm assigning to vip-split

MitchExpensify commented 7 months ago

Let's fix!

melvin-bot[bot] commented 7 months ago

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

melvin-bot[bot] commented 7 months ago

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

MitchExpensify commented 7 months ago

Waiting on proposals

hoangzinh commented 7 months ago

@bernhardoj proposal looks good to me

πŸŽ€πŸ‘€πŸŽ€ C+ reviewed

melvin-bot[bot] commented 7 months ago

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

melvin-bot[bot] commented 7 months ago

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

melvin-bot[bot] commented 7 months ago

@hoangzinh @MitchExpensify @srikarparsi 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!

srikarparsi commented 7 months ago

Assigning @bernhardoj

melvin-bot[bot] commented 7 months ago

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

Offer link Upwork job

melvin-bot[bot] commented 7 months ago

πŸ“£ @bernhardoj πŸŽ‰ 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 πŸ“–

bernhardoj commented 7 months ago

PR is ready

cc: @hoangzinh

melvin-bot[bot] commented 6 months ago

This issue has not been updated in over 15 days. @hoangzinh, @MitchExpensify, @srikarparsi, @bernhardoj eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

hoangzinh commented 6 months ago

oops the PR was deployed to Prod 2 weeks ago https://github.com/Expensify/App/pull/40270#issuecomment-2073664238. @MitchExpensify can you help to process payment here? Thanks

MitchExpensify commented 6 months ago

Payment summary:

C $250 @bernhardoj (Upwork) C+ $250 @hoangzinh (Upwork)

MitchExpensify commented 6 months ago

Done! Thanks