Skip to content

Conversation

@ebk45
Copy link
Contributor

@ebk45 ebk45 commented Sep 27, 2023

What does this PR do?

Exposes the ability to PUT and GET organisation members

How to test?

Unit tests have been updated to reflect these changes

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #150 (b5112dd) into master (1ffd719) will increase coverage by 0.19%.
Report is 3 commits behind head on master.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #150      +/-   ##
============================================
+ Coverage     75.64%   75.83%   +0.19%     
- Complexity      279      284       +5     
============================================
  Files            42       42              
  Lines           969      985      +16     
  Branches         42       43       +1     
============================================
+ Hits            733      747      +14     
- Misses          211      212       +1     
- Partials         25       26       +1     
Files Coverage Δ
.../spotify/github/v3/clients/OrganisationClient.java 92.85% <100.00%> (+7.14%) ⬆️

... and 1 file with indirect coverage changes

@Test
public void updateMembership() throws Exception {
final OrgMembershipCreate orgMembershipCreateRequest =
json.fromJson(getFixture("membership_update.json"), OrgMembershipCreate.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this fixture included in resources. Does it already exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this already exists, both the team memberships and org memberships have the same request schema so it didn't seem worth it to create a new file with the same content. They have different responses though, so that's been reflect with the new file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for clarifying.

public CompletableFuture<OrgMembership> updateOrgMembership(final OrgMembershipCreate request, final String username) {
final String path = String.format(MEMBERSHIP_TEMPLATE, org, username);
log.debug("Updating membership in org: " + path);
return github.put(path, github.json().toJsonUnchecked(request), OrgMembership.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we're using json unchecked here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly was just following what was already being used in other tests.

@annelled annelled merged commit a01095b into master Sep 27, 2023
@annelled annelled deleted the org-members branch September 27, 2023 12:24
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.

4 participants