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

[HOLD for payment 2024-03-22] [$500] [MEDIUM] Approved expense preview does not show GBR when submitter needs to add a bank account #34278

Closed
6 tasks done
m-natarajan opened this issue Jan 10, 2024 · 41 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Jan 10, 2024

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


Version Number:
Reproducible in staging?: needs reproduction
Reproducible in production?:
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: @anmurali
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1704819780539799

Action Performed:

Create a money request and submit it to a Collect workspace. Approve and pay as the workspace admin.

Expected Result:

After an expense is approved, the submitting user should see a GBR in the expense preview which prompts them to add a bank account by clicking in to reveal the Add bank account CTA.

Actual Result:

After an expense is approved, the submitting does not see a GBR in the expense preview, and there is no indicator that they must click in to find the Add bank account CTA.

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

Add any screenshot/video evidence
File (2)

File (3)

IMG_0239

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e7910dca0f3c5b57
  • Upwork Job ID: 1747385136600576000
  • Last Price Increase: 2024-01-23
  • Automatic offers:
    • cubuspl42 | Reviewer | 28131009
    • tienifr | Contributor | 28131010
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 10, 2024
Copy link

melvin-bot bot commented Jan 10, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jan 12, 2024
@stephanieelliott
Copy link
Contributor

Hey @m-natarajan were you able to reproduce this, or should we add the Needs Reproduction label?

Copy link

melvin-bot bot commented Jan 16, 2024

@stephanieelliott Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@stephanieelliott stephanieelliott added Needs Reproduction Reproducible steps needed External Added to denote the issue can be worked on by a contributor labels Jan 16, 2024
@melvin-bot melvin-bot bot changed the title We are not showing the GBR on an approved expense preview when the user needs to add a bank account [$500] We are not showing the GBR on an approved expense preview when the user needs to add a bank account Jan 16, 2024
Copy link

melvin-bot bot commented Jan 16, 2024

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

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

melvin-bot bot commented Jan 16, 2024

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

@tienifr
Copy link
Contributor

tienifr commented Jan 17, 2024

Proposal

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

No GBR in the approved expense report preview

What is the root cause of that problem?

This is new feature, we never had it implemented.

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

When the user needs to add a bank account, they will see a button inside the IOU report, the condition to show that button is here.

But in the chat report REPORTPREVIEW here, we only shows RBR if there's an error, we currently don't show any GBR for required action like the user needs to add a bank account. We need to show GBR (basically a Expensicons.DotIndicator with theme.success as fill) if there's a pending action based on the same condition mentioned above that show the button itself in the IOU report.

Pseudocode (to add here and in report preview):

const shouldPromptUserToAddBankAccount = ...; // same condition as the one to show Add bank account button in IOU report
const shouldShowRBR = !iouSettled && hasErrors;

...

{shouldShowRBR && (
    <Icon
        src={Expensicons.DotIndicator}
        fill={theme.danger}
    />
)}
{!shouldShowRBR && shouldPromptUserToAddBankAccount && (
   <Icon
        src={Expensicons.DotIndicator}
        fill={theme.success}
    />
)
}

What alternative solutions did you explore? (Optional)

NA

@happy-man
Copy link

Hello. I want to work on the issue but I'm unable to do initial npm install.
I've got such error
image
As I understand from error messages project's version of react is 18 and @storybook package wants a version not higher than 17.
Please, help me resolve this issue properly. Thanks.

Copy link

melvin-bot bot commented Jan 17, 2024

📣 @happy-man! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@cubuspl42
Copy link
Contributor

@happy-man

Welcome to Expensify! Please ensure to read the contributing guidelines and check out a few other issues with other contributors' proposals to get a feeling of how the Expensify process works. If you have any general questions unrelated to this issue, ask them on #expensify-open-source Slack channel.

@happy-man
Copy link

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01ecd21cecad2f34a0

Copy link

melvin-bot bot commented Jan 17, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@happy-man
Copy link

@happy-man

Welcome to Expensify! Please ensure to read the contributing guidelines and check out a few other issues with other contributors' proposals to get a feeling of how the Expensify process works. If you have any general questions unrelated to this issue, ask them on #expensify-open-source Slack channel.

Hello! Thanks. How can I be added to slack channel?

@stephanieelliott
Copy link
Contributor

@happy-man please read the contributing guidelines, this instructions for getting added to Slack are included there.

