Skip to content

Conversation

@ash-wright123
Copy link
Collaborator

@ash-wright123 ash-wright123 commented Mar 28, 2025

Issue

https://mxcom.atlassian.net/browse/CT-1286

Changes

Added the connect consent screen behind a post hog feature flag.

Testing Instructions

@ash-wright123 ash-wright123 added the Needs Review This item needs review label Mar 31, 2025
@mwclemy
Copy link
Collaborator

mwclemy commented Apr 3, 2025

I don't see the back button on the consent screen. Shouldn't it show up by default? Also, after clicking the "I Consent" button, it correctly navigates to the credentials page. However, the back button on the credentials page takes me back to the search institution screen, which doesn’t seem right. I would expect it to navigate back to the consent screen if it exists.

@mwclemy
Copy link
Collaborator

mwclemy commented Apr 3, 2025

Other than my comments above, everything else looks good to me.

@ash-wright123
Copy link
Collaborator Author

Hmm the back button shows up when I run it:
Screenshot 2025-04-07 at 12 43 19 PM

mwclemy
mwclemy previously approved these changes Apr 7, 2025
Copy link
Collaborator

@mwclemy mwclemy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@mwclemy mwclemy removed the Needs Review This item needs review label Apr 7, 2025
@mwclemy mwclemy added the Needs Review This item needs review label Apr 11, 2025
@mwclemy
Copy link
Collaborator

mwclemy commented Apr 11, 2025

I've also noticed you have two failing tests.

@ash-wright123
Copy link
Collaborator Author

#93 (comment)
Will fix the tests that are failing.

Copy link
Collaborator

@mwclemy mwclemy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mwclemy mwclemy removed the Needs Review This item needs review label Apr 11, 2025
@ash-wright123 ash-wright123 merged commit a6d4d96 into master Apr 11, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants