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

Add support to pass callback to poll function #252

Merged
merged 1 commit into from
Sep 24, 2019

Conversation

magizh-okta
Copy link
Contributor

  • If a callback is passed to polling function during mfa challenge, the callback is invoked on
    every successful poll with the transaction as argument.

  • This is needed to know if the response info changed during poll.
    eg) OV risk scoring number challenge flow.

https://okta.app.box.com/s/yhybly12s4jne89fsoc41rgkqxpmd2g8

* If a callback is passed to polling function during mfa challenge, the callback is invoked on
every successful poll with the transaction as argument.

* This is needed to know if the response info changed during poll.
eg) OV risk scoring number challenge flow.
@@ -0,0 +1,82 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

Choose a reason for hiding this comment

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

is this the first test using jest? can we have jasmine and jest in this repo with no prob? or should we stick to jasmine for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think all the tests are moved to jest already. I guess this is the first one using snapshots.

The jasmine based tests are under https://github.com/okta/okta-auth-js/tree/master/test/karma

https://github.com/okta/okta-auth-js/pull/252/files#diff-9dd9cfed8da8bb5fe5cd6c5cddb0785fR1

Copy link

Choose a reason for hiding this comment

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

looks good to me

},
expectations: function (test) {
expect(test.transactionCallbackFn.calls.count()).toBe(2);
expect(test.transactionCallbackFn.calls.argsFor(0)[0]).toMatchSnapshot();
Copy link
Contributor

Choose a reason for hiding this comment

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

I have not yet grokked util.itMakesCorrectRequestResponse yet, but why test for two calls and then only match the snapshot for one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the responses that we get during the poll is same in this case as this is just polling.

Copy link
Contributor

@aarongranick-okta aarongranick-okta left a comment

Choose a reason for hiding this comment

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

lgtm

@magizh-okta magizh-okta merged commit 81cdeea into okta:master Sep 24, 2019
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