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.

Sign up for GitHub

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

Jump to bottom

OKTA-289659: Add Event Hooks Integration Tests #399

Merged
merged 15 commits into from
Jun 17, 2020
Merged

OKTA-289659: Add Event Hooks Integration Tests #399

merged 15 commits into from
Jun 17, 2020

Conversation

arvindkrishnakumar-okta
Copy link
Contributor

No description provided.

bdemers
bdemers suggested changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@bdemers bdemers 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!
one nit, one question about the enum class, a couple of comments/ideas about builder classes to make the code more approachable (and less code to document)

@bdemers
Copy link
Contributor

bdemers commented Jun 8, 2020

@arvindkrishnakumar-okta OH, one other thing. It doesn't look like we have a test for assigning an IT with a policy.

We have an older test over in okta/okta-hooks-sdk-java: https://github.com/okta/okta-hooks-sdk-java/blob/master/integration-tests/src/test/groovy/com/okta/hooks/sdk/its/RegistrationInlineHookIT.groovy#L121-L173
that creates a policy with the inline hook.

This uses an older "non-public" API, so I hope there is a newer equivalent.

We also have an OAuth based hook IT, that uses a policy rule:
https://github.com/okta/okta-hooks-sdk-java/blob/master/integration-tests/src/test/groovy/com/okta/hooks/sdk/its/OAuthTokenInlineHookIT.groovy#L358-L364

@arvindkrishnakumar-okta
Copy link
Contributor Author

arvindkrishnakumar-okta commented Jun 16, 2020 •
edited
Loading

@arvindkrishnakumar-okta OH, one other thing. It doesn't look like we have a test for assigning an IT with a policy.

We have an older test over in okta/okta-hooks-sdk-java: https://github.com/okta/okta-hooks-sdk-java/blob/master/integration-tests/src/test/groovy/com/okta/hooks/sdk/its/RegistrationInlineHookIT.groovy#L121-L173
that creates a policy with the inline hook.

This uses an older "non-public" API, so I hope there is a newer equivalent.

We also have an OAuth based hook IT, that uses a policy rule:
https://github.com/okta/okta-hooks-sdk-java/blob/master/integration-tests/src/test/groovy/com/okta/hooks/sdk/its/OAuthTokenInlineHookIT.groovy#L358-L364

Noted, will add that as part of InlineHooks IT (#400).

@arvindkrishnakumar-okta arvindkrishnakumar-okta force-pushed the okta-289659-add-event-hooks-integration-tests branch from 91333f6 to 7a8bb6f Compare June 16, 2020 20:35
bdemers
bdemers reviewed Jun 17, 2020
View reviewed changes
integration-tests/src/test/groovy/com/okta/sdk/tests/it/UserTypesIT.groovy
@@ -15,7 +15,7 @@
*/
package com.okta.sdk.tests.it

import com.okta.sdk.resource.UserType
import com.okta.sdk.resource.user.type.UserType
Copy link
Contributor

Choose a reason for hiding this comment

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

user.type seems a bit redundant, maybe just tag it with user?
(maybe there are a bunch of other user type objects 🤷)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed :)

UserType and UserTypeList are the only classes in package com.okta.sdk.resource.user.type.

image

Will consider moving it to a different package (say com.okta.sdk.resource.user) as the user.type seems redundant. Will do this as part of separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://oktainc.atlassian.net/browse/OKTA-305898 created to address this improvement.

bdemers
bdemers approved these changes Jun 17, 2020
View reviewed changes
Copy link
Contributor

@bdemers bdemers left a comment

Choose a reason for hiding this comment

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

One nit, and one comment (unrelated to this PR)

@arvindkrishnakumar-okta
Copy link
Contributor Author

One nit, and one comment (unrelated to this PR)

addressed this.

