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

[$500] Expense - "Pay with Expensify" button is grayed out briefly after requesting money #35894

Closed
6 tasks done
izarutskaya opened this issue Feb 6, 2024 · 36 comments
Closed
6 tasks done
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

@izarutskaya
Copy link

izarutskaya commented Feb 6, 2024

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: v.1.4-37.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:
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:

Action Performed:

Precondition: User is a member of workspace in which the currency is different from the local currency.

  1. Go to staging.new.expensify.com
  2. Go to workspace chat.
  3. Create a manual request.
  4. Create another manual request.

Expected Result:

"Pay with Expensify" button in the main chat will not gray out.

Actual Result:

"Pay with Expensify" button is grayed out briefly after requesting money.

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Bug6369082_1707216168534.bandicam_2024-02-06_08-24-33-092.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b74cf4a81b61a7fd
  • Upwork Job ID: 1754829907209736192
  • Last Price Increase: 2024-02-13
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment 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 Feb 6, 2024
@melvin-bot melvin-bot bot changed the title Expense - "Pay with Expensify" button is grayed out briefly after requesting money [$500] Expense - "Pay with Expensify" button is grayed out briefly after requesting money Feb 6, 2024
Copy link

melvin-bot bot commented Feb 6, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 6, 2024
Copy link

melvin-bot bot commented Feb 6, 2024

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

Copy link

melvin-bot bot commented Feb 6, 2024

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Feb 6, 2024
Copy link
Contributor

github-actions bot commented Feb 6, 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.

Copy link

melvin-bot bot commented Feb 6, 2024

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

@izarutskaya
Copy link
Author

We think that this bug might be related to #wave5-free-submitters
CC @dylanexpensify

@izarutskaya
Copy link
Author

Not repro in prod

bandicam.2024-02-06.08-26-05-396.mp4

@danieldoglas
Copy link
Contributor

Hm, I don't think this needs to be a deploy blocker. But if it's not happening in production, we need to find the offending PR. @aimane-chnaif , can you please help out with that?

@danieldoglas danieldoglas removed the DeployBlockerCash This issue or pull request should block deployment label Feb 6, 2024
@tienifr
Copy link
Contributor

tienifr commented Feb 6, 2024

This comes from #35062

Proposal

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

"Pay with Expensify" button is grayed out briefly after requesting money.

What is the root cause of that problem?

We want to disable the settlement button temporarily when the amount is being updated, this only happens if the currency of the request is different from the report.

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

I think we should improve the UX of the disabling because right now the user doesn't know why the button is disabled. While the total is updating in this case, we can either:

  • Add a tooltip telling the user "The IOU can be settled after the total amount is calculated"
  • Or the button text can be changed to something like Calculating amount...
  • Or still keep the button enabled, but throw an error saying the amount is not finalized, if the user tries to click on the button (this is more aligned with our general app patterns)

What alternative solutions did you explore? (Optional)

NA

@aimane-chnaif
Copy link
Contributor

I am not able to reproduce both on main and staging.

Screen.Recording.2024-02-06.at.12.39.10.PM.mov

Am I missing anything?

@tienifr
Copy link
Contributor

tienifr commented Feb 6, 2024

@aimane-chnaif Please make sure the request currency is different from the IOU currency

@aimane-chnaif
Copy link
Contributor

Thanks! Reproduced

@aimane-chnaif
Copy link
Contributor

This seems more like improvement than bug.
@danieldoglas should we confirm the expected behavior?

@danieldoglas
Copy link
Contributor

I do agree that this sounds like an improvement, but if it doesn't happen in production, we need to find out what changed this and if it was intentional

@aimane-chnaif
Copy link
Contributor

cc: @BartoszGrajdek

@BartoszGrajdek
Copy link
Contributor

BartoszGrajdek commented Feb 6, 2024

Hi! I'm the author of #35062 which is the root cause of this.

I have recently found a new bug happening whenever there was a money report with requests using currencies different than the money report itself. User could add money requests with various currencies to the money report while being offline and click "Pay elsewhere" immediately, this is a bug because the total amount wouldn't be updated in the meantime (it can only come from the backend after we convert currencies, so the user needs to be online for that to happen), since the total amount isn't updated user can't be actually certain on what amount he should pay.

Now my solution was to add a condition whenever there are both transactions with pending fields and there's a transaction using currency different than the money report itself to prevent the user from paying for money report until everything gets updated.

I'm not sure how you would like to proceed from here, I agree that we could add some UX improvements, but I don't think that it's a bug.

