-
Notifications
You must be signed in to change notification settings - Fork 19
Conversation
We need to destroy the state received from backend on API call to `/api/v1/connectors/{id}/credentials` in order for it not to overwrite the cookie received on `/api/v1/credentials/callback`. This eliminates the need to parse the cookie in `oauth-redirect.html` and pass/store it in the app.
PR Storybook available here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the take, now I see the problem!
@@ -97,8 +97,10 @@ export const ConfigurationPage: React.FunctionComponent = () => { | |||
* the API in the document. This cookie will be later used by the BE in the | |||
* redirect page set up in the 3rd party. | |||
*/ | |||
if (connectResource) { | |||
if (connectResource && connectResource.state) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see the logic but the fix is not the mutation (never mutate an object in js), but rather wrapping this code in an useEffect to avoid it run on subsequent renders. I can do this in the next few if you want!
improvement: handle setting of the cookie in its own effect
PR Storybook available here |
chore: update the comments to reflect the new improved flow
PR Storybook available here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Pull request approved by @riccardo-forina - applying approved label |
We need to destroy the state received from backend on API call to
/api/v1/connectors/{id}/credentials
in order for it not to overwrite the cookie received on/api/v1/credentials/callback
. This eliminates the need to parse the cookie inoauth-redirect.html
and pass/store it in the app.