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 2025-01-13] [GR Phase 3] Step 2 - navigation, default data, validation logic and API call to save it #50904

Closed
madmax330 opened this issue Oct 16, 2024 · 27 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2

Comments

@madmax330
Copy link
Contributor

Design doc section: https://docs.google.com/document/d/1Y4P7G9h0Easjcea7wyk52gKrNB9o1yTjpmJFYkrHDW4/edit?tab=t.0#bookmark=id.bos524lch8mt

@pasyukevich
Copy link
Member

Hi, I'm Eugene from Callstack - expert contributor group. I’d like to work on this issue.

@pasyukevich
Copy link
Member

Updates:
Finished with replacing mocks to the API call to get fields
Added one more missed screen - AccountHolderDetails
Updated the Navigation between substeps logic

Keep working on the API call for bank account creation

@pasyukevich
Copy link
Member

Finished API integrations

Working on error handling for specific cases

@pasyukevich
Copy link
Member

Today's update:

Covering edge cases, polishing the code

Waiting for the data to have a successful path with API

@pasyukevich
Copy link
Member

Today's update:

Updated request with required parameters for the successful path with API

@pasyukevich
Copy link
Member

Today's update:

Added error handling for the API create bank account request

Adding error handling for the get fields request

@pasyukevich
Copy link
Member

Today's update:

All updates and polishing are done - preparing the videos

@pasyukevich
Copy link
Member

Today's update:

Found the problem on the native side in case of error leads to inconsistent behavior
Preparing the fix

@pasyukevich
Copy link
Member

Today's update:

Merged main

Found the root cause of the problem, changing the error handling approach for the failed API create bank account request

@hungvu193
Copy link
Contributor

Any updates here?

@pasyukevich
Copy link
Member

Sure, current updates:

  • Changed Confirmation substep to the Form for better API request handling

  • Facing the issue that we cannot handle the loading state with the same onyx key since it will trigger updates of the wrapper. Working now on this

@pasyukevich
Copy link
Member

Today's update:

  • Problem solved, error handling updated
  • Faced with changes on BE, waiting for clarification for the updated contract

@pasyukevich
Copy link
Member

Today's update:

  • Adjusted request submitted with new props
  • Waiting for BE fix to have the success path

@pasyukevich
Copy link
Member

Waiting for API updates

@pasyukevich
Copy link
Member

Applied updated to stabilize the flow

I started recording videos, now blocked by the API error that it was before

Waiting for API updates

image

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Monthly KSv2 labels Dec 13, 2024
@pasyukevich
Copy link
Member

Ready for review

Copy link

melvin-bot bot commented Jan 1, 2025

⚠️ 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.

Copy link

melvin-bot bot commented Jan 1, 2025

⚠️ 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 melvin-bot bot removed the Weekly KSv2 label Jan 6, 2025
Copy link

melvin-bot bot commented Jan 6, 2025

This issue has not been updated in over 15 days. @madmax330, @hungvu193, @pasyukevich 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!

@melvin-bot melvin-bot bot added Monthly KSv2 Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Monthly KSv2 labels Jan 6, 2025
@melvin-bot melvin-bot bot changed the title [GR Phase 3] Step 2 - navigation, default data, validation logic and API call to save it [HOLD for payment 2025-01-13] [GR Phase 3] Step 2 - navigation, default data, validation logic and API call to save it Jan 6, 2025
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 6, 2025
Copy link

melvin-bot bot commented Jan 6, 2025

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

Copy link

melvin-bot bot commented Jan 6, 2025

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.80-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 2025-01-13. 🎊

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

  • @hungvu193 requires payment through NewDot Manual Requests
  • @pasyukevich does not require payment (Contractor)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 13, 2025
Copy link

melvin-bot bot commented Jan 13, 2025

Issue is ready for payment but no BZ is assigned. @Christinadobrzyn you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

Copy link

melvin-bot bot commented Jan 13, 2025

Payment Summary

Upwork Job

  • Reviewer: @hungvu193 owed $250 via NewDot
  • Contributor: @pasyukevich is from an agency-contributor and not due payment

BugZero Checklist (@Christinadobrzyn)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@Christinadobrzyn
Copy link
Contributor

Payment summary here - #50904 (comment)

@hungvu193 do we need a regression test for this?

@hungvu193
Copy link
Contributor

Regression Test:

  1. Change workspace currency to EUR, GBP, CAD or AUD.
  2. Go to Workpsace, More feature page and Enable workflows for the workspace.
  3. Open workflows.
  4. Try to connect a bank account.
  5. Go to the second step - a new component should be displayed.
  6. The first substep validations and field lists will be received from the BE.
  7. For the AUD currency we should have 3 substeps (on the second upload file step, for other currencies should be skipped)
  8. Verify Upload picker validations (10 files limit, 5MB total, PDF/PNG/JPEG only)
  9. Go to the third step
  10. Repeat the steps and verify that the error is shown due to invalid data

Do we 👍 or 👎 ?

@Christinadobrzyn
Copy link
Contributor

Regression test - https://github.com/Expensify/Expensify/issues/460902

Payment summary here - #50904 (comment)

I think this can be closed! @hungvu193 don't forget to request payment in ND!

@garrettmknight
Copy link
Contributor

$250 approved for @hungvu193

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 Daily KSv2
Projects
None yet
Development

No branches or pull requests

5 participants