anmurali commented 3 months ago

There are two intents Get paid back by my employer and Chat and split bills with friends where one of the onboading tasks we show is Enable your wallet. But the wallet flow is onerous and we actually have not cleaned it up to show clear and actionable error messages and this is not a priority either till more people use IOU payments.

So for now, let's replace this task with a Add personal bank account

Title: Add personal bank account Description:

You’ll need to add your personal bank account to get paid back. Don’t worry, it’s easy! Here’s how to set up your bank account:

Click your profile picture.
Click Wallet > Bank accounts > + Add bank account.
Connect your bank account.

Once that’s done, you can request money from anyone and get paid back right into your personal bank account.

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01a9b5735e8551a25a
Upwork Job ID: 1816570512509995571
Last Price Increase: 2024-07-25
Automatic offers:

nyomanjyotisa | Contributor | 103281558

Issue Owner

Current Issue Owner: @kadiealexander

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @kadiealexander (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

melvin-bot[bot] commented 3 months ago

:warning: It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time :warning:

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify (NewFeature)

trjExpensify commented 3 months ago

(we good here, Jon.. copy updates and shizz).

nyomanjyotisa commented 3 months ago

Proposal

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

Replace Enable your wallet with Add personal bank account task during onboarding

What is the root cause of that problem?

New feature

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

We need update the enableWallet task for both Get paid back by my employer and Chat and split bills with friends here and here with Add personal bank account:

                {
                    type: 'addBankAccount',
                    autoCompleted: false,
                    title: 'Add personal bank account',
                    description:
                        'You’ll need to add your personal bank account to get paid back. Don’t worry, it’s easy!\n' +
                        '\n' +
                        'Here’s how to set up your bank account:\n' +
                        '\n' +
                        '1. Click your profile picture.\n' +
                        '2. *Click Wallet* > *Bank accounts* > *+ Add bank account*.\n' +
                        '3. Connect your bank account.\n' +
                        '\n' +
                        'Once that’s done, you can request money from anyone and get paid back right into your personal bank account.',
                },

RESULT

What alternative solutions did you explore? (Optional)

mollfpr commented 3 months ago

The proposal from @nyomanjyotisa looks good to me!

🎀 👀 🎀 C+ reviewed!

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

📣 @nyomanjyotisa 🎉 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 📖

melvin-bot[bot] commented 3 months ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

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

https://github.com/Expensify/App/pull/46359

If no regressions arise, payment will be issued on 2024-08-09. :confetti_ball:

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

@mollfpr requires payment through NewDot Manual Requests
@nyomanjyotisa requires payment automatic offer (Contributor)

melvin-bot[bot] commented 3 months ago

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

[ ] [@mollfpr] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
[ ] [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

melvin-bot[bot] commented 3 months ago

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

https://github.com/Expensify/App/pull/46359

If no regressions arise, payment will be issued on 2024-08-13. :confetti_ball:

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

@mollfpr requires payment through NewDot Manual Requests
@nyomanjyotisa requires payment automatic offer (Contributor)

melvin-bot[bot] commented 3 months ago

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

[ ] [@mollfpr] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
[ ] [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

mollfpr commented 3 months ago

No payments are due yet. The PR is reverted here https://github.com/Expensify/App/pull/46630.

We check the name of the onboarding tasks to create in Auth, and at the moment Auth doesn't expects any task called addBankAccount

cc @francoisl @Beamanator

Beamanator commented 3 months ago

Thank you sir, closing!

mollfpr commented 3 months ago

@Beamanator Should we keep the issue open and get the BE change?

trjExpensify commented 3 months ago

Yes, this was reverted.

Beamanator commented 3 months ago

Oh merp my bad for not understanding 🙃 I'll let @tgolen do his thing here, I'll back away slowly..........

tgolen commented 3 months ago

OK... so, I'm coming up to speed on this. It sounds like a backend change is necessary to support the front-end PR. Is anyone on that?

trjExpensify commented 3 months ago

Doesn't look like anyone is on it, and the revert was because Auth expects the task name to match: https://github.com/Expensify/App/pull/46630#issue-2441277658

tgolen commented 3 months ago

OK, thanks! Any volunteers to do the BE fix? I probably won't be able to get to it until the tail end of next week.

tgolen commented 3 months ago

Assuming there are no volunteers, I will try to get to this later this week.

trjExpensify commented 3 months ago

Yeah, that assumption is correct. Thanks, Tim!

tgolen commented 3 months ago

@francoisl Would you mind pointing me to the right spot in Auth where this needs to be added? I'm not familiar with the flow at all so I'm not sure where to look.

trjExpensify commented 3 months ago

I believe it's here: https://github.com/Expensify/Auth/blob/29ab63ee487aa54a00f9855c5a76a768e374aa76/auth/lib/GuidedSetup.h#L60

francoisl commented 3 months ago

That's right but there's a little more too:

Create a constant for the new task here
Add the constant to this set so that completeSetupTask() doesn't throw
Add the logic to auto-complete the task by calling auth.guidedSetup.completeSetupTask() when the user takes the corresponding action

tgolen commented 3 months ago

Ah, cool. Thanks! That should get me started

trjExpensify commented 3 months ago

Is this a new task or just replacing the existing enableWallet task? :/

anmurali commented 3 months ago

Replace enable wallet task.

francoisl commented 3 months ago

Oh right, then it's easier, especially for step 3. I'd suggest making the change in multiple steps though:

Add a new task const that is essentially a duplicate of TASK_ENABLE_WALLET in Auth, deploy that
Use the new task in App, wait a bit for most clients to update
Remove the old task from Auth

tgolen commented 3 months ago

hehe, @francoisl you know a lot about this. Are you sure you aren't the volunteer I was looking for? 😉 😉

francoisl commented 3 months ago

If it can wait until mid next week sure, otherwise I don't think I'll have time until then.

tgolen commented 3 months ago

OK, I'll see if I have time to get to it before then.

On Wed, Aug 14, 2024 at 3:43 PM Francois Laithier @.***> wrote:

If it can wait until mid next week sure, otherwise I don't think I'll have time until then.

— Reply to this email directly, view it on GitHub https://github.com/Expensify/App/issues/46231#issuecomment-2289966974, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJMAB5SNOCT3OU4EIKUUN3ZRPFOHAVCNFSM6AAAAABLPHZJ2OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBZHE3DMOJXGQ . You are receiving this because you were mentioned.Message ID: @.***>

tgolen commented 3 months ago

Daily Update

I created an Auth PR which implements the new task as merely a copy of the old enable wallet task. The code change was straight forward, but updating the tests were not as straight forward. I hope I did them justice!

Next Steps

@deetergp @francoisl review and merge the Auth PR
Wait for it to deploy
@tgolen Re-revert the frontend PR and get it merged
@tgolen Write another Auth PR to remove the old enable wallet task

ETA

First Auth PR merged by tomorrow, Aug 16
Frontend PR merged and deployed by Friday, Aug 23
Second Auth PR merged by Tuesday, Aug 27

tgolen commented 2 months ago

Daily Update

The Auth PR has been merged

Next Steps

Wait for Auth to deploy
@nyomanjyotisa can reimplement the original PR that was reverted

ETA

Auth deploy should happen today, Aug 20. I'll post back here when I see it's been deployed.

kadiealexander commented 2 months ago

@tgolen any further updates on this one? Thanks!

tgolen commented 2 months ago

Yeah, the auth change was deployed 3 days ago so @nyomanjyotisa can go ahead and reimplement the frontend.

melvin-bot[bot] commented 2 months ago

@tgolen, @mollfpr, @kadiealexander, @nyomanjyotisa Whoops! This issue is 2 days overdue. Let's get this updated quick!

mollfpr commented 2 months ago

@nyomanjyotisa Could you create the new PR? Thanks!

kadiealexander commented 2 months ago

@nyomanjyotisa bump!

nyomanjyotisa commented 2 months ago

@mollfpr PR is ready for review, sorry for the delay 🙏

VictoriaExpensify commented 2 months ago

@mollfpr - have you had a chance to review this yet?

mollfpr commented 2 months ago

@VictoriaExpensify The changes on the PR look good to me! We are waiting for the BE changes to make the task auto mark working.

mollfpr commented 1 month ago

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR: [@mollfpr] 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:

No offending PR.

[@mollfpr] 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:

The regression step should be good!

[@mollfpr] Determine if we should create a regression test for this bug. [@mollfpr] 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.

Login with new account
Choose 'Get paid back by my employer or 'Chat and split bills with' on onboarding
Enter your name
Verify that 'Add personal bank account' task exist in Concierge chat
👍 or 👎

@kadiealexander The PR is deployed to production last week, could you give the payment summary? Thank you!

kadiealexander commented 1 month ago

Payouts due:

[x] Contributor: $250 @nyomanjyotisa (Offer link)
[x] Reviewer: $250 @mollfpr via manual request

Upwork job is here.

garrettmknight commented 1 month ago

$250 approved for @mollfpr

Expensify / App

[Guided Setup] [$250] Replace Enable your wallet with Add personal bank account task during onboarding #46231

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Daily Update

Next Steps

ETA

Daily Update

Next Steps

ETA