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-498007: add search parameter to groups endpoint #133

Closed
wants to merge 6 commits into from
Closed

OKTA-498007: add search parameter to groups endpoint #133

wants to merge 6 commits into from

Conversation

bryanapellanes-okta
Copy link
Contributor

@bryanapellanes-okta bryanapellanes-okta commented Jun 1, 2022 •
edited
Loading

Adds the search parameter to align with documented parameters, see request parameters section of https://developer.okta.com/docs/reference/api/groups/#list-groups

arvindkrishnakumar-okta
arvindkrishnakumar-okta approved these changes Jun 1, 2022
View reviewed changes
Copy link
Contributor

@arvindkrishnakumar-okta arvindkrishnakumar-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

monde
monde approved these changes Jun 1, 2022
View reviewed changes
Copy link
Collaborator

@monde monde 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 giving the discussion in https://oktainc.atlassian.net/browse/OKTA-498007 and https://oktainc.atlassian.net/browse/OKTA-406629

laura-rodriguez
laura-rodriguez reviewed Jun 1, 2022
View reviewed changes
Copy link
Collaborator

@laura-rodriguez laura-rodriguez left a comment

Choose a reason for hiding this comment

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

@bryanapellanes-okta can you please add more details of what we're solving here? Links to the API docs and a draft PR of the .NET SDK might help. Thank you 🙏

@bryanapellanes-okta
Copy link
Contributor Author

bryanapellanes-okta commented Jun 2, 2022 •
edited
Loading

@bryanapellanes-okta can you please add more details of what we're solving here? Links to the API docs and a draft PR of the .NET SDK might help. Thank you 🙏

Updated description; this change adds back the search parameter as described in this JIRA: https://oktainc.atlassian.net/browse/OKTA-498007.

@bryanapellanes-okta
Copy link
Contributor Author

Working on draft of regenerated Sdk now, will provide PR link shortly.

@bryanapellanes-okta
Copy link
Contributor Author

@laura-rodriguez draft PR for regenerated dotnet Sdk: okta/okta-sdk-dotnet#552

@monde monde mentioned this pull request Jun 2, 2022
Closed
12 tasks
@laura-rodriguez
Copy link
Collaborator

Thank you @bryanapellanes-okta! This LGTM. Thinking if we should keep search in the same order as before just to avoid a breaking change in the SDK. SDKs can still mitigate it if we decide to continue with this order. I'll let you decide.

laura-rodriguez
laura-rodriguez approved these changes Jun 2, 2022
View reviewed changes
Copy link
Collaborator

@laura-rodriguez laura-rodriguez left a comment

Choose a reason for hiding this comment

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

LGTM

@bryanapellanes-okta bryanapellanes-okta changed the base branch from master to 2.13 June 3, 2022 16:46
@bryanapellanes-okta bryanapellanes-okta changed the base branch from 2.13 to master June 3, 2022 16:48
@bretterer bretterer closed this Jun 16, 2022
@bretterer bretterer reopened this Jun 16, 2022
@bryanapellanes-okta
Copy link
Contributor Author

Closing to reopen rebased on current master

@bryanapellanes-okta bryanapellanes-okta deleted the OKTA-498007-add-search-param branch June 16, 2022 18:37
@bryanapellanes-okta bryanapellanes-okta mentioned this pull request Jun 16, 2022
Merged
@bryanapellanes-okta bryanapellanes-okta mentioned this pull request Oct 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@monde monde monde approved these changes

@laura-rodriguez laura-rodriguez laura-rodriguez approved these changes

@arvindkrishnakumar-okta arvindkrishnakumar-okta arvindkrishnakumar-okta approved these changes

@bretterer bretterer Awaiting requested review from bretterer bretterer is a code owner

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

Successfully merging this pull request may close these issues.
6 participants
@bryanapellanes-okta @laura-rodriguez @monde @arvindkrishnakumar-okta @bretterer @stevenwong-okta