Skip to content

feat: Add ListProvisionedScimGroupsForEnterprise inside SCIM service with test - #3467

Merged
gmlewis merged 5 commits into
google:masterfrom
cyberious:master
Feb 12, 2025
Merged

feat: Add ListProvisionedScimGroupsForEnterprise inside SCIM service with test#3467
gmlewis merged 5 commits into
google:masterfrom
cyberious:master

Conversation

@cyberious

Copy link
Copy Markdown
Contributor

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:

  • Added SCIMGroupAttributes type to represent supported SCIM group attributes, including DisplayName, Members, Schemas, ExternalID, ID, and Meta. ([github/scim.goR20-R38](https://github.com/google/go-github/pull/3467/files#diff-c2dda2100fd0c700da11a5d96582391b4923658db7e717627d248ae1c029c5fbR20-R38))
  • Introduced SCIMProvisionedGroups type to represent the result of calling ListSCIMProvisionedIdentities, including Schemas, TotalResults, ItemsPerPage, StartIndex, and Resources. ([github/scim.goR78-R86](https://github.com/google/go-github/pull/3467/files#diff-c2dda2100fd0c700da11a5d96582391b4923658db7e717627d248ae1c029c5fbR78-R86))
  • Added 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:

  • Added TestSCIMService_ListSCIMProvisionedGroups function to test the new ListProvisionedScimGroupsForEnterprise 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))

@codecov

codecovBot commented Feb 8, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.04%. Comparing base (ce42642) to head (3781247).
Report is 1 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

@gmlewis

Copy link
Copy Markdown
Collaborator

Please run step 4 of CONTRIBUTING.md and push the results (./script/generate.sh, ./script/lint.sh, and ./script/test.sh).

@gmlewisgmlewis added the NeedsReview PR is awaiting a review before merging. label Feb 8, 2025
@cyberious

cyberious commented Feb 8, 2025

Copy link
Copy Markdown
ContributorAuthor

@gmlewis updated it. I apologize for not seeing that earlier.

Squashed into the same commit

@cyberiouscyberious changed the title Add ListProvisionedScimGroupsForEnterprise inside SCIM service with testfeat: Add ListProvisionedScimGroupsForEnterprise inside SCIM service with testFeb 8, 2025
@gmlewis

Copy link
Copy Markdown
Collaborator

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.
However, for this tiny PR, it really makes no difference, so no big deal... but just wanted to let you know in case you make larger PRs in the future, which are totally welcome. 😄 ❤️

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

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.

Comment threadgithub/scim.go
@googlegoogle deleted a comment from ohpintu90Feb 9, 2025

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

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!

@stevehipwellstevehipwell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor nit/question.

Comment threadgithub/scim.go Outdated

@stevehipwellstevehipwell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment threadgithub/scim.go Outdated
cyberiousand others added 2 commits February 11, 2025 15:25
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

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.

@gmlewisgmlewis removed the NeedsReview PR is awaiting a review before merging. label Feb 11, 2025
@gmlewis
gmlewis merged commit 77684a4 into google:masterFeb 12, 2025
jlaportebot added a commit to jlaportebot/go-github that referenced this pull request Jun 28, 2026
Sign up for freeto 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.

3 participants

@cyberious@gmlewis@stevehipwell