@arvindkrishnakumar-okta arvindkrishnakumar-okta merged commit 99e9de1 into dev_openapi_v2.0.0_major_rel Jun 17, 2020
@arvindkrishnakumar-okta arvindkrishnakumar-okta deleted the okta-289659-add-event-hooks-integration-tests branch June 17, 2020 18:25
arvindkrishnakumar-okta added a commit that referenced this pull request Jul 7, 2020
@arvindkrishnakumar-okta @ivanezeigbo-okta @bdemers
OKTA-302317: Release Java Mgmt SDK v2.0.0 (#422)
03cabad 
* upgrade to openapi v2.0.0 (#380)

* openapi spec update - remove 200 response from generateCsrForApplication

* Update OktaOrgCleaner.groovy

* Fix flaky GroupsIT#groupUserOperationsTest test

* Update GroupsIT.groovy

* OKTA-279039: Fix flaky ApplicationsIT associateUserWithApplication test

* Update GroupsIT.groovy

* Addressed review comment

* OKTA-289656: Add Applications API Integration Tests

* OKTA-289655: Add Admin role API Integration Tests

* Open API Csr rename update (#394)

* openapi csr rename update

* Merge pull request #396

* revert csr model rename (#398)

* revert csr model rename

* open api spec update (#401)

* open api spec update

* Open API spec - fix binding enum value for ProtocolEndpoint model (#406)

* OKTA-289662: Add SMS Templates API Integration Tests (#395)

* OKTA-289665: Add Linked Objects Integration Tests (#404)

* Updated Licence Header in LinkedObjectsIT (#410)

* OKTA-289668: Add User Type Integration Tests (#405)

* Open API Update - Sync with #238 #239  (#411)

* sync with open api pr #240 (#413)

* Open API Update - Sync with #241 (#412)

* Open API Update - Sync with #242 (#414)

* Open API Update - Sync with Open API PR 243 (#416)

* OKTA-289657: Add Factors API Integration Tests (#391)

* OKTA-289659: Add Event Hooks Integration Tests (#399)

* OKTA-289659: Add Authorization Server Integration Tests (#402)

* OKTA-289660: Add Inline Hooks Integration Tests (#400)

* Open API Update - Sync with Open API PR 244 (#418)

* fixed copy paste error

* OKTA-289661: Add Feature API Integration Tests (#397)

* Open API Update - Sync with Open API PR 249 (#420)

* OKTA-289663: Add IdP Integration Tests (#409)

* resolved merge conflicts with master changes

* removed Default prefix from few Idp builders

* added swagger code gen check for backward compatibility requirements per review comments

* fixed findbugs error

* updates per swagger regen code and one other update to Idp builder

* fixed expiresAt param type in spec

* deprecated old classes

* refactored InlineHooksIT - deactivate created hooks so it can be cleaned up after test

* Remove deprecated code (#426)

* Remove deprecated code

* fix typo

* fixed PMD violation - unused inports

* Update integration-tests/src/test/groovy/com/okta/sdk/tests/it/CrudTestSupport.groovy

Co-authored-by: Brian Demers <bdemers@apache.org>

* Update integration-tests/src/test/groovy/com/okta/sdk/tests/it/FactorsIT.groovy

Co-authored-by: Brian Demers <bdemers@apache.org>

* Update integration-tests/src/test/groovy/com/okta/sdk/tests/it/UsersIT.groovy

Co-authored-by: Brian Demers <bdemers@apache.org>

* Update integration-tests/src/test/groovy/com/okta/sdk/tests/it/GroupsIT.groovy

Co-authored-by: Brian Demers <bdemers@apache.org>

* minor improvements (#427)

* OKTA-308935: Create Migration Guide for Java Mgmt SDK v2.0.0 (#423)

Co-authored-by: Ivan Ezeigbo <56391095+ivanezeigbo-okta@users.noreply.github.com>
Co-authored-by: Brian Demers <bdemers@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdemers bdemers bdemers approved these changes

@bretterer bretterer bretterer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@arvindkrishnakumar-okta @bdemers @bretterer