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

[change] Move redirect logic from OrganizationWrapper to each component #272

Open
nemesifier opened this issue Jun 16, 2021 · 1 comment
Labels
enhancement New feature or request

Comments

@nemesifier
Copy link
Member

nemesifier commented Jun 16, 2021

I suspected that it was wrong to define the redirection logic in OrganizationWrapper but now I think I had the confirmation.
Basically the organization wrapper files decides if the user should be redirected to verification or not and so on.
But that component is re-rendered everytime setLoading() is called.

I found out while working on #170 because I need to redirect the user to the credit card verification which is an external link, and in the automated tests I found out the redirection was called twice.

In reality I found out that multiple HTTP requests were being made to the payment URL, which would have very bad consequences in production because it would initiate multiple payments to the payment gateway.

I think we will have to change this part and move most of that logic in each individual component, this also makes it easier to test it.

Making OrganizationWrapper leaner should also have a beneficial effect on rendering performance because less logic will be executed each time setLoading() is called.

@codesankalp what do you think about this? Do you agree? Or is the current way of doing things right?
We can work on this if there's still time as last thing.

@nemesifier nemesifier added the enhancement New feature or request label Jun 16, 2021
@codesankalp
Copy link
Member

I agree with this, redirection must be handled from the component so that routes defined in the organization wrapper will be more cleaner. Because currently, we check for some conditions and then redirect.
I also face multiple redirections when doing mobile verification, which can also be solved using this.

  • I will also implement proper routings such as AuthRoute and simple Route which will make it better.

I will start working on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants