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

[$1000] Money request is shown from the other account instead of current user. #22862

Closed
1 of 6 tasks
kavimuru opened this issue Jul 14, 2023 · 25 comments
Closed
1 of 6 tasks
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

@kavimuru
Copy link

kavimuru commented Jul 14, 2023

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


Action Performed:

  1. Create a new chat.
  2. Click on '+' and request money.

Expected Result:

The IOU should be from the current user.

Actual Result:

The IOU is shown to be from the user we are chatting with.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.40-5
Reproducible in staging?: y
Reproducible in production?: n
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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-07-13.at.8.38.25.PM.mov
Recording.1271.mp4

Expensify/Expensify Issue URL:
Issue reported by: @esh-g
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689261117044569

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019b2ea0a8e06090db
  • Upwork Job ID: 1684323672353202176
  • Last Price Increase: 2023-07-26
@kavimuru kavimuru added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 14, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 14, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 14, 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

@OSBotify
Copy link
Contributor

👋 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.

@melvin-bot
Copy link

melvin-bot bot commented Jul 14, 2023

Triggered auto assignment to @robertjchen (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@francoisl
Copy link
Contributor

Is this actually a bug? The same behavior is happening on production for me. Besides, it says " owes",
showing that the money request was sent to them

Production Staging
image image

@jeet-dhandha
Copy link
Contributor

The message should have gone from the requester's account instead of the payer's account.

@francoisl
Copy link
Contributor

I think that's how it's supposed be rendered, the preview clearly says who owes money to who. I can see how it can be confusing though – let's see of someone who's more familiar with this can chime in.

Anyway, even if it were a bug, the same also happens on production, so I'm going to remove the deploy blocker label for now.

@francoisl francoisl removed the DeployBlockerCash This issue or pull request should block deployment label Jul 14, 2023
@esh-g
Copy link
Contributor

esh-g commented Jul 14, 2023

@francoisl I'm not able to repro on prod.. Maybe it is a glitch in accountID sequence or something that is specific to accounts?
https://github.com/Expensify/App/assets/77237602/fe907f95-35c3-42ae-a97a-75f96e6866ff

@mountiny
Copy link
Contributor

mountiny commented Jul 14, 2023

I think this might be related to when you start chatting with unknown user and you request money, i believe the email used can be incorrect in such case but I dont have clear repro steps yet

@rojiphil
Copy link
Contributor

I can consistently reproduce this on staging. Here is the video.

reportpreview.mp4

@rojiphil
Copy link
Contributor

rojiphil commented Jul 14, 2023

I notice that the backend also works this way because actorAccountID is set to that of the other user and not the current user. If this is a bug, even backend would need to fix.

@robertjchen robertjchen added Weekly KSv2 Daily KSv2 and removed Hourly KSv2 Weekly KSv2 labels Jul 16, 2023
@melvin-bot melvin-bot bot added the Overdue label Jul 18, 2023
@michaelhaxhiu
Copy link
Contributor

@rojiphil sorry but I don't follow the bug from your recording. What is incorrect or inconsistent exactly? Mind screenshotting the point in the video where the problem exists so I can understand?

@melvin-bot melvin-bot bot removed the Overdue label Jul 18, 2023
@michaelhaxhiu
Copy link
Contributor

IMO this bug isn't reproducible reliably. I tried with real users, and then I tried to take @mountiny's advice too:

I think this might be related to when you start chatting with unknown user and you request money, i believe the email used can be incorrect in such case but I dont have clear repro steps yet

But when I use an unknown phone numbers, the DM flickers for a moment and I get re-routed to a chat DM with Concierge. So like it won't really let me reproduce. Here's a quick video:

https://recordit.co/lFXdZNBVQE

If I pause at the split second frame, it looks like the "amount" and "owes" text is all accurate to me 🤔

image

@michaelhaxhiu
Copy link
Contributor

@robertjchen what do you think about this one?

@rojiphil
Copy link
Contributor

rojiphil commented Jul 19, 2023

@rojiphil sorry but I don't follow the bug from your recording. What is incorrect or inconsistent exactly? Mind screenshotting the point in the video where the problem exists so I can understand?

@michaelhaxhiu The problem is not with the amount or who owes it. The problem is that the chat initiator's avatar and name are not displayed in the chat. If you look at my video here - #22862 (comment), chat is sent from T5 to T6. However, the chat is attributed to be sent by T6 (refer to the attached screenshot) even though it was initiated by T5. According to me, this is the problem.

The root cause of this issue is at this line where we are building an optimistic report preview. Here, the actorAccountID is set with report's managerID which could be payer/payee depending on the context. Setting actorAccountID with currentUserAccountID seems to be the right thing to do here but, then, the response of the API request resets the actorAccountID back. Not sure, if this is by design or if this is a real bug that needs a fix in frontend and backend. To me, this looks like a bug.

bug_detail

@melvin-bot melvin-bot bot added the Overdue label Jul 21, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 24, 2023

@robertjchen, @michaelhaxhiu Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot
Copy link

melvin-bot bot commented Jul 26, 2023

@robertjchen, @michaelhaxhiu Still overdue 6 days?! Let's take care of this!

@michaelhaxhiu
Copy link
Contributor

@rojiphil Thanks for fleshing that out more for me. I find your input compelling. Let's go external and see if we can fix the small issue.

@melvin-bot melvin-bot bot removed the Overdue label Jul 26, 2023
@michaelhaxhiu michaelhaxhiu added External Added to denote the issue can be worked on by a contributor Overdue labels Jul 26, 2023
@melvin-bot melvin-bot bot changed the title Money request is shown from the other account instead of current user. [$1000] Money request is shown from the other account instead of current user. Jul 26, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 26, 2023

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

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

melvin-bot bot commented Jul 26, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 26, 2023

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

@rojiphil
Copy link
Contributor

Proposal

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

The problem is that the chat initiator's avatar and name are not displayed in the chat. If you look at my video here - #22862 (comment), chat is sent from T5 to T6. However, the chat is attributed to be sent by T6 even though it was initiated by T5. This is the problem we are trying to solve.

What is the root cause of that problem?

The root cause of this issue is at this line where we are building an optimistic report preview. Here, the actorAccountID is set with report's managerID which could be payer/payee depending on the context.

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

Setting actorAccountID with currentUserAccountID would ensure that the chat initiator's details will be set to the current user who initiated it.
The change has to be made at this line here like this:

actorAccountID: currentUserAccountID,

Further, when API with command RequestMoney is sent, the BE should ensure that reportAction with actionName: REPORTPREVIEW has the actorAccountID set to the first time initiator's account id. I think, subsequent RequestMoney requests from either actors (T5 or T6) shouldn't update the actorAccountID. wdyt?

What alternative solutions did you explore? (Optional)

@melvin-bot
Copy link

melvin-bot bot commented Jul 28, 2023

@robertjchen @michaelhaxhiu @Santhosh-Sellavel 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!

@Santhosh-Sellavel
Copy link
Collaborator

This looks like expected behavior to me @michaelhaxhiu please check with the product & design team.

@melvin-bot melvin-bot bot added the Overdue label Jul 31, 2023
@michaelhaxhiu
Copy link
Contributor

Thansk for your input @Santhosh-Sellavel. I spent time to look into this a little further and have changed my stance, we do this intentionally. Closing this GH.

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
None yet
Development

No branches or pull requests

10 participants