@tienifr
Copy link
Contributor

tienifr commented Feb 8, 2024

@johnmlee101 The problem also is if the user is in offline mode, the button will be grayed out forever without explanations.

Also I think the form pattern in the app is to never disable a button, but to still let the user click it and tell them what's wrong via an error message.

I suggest to keep the button enabled and when the user clicks it, show an error saying something like "Currency conversion is in progress, please try again later". That way, the user will know what goes wrong and what they should do, and align with our app's form patterns.

@danieldoglas danieldoglas added Daily KSv2 and removed Hourly KSv2 labels Feb 8, 2024
@garrettmknight
Copy link
Contributor

@tienifr that sounds like a different problem altogether. I was just testing it and it looks like the buttons are greyed out when offline, but I was able to submit and expense/approve it on a Collect workspace + pay one on a Free workspace.

@melvin-bot melvin-bot bot added the Overdue label Feb 12, 2024
@danieldoglas
Copy link
Contributor

@garrettmknight so you think we should leave this as is for now, or still change the text?

@melvin-bot melvin-bot bot removed the Overdue label Feb 12, 2024
@tienifr
Copy link
Contributor

tienifr commented Feb 12, 2024

@tienifr that sounds like a different problem altogether. I was just testing it and it looks like the buttons are greyed out when offline

@garrettmknight I believe it's due to the same root cause, before this PR #35062, the button is not disabled while offline and the IOU can still be paid while offline.

Screenshot 2024-02-12 at 11 33 29 PM

Inside the IOU in this case we'll have the "Total will update when you're back online" text so I think we should have something similar in the report preview, rather than a disabled "Pay elsewhere" button without any explanation (eg. we can make the Total will update when you're back online the tooltip of the disabled button, or just show that text instead of the button)

Copy link

melvin-bot bot commented Feb 13, 2024

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

@garrettmknight
Copy link
Contributor

Thanks for the explanation - I think I get it now, but I have a different concern with the ability to 'Pay' when offline. Submitters can change the amount after it's been approved/paid offline and when the approver/payer comes online, we approve or pay the updated report total. That's being discussed here and I've opened up a GH to correct here.

I think solving that problem might actually fix this one, so I'm proposing we pop this on hold for the other issue and reopen this if the flow is still valid.

@garrettmknight garrettmknight changed the title [$500] Expense - "Pay with Expensify" button is grayed out briefly after requesting money [HOLD on E/E #369265] [$500] Expense - "Pay with Expensify" button is grayed out briefly after requesting money Feb 13, 2024
@garrettmknight garrettmknight added Weekly KSv2 and removed Daily KSv2 labels Feb 14, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 23, 2024
@garrettmknight
Copy link
Contributor

Holding on #36440

@melvin-bot melvin-bot bot removed the Overdue label Feb 23, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 4, 2024
@danieldoglas
Copy link
Contributor

still on hold

@melvin-bot melvin-bot bot removed the Overdue label Mar 4, 2024
@garrettmknight
Copy link
Contributor

Holding on #36440

@melvin-bot melvin-bot bot added the Overdue label Mar 14, 2024
@garrettmknight garrettmknight added Daily KSv2 and removed Weekly KSv2 labels Mar 14, 2024
@melvin-bot melvin-bot bot removed the Overdue label Mar 14, 2024
@garrettmknight
Copy link
Contributor

Tested, still getting the greyed out part happening.

@garrettmknight garrettmknight changed the title [HOLD on E/E #369265] [$500] Expense - "Pay with Expensify" button is grayed out briefly after requesting money [$500] Expense - "Pay with Expensify" button is grayed out briefly after requesting money Mar 14, 2024
@garrettmknight
Copy link
Contributor

@tienifr does your prososal still stand?

@garrettmknight
Copy link
Contributor

Actually, closing since this is behavior doesn't really stop anyone from using the app. This feels like extreme polish.

@tienifr
Copy link
Contributor

tienifr commented Mar 16, 2024

Actually, closing since this is behavior doesn't really stop anyone from using the app. This feels like extreme polish.

This behavior might be a polish in online mode, but I believe in offline mode it's a far more serious issue since it will cause much confusion, see below.

The problem also is if the user is in offline mode, the button will be grayed out forever without explanations.

Also it's violating our app's form pattern, more explanation here, do you think we should reopen this?

@tienifr does your prososal still stand?

@garrettmknight Yes my proposal would still work well for this case.

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

8 participants