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

[LOW] [Splits] [$500] Web - IOU is greyed out, error message appears in chat and console #32076

Closed
1 of 6 tasks
lanitochka17 opened this issue Nov 27, 2023 · 76 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Internal Requires API changes or must be handled by Expensify staff Monthly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Nov 27, 2023

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.4.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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #31102

Action Performed:

  1. Navigate to https://staging.new.expensify.com/
  2. Log in with a new account
  3. Start a chat using the FAB
  4. Input an account who isn't registered yet
  5. Send a message
  6. Upload a picture using the composers "+" button
  7. Request money (manual) using the FAB
  8. Input an email (can be the one from step 4 or a new one)
  9. Finish the IOU

Expected Result:

I should be able to make the IOU without error messages or console errors

Actual Result:

I get an error message and the IOU is greyed out. Two errors appear in the console as soon as the error message appears in the chat
Unexpected error creating this chat, please try again later. There is a previously existing chat between these users

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6292386_1701121325770.bandicam_2023-11-27_21-33-01-176.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ae5fe97cb4eccf95
  • Upwork Job ID: 1729285304604618752
  • Last Price Increase: 2023-11-27
Issue OwnerCurrent Issue Owner: @iwiznia
@lanitochka17 lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 27, 2023
@melvin-bot melvin-bot bot changed the title IOU - IOU is greyed out, errror message appears in chat and console [$500] IOU - IOU is greyed out, errror message appears in chat and console Nov 27, 2023
Copy link

melvin-bot bot commented Nov 27, 2023

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

Copy link

melvin-bot bot commented Nov 27, 2023

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

Copy link

melvin-bot bot commented Nov 27, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 27, 2023
Copy link

melvin-bot bot commented Nov 27, 2023

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

@tienifr
Copy link
Contributor

tienifr commented Nov 28, 2023

Proposal

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

When creating IOU with users we already have chat with, get error "Unexpected error creating this chat, please try again later. There is a previously existing chat between these users"

What is the root cause of that problem?

We're not handling the preexisting chat case properly for the IOU case. The proper way to handle it is to redirect to the preexisting chat and clear the unnecessary data here, which already works fine in case of New chat created that has preexisting chat.

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

  1. In RequestMoney command, the back-end should return preexistingReportID (similar to OpenReport) if the report creation failure is due to existing report between the 2 users. It should also handle so that the money request is created properly on that preexisting report
  2. Front-end will use it to redirect users to the correct chat, and clean up the optimistic report, report actions and related data of the duplicated chat, similar to what's already done for the new chat case here

What alternative solutions did you explore? (Optional)

Other related flow like split bill might have the same issue, we'll need to double check and fix for them as well.

@narefyev91
Copy link
Contributor

Proposal from @tienifr looks good to me #32076 (comment)

  1. We need to add internal BE engineer to work on changes
  2. We should assign @tienifr to make draft PR for FE side
  3. Combine with BE change and test draft PR to be sure that issue will be resolved
    🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Nov 28, 2023

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

@joekaufmanexpensify
Copy link
Contributor

I'm thinking we should hold off on proceeding with this for right now. We are having some larger internal discussions about this hidden feature here that could impact whether we need to actually fix this in this issue.

@iwiznia
Copy link
Contributor

iwiznia commented Nov 28, 2023

ok, putting this on HOLD then. @joekaufmanexpensify you can let us know when we should unhold

@iwiznia iwiznia changed the title [$500] IOU - IOU is greyed out, errror message appears in chat and console [HOLD] [$500] IOU - IOU is greyed out, errror message appears in chat and console Nov 28, 2023
@tienifr
Copy link
Contributor

tienifr commented Nov 29, 2023

I'm thinking we should hold off on proceeding with this for right now. We are having some larger internal discussions about this hidden feature here that could impact whether we need to actually fix this in this issue.

@joekaufmanexpensify @iwiznia just FYI this doesn't just happen in the Hidden case. It will also happen if user A initiates chat with user B on 1 device and also creates a money request to user B on another device in offline mode, then go online. It will face the same issue.

@joekaufmanexpensify
Copy link
Contributor

Hm, interesting. I'm not sure if creating a chat with one user while online on one device, and then simultaneously attempting a money request with the same user on another device while offline is something a legitimate user would do.

If the end result of the above discussion is that the hidden case will be fixed by other changes we are making, I'd probably rather wait and see if any actual customers ever experience this other case before doing anything.

@joekaufmanexpensify
Copy link
Contributor

Making this a weekly for now, since the discussion is not yet resolved.

@joekaufmanexpensify joekaufmanexpensify added Weekly KSv2 and removed Daily KSv2 labels Nov 30, 2023
@joekaufmanexpensify
Copy link
Contributor

Discussion is still ongoing

@joekaufmanexpensify
Copy link
Contributor

Same. Looks like we were timing some queries to inform the direction we go in.

@joekaufmanexpensify
Copy link
Contributor

Bumped discussion to determine whether anything is needed here.

@joekaufmanexpensify
Copy link
Contributor

@iwiznia I discussed this with Puneet more today, and the result of that discussion was that we should proceed with fixing the bug in this issue.

We are already handling part of this bug here so it works if the actions are all in one session. Meaning, if someone starts a chat with someone they don't know, and then separately creates an IOU with the same person in the same session, we will correctly consolidate the message and IOU into one chat.

However, we still won't handle the piece related to multiple sessions correctly. Meaning if someone did this, and then signed out and back in, we'd try and put the IOU onto a new chat and it'd error out (as shown above in OP). Puneet mentioned we already handle this case for chats, and we can just re-use that same backend logic for IOUs here.

@joekaufmanexpensify joekaufmanexpensify changed the title [HOLD] [$500] IOU - IOU is greyed out, errror message appears in chat and console [$500] IOU - IOU is greyed out, errror message appears in chat and console Dec 18, 2023
@joekaufmanexpensify
Copy link
Contributor

LMK if you agree! Taking this off of hold for now

@melvin-bot melvin-bot bot removed the Overdue label Feb 23, 2024
@iwiznia
Copy link
Contributor

iwiznia commented Feb 23, 2024

No update

@melvin-bot melvin-bot bot added the Overdue label Mar 4, 2024
@iwiznia
Copy link
Contributor

iwiznia commented Mar 4, 2024

No update

@melvin-bot melvin-bot bot removed the Overdue label Mar 4, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 12, 2024
@iwiznia
Copy link
Contributor

iwiznia commented Mar 13, 2024

No update

@melvin-bot melvin-bot bot removed the Overdue label Mar 13, 2024
@arielgreen arielgreen changed the title [$500] Web - IOU is greyed out, error message appears in chat and console [LOW] [Splits] [$500] Web - IOU is greyed out, error message appears in chat and console Mar 22, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 22, 2024
@iwiznia
Copy link
Contributor

iwiznia commented Mar 22, 2024

No update

@melvin-bot melvin-bot bot removed the Overdue label Mar 22, 2024
@iwiznia iwiznia added Monthly KSv2 and removed Weekly KSv2 labels Mar 22, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 23, 2024
@iwiznia
Copy link
Contributor

iwiznia commented Apr 24, 2024

No update

@melvin-bot melvin-bot bot removed the Overdue label Apr 24, 2024
@melvin-bot melvin-bot bot added the Overdue label May 27, 2024
@iwiznia
Copy link
Contributor

iwiznia commented May 27, 2024

No update

@melvin-bot melvin-bot bot removed the Overdue label May 27, 2024
@melvin-bot melvin-bot bot added the Overdue label Jun 28, 2024
@iwiznia
Copy link
Contributor

iwiznia commented Jul 2, 2024

No update

@melvin-bot melvin-bot bot removed the Overdue label Jul 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Aug 2, 2024
@parasharrajat
Copy link
Member

parasharrajat commented Aug 4, 2024

@joekaufmanexpensify Let's push this forward to the finish line. Is there a reason to hold this issue? Please change the label to weekly.

@joekaufmanexpensify
Copy link
Contributor

Hm, I don't think it is explicitly on hold. It's a question of internal priorities, given the next step here is for @iwiznia to put up the backend fix discussed here. Defer to him on when he thinks he'll be able to prioritize it. Happy to move it to weekly if that will be soon.

@parasharrajat
Copy link
Member

parasharrajat commented Aug 5, 2024

It's been almost 8 months. Let's get this prioritized now. What do you think @iwiznia?

@iwiznia
Copy link
Contributor

iwiznia commented Aug 12, 2024

The changes needed are quite large and involve lots of api commands... given the low severity of the issues and the fact that reproduction steps are quite complex, I think we might be better off closing it.

@melvin-bot melvin-bot bot removed the Overdue label Aug 12, 2024
@iwiznia iwiznia closed this as completed Aug 14, 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. Internal Requires API changes or must be handled by Expensify staff Monthly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants