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

OKTA-747278 : Enable polling on OV same device enroll setup view #3699

Merged

Conversation

leemchale-okta
Copy link
Contributor

@leemchale-okta leemchale-okta commented Aug 22, 2024

Description:

This PR enables polling on the OV Same device enrollment setup screen. This allows the SIW to receive the updated response from the backend when enrollment has been completed in the OV app.

Related back-end change: https://github.com/atko-eng/okta-core/pull/100424

PR Checklist

Issue:

Reviewers:

Screenshot/Video:

Downstream Monolith Build:

} else if (contextualData.devicebootstrap?.setupOVUrl) {
deviceMap = contextualData.devicebootstrap;
deviceMap = {...contextualData.devicebootstrap};
Copy link
Contributor Author

@leemchale-okta leemchale-okta Aug 27, 2024

Choose a reason for hiding this comment

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

make a copy instead of editing the original transaction contextualData in appState, since this was causing the prevTransaction and new transaction to not be identical, causing the SIW to re-render on every poll.

@leemchale-okta leemchale-okta force-pushed the LM-same-device-enroll-add-polling-OKTA-747278 branch from 6e8d8c2 to 17cc5c4 Compare August 27, 2024 23:27
@leemchale-okta leemchale-okta marked this pull request as ready for review August 29, 2024 00:26
@leemchale-okta leemchale-okta changed the title enable polling on same device enroll view OKTA-747278 : Enable polling on same device enroll view Aug 29, 2024
@leemchale-okta leemchale-okta changed the title OKTA-747278 : Enable polling on same device enroll view OKTA-747278 : Enable polling on OV same device enroll setup view Aug 29, 2024
@@ -65,8 +65,7 @@ const Body = BaseFormWithPolling.extend(Object.assign(
shouldStartPolling = true;
} else if (['samedevice', 'devicebootstrap'].includes(selectedChannel)) {
// no selector if the channel is same device or device bootstrap
// additionally, stop polling as it should be a terminal page
shouldStartPolling = false;
shouldStartPolling = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

let't separate out same device and device bootstrap here. OV2OV is not shipping out with this together, so we should keep deviceBootstrap polling as false.
Also, let's make a follow up Jira to get test for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@trevoring-okta
Copy link
Contributor

Looks like you closed https://github.com/atko-eng/okta-core/pull/99376, will flipping that shouldStartPolling flag to true cause issues without a corresponding backend change or is that fine?

@leemchale-okta
Copy link
Contributor Author

Looks like you closed https://github.com/atko-eng/okta-core/pull/99376, will flipping that shouldStartPolling flag to true cause issues without a corresponding backend change or is that fine?

@trevoring-okta oh I closed that PR and I made a new PR here: https://github.com/atko-eng/okta-core/pull/100424. Thanks for catching that I updated the description.

trevoring-okta
trevoring-okta previously approved these changes Aug 29, 2024
Copy link
Contributor

@trevoring-okta trevoring-okta left a comment

Choose a reason for hiding this comment

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

Looks fine to me from a SIW POV

@oktapp-aperture-okta oktapp-aperture-okta bot merged commit fb12a83 into master Aug 29, 2024
3 checks passed
@oktapp-aperture-okta oktapp-aperture-okta bot deleted the LM-same-device-enroll-add-polling-OKTA-747278 branch August 29, 2024 21:42
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.

4 participants