-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat: Add ListProvisionedScimGroupsForEnterprise inside SCIM service with test #3467
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3467 +/- ##
=======================================
Coverage 91.03% 91.04%
=======================================
Files 179 179
Lines 15538 15551 +13
=======================================
+ Hits 14145 14158 +13
Misses 1221 1221
Partials 172 172 ☔ View full report in Codecov by Sentry. |
Please run step 4 of CONTRIBUTING.md and push the results ( |
@gmlewis updated it. I apologize for not seeing that earlier. Squashed into the same commit |
Just FYI - As described in CONTRIBUTING.md, there is no need to force-push in this repo, as we always squash-and-merge so that the commit history is clean. In fact, we discourage it because it makes it more challenging for reviewers to see what changed since their last review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @cyberious !
One minor tweak, please, then awaiting second LGTM+Approval from any other contributor to this repo before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @cyberious !
LGTM
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
@stevehipwell - might you have time for a code review? Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit/question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Glenn Lewis <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @cyberious and @stevehipwell !
LGTM
Merging.
This pull request introduces several enhancements to the SCIM (System for Cross-domain Identity Management) functionality in the
github/scim.go
file. The key changes include the addition of new types to represent SCIM group attributes and provisioned groups, as well as a new method to list provisioned SCIM groups for an enterprise. Additionally, a corresponding test function has been added to ensure the correctness of the new method.Enhancements to SCIM functionality:
SCIMGroupAttributes
type to represent supported SCIM group attributes, includingDisplayName
,Members
,Schemas
,ExternalID
,ID
, andMeta
. ([github/scim.goR20-R38](https://github.com/google/go-github/pull/3467/files#diff-c2dda2100fd0c700da11a5d96582391b4923658db7e717627d248ae1c029c5fbR20-R38)
)SCIMProvisionedGroups
type to represent the result of callingListSCIMProvisionedIdentities
, includingSchemas
,TotalResults
,ItemsPerPage
,StartIndex
, andResources
. ([github/scim.goR78-R86](https://github.com/google/go-github/pull/3467/files#diff-c2dda2100fd0c700da11a5d96582391b4923658db7e717627d248ae1c029c5fbR78-R86)
)ListProvisionedScimGroupsForEnterprise
method to list provisioned SCIM groups for an enterprise, including the necessary API documentation reference. ([github/scim.goR248-R269](https://github.com/google/go-github/pull/3467/files#diff-c2dda2100fd0c700da11a5d96582391b4923658db7e717627d248ae1c029c5fbR248-R269)
)Testing:
TestSCIMService_ListSCIMProvisionedGroups
function to test the newListProvisionedScimGroupsForEnterprise
method, ensuring it handles responses correctly and validates various scenarios. ([github/scim_test.goR124-R217](https://github.com/google/go-github/pull/3467/files#diff-2723dd506a61e0f7abe849217ded7c4aa5dfef10455f14ed10a423c65f4dac49R124-R217)
)