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

Fix failure with 201 status code on adding a collaborator #2559

Merged
merged 2 commits into from
Sep 6, 2022
Merged

Fix failure with 201 status code on adding a collaborator #2559

merged 2 commits into from
Sep 6, 2022

Conversation

annchous
Copy link
Contributor

@annchous annchous commented Sep 3, 2022

Close #2558 and close #2373 too.

I wrote such a local test:

[Fact]
public async Task AddCollaborator_ThrowsNoException()
{
    GitHubClient client = new GitHubClient(new ProductHeaderValue("name"))
    {
        Credentials = new Credentials("token")
    };

    var response = await client.Repository.Collaborator.Add("owner", "name", "user", new CollaboratorRequest(Permission.Admin));
}

There was no exception and response was valid. An old version of this method throws ApiException (this case I have also covered with the corresponding test).

@FrediKats
Copy link
Contributor

@timrogers can you check plz?

@JonruAlveus
Copy link
Collaborator

release_notes: Fix failure with 201 status code on adding a collaborator

@JonruAlveus
Copy link
Collaborator

Hi @annchous, I've run the tests but there's a failure. Please could you check it out?

Octokit.Tests.Clients.RepoCollaboratorsClientTests+TheAddMethod.SurfacesAuthorizationExceptionWhenSpecifyingCollaboratorRequest

Cheers

@annchous
Copy link
Contributor Author

annchous commented Sep 4, 2022

@JonruAlveus There was a problem in mock configuration. Now we use ApiConnection instead of Connection. Now test passes.

@timrogers
Copy link
Contributor

timrogers commented Sep 5, 2022

This is looking great to me! ✅ Are you happy too @JonruAlveus?

We'll need to release this in a new major version as a breaking change since it changes the return type of a method.

@JonruAlveus JonruAlveus merged commit 0069239 into octokit:main Sep 6, 2022
@nickfloyd
Copy link
Contributor

release_notes: Fixed a failure with 201 status code when adding a collaborator

@nickfloyd nickfloyd added Type: Breaking change Used to note any change that requires a major version bump Type: Bug Something isn't working as documented and removed category: breaking-change labels Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Breaking change Used to note any change that requires a major version bump Type: Bug Something isn't working as documented
Projects
None yet
5 participants