Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.89k forks source link

[HOLD for payment 2024-03-08] Implement Tiered Business Bank Account flow #22901

Closed mountiny closed 6 months ago

mountiny commented 1 year ago

This is an implementation issue for the Tiered bank account setup flow in NewDot internal project, design doc can be found here.

All the details will be added to this issue once the implementation will be ready to start.

cc @gedu

melvin-bot[bot] commented 1 year ago

Current assignee @anmurali is eligible for the NewFeature assigner, not assigning anyone new.

melvin-bot[bot] commented 1 year ago

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

gedu commented 1 year ago

Hey Edu from Callstack

anmurali commented 1 year ago

Detailed design is in review

anmurali commented 1 year ago

Still in review. @gedu there's several sections (Manual tests, automated tests, rollout plan) that you still need to finish for this doc to be done!

anmurali commented 1 year ago

@gedu bump on this. Were you able to get those sections completed?

anmurali commented 1 year ago

Asked for an update here

gedu commented 1 year ago

I've provided more comprehensive information on how to approach this UI/UX migration, concentrating on elaborating on new components and how to manage navigation, states, and so forth. I initiated the Manual Test section, and I'm only left with the final step. Later, I'll proceed with Automated tests.

gedu commented 1 year ago

Added one more step, and asked some questions about some steps

gedu commented 1 year ago

@anmurali I added the Manual, Automated and Rollout sections. There are some questions.

gedu commented 1 year ago

@anmurali Did you have the chance to review the new sections?

anmurali commented 1 year ago

Not yet @gedu - I think you also need all the same engineering reviewers to go back through it. So tagging @ryanschaffer @johnmlee101 @joekaufmanexpensify and @joelbettner on that. Please skim through your comments in the doc and make sure we addressed them and resolve? Also we added manual and automated tests, a rollout plan for your review. https://docs.google.com/document/d/1t8_Fn93LSSU4gG4z7WRt8IFcpSGK4TIe4QamdcTXjxE/edit?usp=sharing

anmurali commented 1 year ago

No update.

anmurali commented 1 year ago

We are now implementing this as part of #wave6-collect-category-tag-submitters-07nov23

gedu commented 1 year ago

I start creating the issues to start working on this

gedu commented 1 year ago

Issues created and added into the Document

melvin-bot[bot] commented 11 months ago

This issue has not been updated in over 15 days. @gedu, @anmurali eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

anmurali commented 10 months ago

Tracking the first part of this update here

mountiny commented 9 months ago

Assigned @nkuoch and @MariaHCD so they get the credits for reviewing the PR as well 🙌 thanks for help!

melvin-bot[bot] commented 9 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 9 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 9 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 9 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.

joekaufmanexpensify commented 9 months ago

I have this one where the android app is crashing when setting up a VBA that feels related to this. LMK if you agree!

mountiny commented 9 months ago

@joekaufmanexpensify The PR was just merged and the issue was reported 2 weeks ago. Could Applause try to retest now after the refactor?

joekaufmanexpensify commented 9 months ago

Ah, interesting. I didn't realize that was the only PR for this one. I see mine is actually a dupe of another too. So going to close in favor of that one.

trjExpensify commented 8 months ago

👋 coming from this issue.

I suspect some regression test scripts need to be passed over to the Applause to update/add/remove based on the new refactored flow. Who's taking care of that for this project? Let's make sure when this is shipped we do that, so we don't have outdated flows and expected behaviour for the VBBA flow.

mountiny commented 8 months ago

I have shared this script with applause when this was deployed to staging. In Slack, discussing here.

trjExpensify commented 8 months ago

Amazing, thanks!

shubham1206agra commented 8 months ago

These are the PRs apart from the main one that I reviewed (Main PR https://github.com/Expensify/App/pull/34498)

shubham1206agra commented 8 months ago

@mountiny Can you post about payment here as we discussed?

melvin-bot[bot] commented 8 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.

mountiny commented 8 months ago

This project spanned couple of months and lots of discussions along the way, testing and QA as well as a follow up PRs. I propose we will pay $5000 for this project to @shubham1206agra given the PRs listed. They have also been removed from the c+ team for some time so they can focus on these changes which meant they were not assigned more work.

Thats why I think the price is appropriate for their work

akinwale commented 8 months ago

@mountiny As discussed, here are the PRs I have reviewed for the project.

Main PR https://github.com/Expensify/App/pull/34498

Follow-up PRs https://github.com/Expensify/App/pull/36472 https://github.com/Expensify/App/pull/37025 https://github.com/Expensify/App/pull/37032

Regressions fixed by external contributors https://github.com/Expensify/App/pull/36573 https://github.com/Expensify/App/pull/36585 https://github.com/Expensify/App/pull/36589 https://github.com/Expensify/App/pull/36621 https://github.com/Expensify/App/pull/36763

mountiny commented 8 months ago

As discussed before for @akinwale here, we have proposed $1500 for the review of the PR and since he has helped with all the regressions too, I think no penalty should be imposed as it was large PR.

So $2000 for @akinwale for their help on this project and the follow up issues.

mountiny commented 8 months ago

Summary:

The breakdown of the PRs is above

@anmurali this is ready for payment

melvin-bot[bot] commented 8 months ago

Payment Summary

[Upwork Job]()

BugZero Checklist (@anmurali)

anmurali commented 8 months ago

Offers sent to @akinwale (here) and @shubham1206agra (here)

akinwale commented 8 months ago

Offers sent to @akinwale (here) and @shubham1206agra (here)

@anmurali Accepted. Thanks.

shubham1206agra commented 8 months ago

@anmurali Can you hold my payment temporarily as per https://expensify.slack.com/archives/C02NK2DQWUX/p1710150138788529?

anmurali commented 8 months ago

Yes. @shubham1206agra - ping here when you're ready for us to process it. @akinwale is paid.

shubham1206agra commented 7 months ago

@anmurali, I have discussed this internally. You may close this issue as I am keeping track of payment internally and will ask to pay once the issue is resolved. Just write in the payment summary that I have not been paid yet.

shubham1206agra commented 6 months ago

@anmurali You can process payment here now.

shubham1206agra commented 6 months ago

@anmurali Bump on the above.

shubham1206agra commented 6 months ago

@anmurali Bump for the payment.

shubham1206agra commented 6 months ago

@anmurali Bump here.

anmurali commented 6 months ago

Paid.