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

feat: add membership APIs to manage team members #137

Merged
merged 14 commits into from
Jul 13, 2023
Merged

Conversation

ebk45
Copy link
Contributor

@ebk45 ebk45 commented Jul 11, 2023

What this PR does:

The organisation client has been refactored to better mirror the GitHub API and also match the existing pattern of nested APIs across the client. There is now a Teams Client that is nested under the organisation client which exposes the existing Teams APIs and adds the additional Membership endpoints below:

  • Get a team membership for a user
  • List members in a team
  • Add/Update a membership for a user
  • Delete a membership from a user
  • List pending team invitations

What happens next:

This refactor allows organisation related endpoints to be attached directly to the organisation client whilst keeping the Teams/Memberships endpoints separated.

Tests:

Unit tests have been created for all the above mentioned endpoints.
Fixtures were created using the expected request and response bodies as detailed in the Github Memberships API documentation here: https://docs.github.com/en/rest/teams/members?apiVersion=2022-11-28

@ebk45 ebk45 self-assigned this Jul 11, 2023
@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Merging #137 (b40ca62) into master (4f3214d) will increase coverage by 0.70%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #137      +/-   ##
============================================
+ Coverage     74.23%   74.94%   +0.70%     
- Complexity      263      272       +9     
============================================
  Files            41       42       +1     
  Lines           920      946      +26     
  Branches         41       41              
============================================
+ Hits            683      709      +26     
  Misses          212      212              
  Partials         25       25              
Impacted Files Coverage Δ
...va/com/spotify/github/v3/clients/GitHubClient.java 76.51% <100.00%> (+0.38%) ⬆️
.../spotify/github/v3/clients/OrganisationClient.java 85.71% <100.00%> (-9.53%) ⬇️
...java/com/spotify/github/v3/clients/TeamClient.java 100.00% <100.00%> (ø)

@ebk45 ebk45 marked this pull request as ready for review July 12, 2023 13:25
Comment on lines +53 to +54
public TeamClient createTeamClient(final GitHubClient github, final String org) {
return TeamClient.create(github, org);
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a breaking change from the earlier version 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I had the same thought, but I think it's fine since we just released the code that we're breaking here last week (#135), therefore nobody is using it yet probably.

Copy link
Member

Choose a reason for hiding this comment

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

Good point that hopefully no one is using it. Can we also unpublish the earlier version somehow? In case someone decide to use it in future?
Other than that I agree it's not worth the effort to fix the breakage.

@ebk45 ebk45 merged commit 7a2276a into master Jul 13, 2023
@ebk45 ebk45 deleted the feat/members-api branch July 13, 2023 12:43
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.

3 participants