To request an invite to Slack, email [email protected] with the subject Slack Channel Invites. We'll send you an invite!

@cubuspl42
Copy link
Contributor

@happy-man

Hi again!

Please ensure to read the contributing guidelines. Have you read the contributing guidelines? If you have some questions, you can also check the contributing guidelines. Sometimes, I feel that people don't read the contributing guidelines, but then I realize that it's unlikely, because two bots and four people ask them to read the contributing guidelines.

@cubuspl42
Copy link
Contributor

@tienifr

This is new feature, we never had it implemented.

Does this chat log clarify anything?

image

@melvin-bot melvin-bot bot added the Overdue label Jan 22, 2024
@tienifr
Copy link
Contributor

tienifr commented Jan 22, 2024

@stephanieelliott @anmurali I think we should update the OP since a few places are not exactly clear:

  • From this thread, it's mentioned that the "specifically the description" is supposed to be in the preview. But the OP in this GH issue doesn't mention about description, it's just about GBR and CTA
  • The thread also doesn't mention anything about GBR, CTA when confirming it's a regression, so a bug so I'm not sure whether it's included in this issue's scope. Are the GBR, CTA issue mentioned in the OP a regression? Or are they new features we want to implement here.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Feb 1, 2024
@stephanieelliott
Copy link
Contributor

PR is under review

@stephanieelliott
Copy link
Contributor

Seems like this is held up on some clarifying questions asked in the PR, I bumped that in Slack in case it was missed within the PR: https://expensify.slack.com/archives/C01GTK53T8Q/p1707427263037779

@cubuspl42
Copy link
Contributor

As per the PR status, the most important feedback was this thread. I bumped @tienifr.

@stephanieelliott
Copy link
Contributor

PR is being actively reviewed.

@stephanieelliott stephanieelliott removed the Needs Reproduction Reproducible steps needed label Feb 27, 2024
@stephanieelliott
Copy link
Contributor

PR is still under active review!

@cubuspl42
Copy link
Contributor

@stephanieelliott We're having problems with reproducing the context of this issue. It's partially my fault, as I should've caught this earlier.

Slack thread: https://expensify.slack.com/archives/C01GTK53T8Q/p1708946657174689?thread_ts=1708772104.743519&cid=C01GTK53T8Q

@stephanieelliott
Copy link
Contributor

Repro issue was resolved in Slack, PR is now under active review

@cubuspl42
Copy link
Contributor

This is on me now, I'll do the checklist today 👍

@stephanieelliott
Copy link
Contributor

Looks like we are working through testing on the PR

@cubuspl42
Copy link
Contributor

Yeah, sorry for the delay! Some things apparently changed under our feet because of the work from other PRs; we want to ensure everything is working as expected.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Mar 15, 2024
@melvin-bot melvin-bot bot changed the title [$500] [MEDIUM] Approved expense preview does not show GBR when submitter needs to add a bank account [HOLD for payment 2024-03-22] [$500] [MEDIUM] Approved expense preview does not show GBR when submitter needs to add a bank account Mar 15, 2024
Copy link

melvin-bot bot commented Mar 15, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Mar 15, 2024
Copy link

melvin-bot bot commented Mar 15, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.52-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-03-22. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Mar 15, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@cubuspl42] The PR that introduced the bug has been identified. Link to the PR:
  • [@cubuspl42] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@cubuspl42] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@cubuspl42] Determine if we should create a regression test for this bug.
  • [@cubuspl42] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@melvin-bot melvin-bot bot added the Overdue label Mar 25, 2024
@cubuspl42
Copy link
Contributor

  • The PR that introduced the bug has been identified. Link to the PR:
    • Essentially it's a new feature fixing a "UX bug"
  • The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
    • N/A
  • A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
    • No need for additional discussion
  • Determine if we should create a regression test for this bug.
    • No
  • If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
    • N/A

@melvin-bot melvin-bot bot removed the Overdue label Mar 25, 2024
@stephanieelliott
Copy link
Contributor

All paid! Summarizing payment on this issue:

  • Contributor: @tienifr $500 PAID via Upwork
  • Contributor+: @cubuspl42 $500 PAID via Upwork

Upwork job is here: https://www.upwork.com/jobs/~01e7910dca0f3c5b57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
No open projects
Development

No branches or pull requests

6 participants