Skip to content

Commit

Permalink
Fix failure with 201 status code on adding a collaborator (#2559)
Browse files Browse the repository at this point in the history
* fix: return RepositoryInvitation instead of throwing an exception on 201

* fix: TheAddMethod test
  • Loading branch information
annchous authored Sep 6, 2022
1 parent 9c3317f commit 0069239
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public interface IObservableRepoCollaboratorsClient
/// <param name="user">Username of the new collaborator</param>
/// <param name="permission">The permission to set. Only valid on organization-owned repositories.</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
IObservable<bool> Add(string owner, string name, string user, CollaboratorRequest permission);
IObservable<RepositoryInvitation> Add(string owner, string name, string user, CollaboratorRequest permission);

/// <summary>
/// Adds a new collaborator to the repository.
Expand All @@ -195,7 +195,7 @@ public interface IObservableRepoCollaboratorsClient
/// <param name="user">Username of the new collaborator</param>
/// <param name="permission">The permission to set. Only valid on organization-owned repositories.</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
IObservable<bool> Add(long repositoryId, string user, CollaboratorRequest permission);
IObservable<RepositoryInvitation> Add(long repositoryId, string user, CollaboratorRequest permission);

/// <summary>
/// Invites a user as a collaborator to a repository.
Expand Down
4 changes: 2 additions & 2 deletions Octokit.Reactive/Clients/ObservableRepoCollaboratorsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ public IObservable<Unit> Add(string owner, string name, string user)
/// <param name="user">Username of the new collaborator</param>
/// <param name="permission">The permission to set. Only valid on organization-owned repositories.</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
public IObservable<bool> Add(string owner, string name, string user, CollaboratorRequest permission)
public IObservable<RepositoryInvitation> Add(string owner, string name, string user, CollaboratorRequest permission)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Expand Down Expand Up @@ -303,7 +303,7 @@ public IObservable<Unit> Add(long repositoryId, string user)
/// <param name="user">Username of the new collaborator</param>
/// <param name="permission">The permission to set. Only valid on organization-owned repositories.</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
public IObservable<bool> Add(long repositoryId, string user, CollaboratorRequest permission)
public IObservable<RepositoryInvitation> Add(long repositoryId, string user, CollaboratorRequest permission)
{
Ensure.ArgumentNotNullOrEmptyString(user, nameof(user));
Ensure.ArgumentNotNull(permission, nameof(permission));
Expand Down
2 changes: 1 addition & 1 deletion Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ public async Task SurfacesAuthorizationExceptionWhenSpecifyingCollaboratorReques
var connection = Substitute.For<IApiConnection>();
var client = new RepoCollaboratorsClient(connection);

connection.Connection.Put<object>(Arg.Any<Uri>(), Arg.Any<object>()).ThrowsAsync(new AuthorizationException());
connection.Put<RepositoryInvitation>(Arg.Any<Uri>(), Arg.Any<object>()).ThrowsAsync(new AuthorizationException());

await Assert.ThrowsAsync<AuthorizationException>(() => client.Add("owner", "test", "user1", new CollaboratorRequest(Permission.Pull)));
await Assert.ThrowsAsync<AuthorizationException>(() => client.Add(1, "user1", new CollaboratorRequest(Permission.Pull)));
Expand Down
4 changes: 2 additions & 2 deletions Octokit/Clients/IRepoCollaboratorsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public interface IRepoCollaboratorsClient
/// <param name="user">Username of the new collaborator</param>
/// <param name="permission">The permission to set. Only valid on organization-owned repositories.</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
Task<bool> Add(string owner, string name, string user, CollaboratorRequest permission);
Task<RepositoryInvitation> Add(string owner, string name, string user, CollaboratorRequest permission);

/// <summary>
/// Adds a new collaborator to the repository.
Expand All @@ -195,7 +195,7 @@ public interface IRepoCollaboratorsClient
/// <param name="user">Username of the new collaborator</param>
/// <param name="permission">The permission to set. Only valid on organization-owned repositories.</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
Task<bool> Add(long repositoryId, string user, CollaboratorRequest permission);
Task<RepositoryInvitation> Add(long repositoryId, string user, CollaboratorRequest permission);

/// <summary>
/// Invites a new collaborator to the repo
Expand Down
26 changes: 5 additions & 21 deletions Octokit/Clients/RepoCollaboratorsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -292,21 +292,13 @@ public Task Add(string owner, string name, string user)
/// <param name="permission">The permission to set. Only valid on organization-owned repositories.</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
[ManualRoute("PUT", "/repos/{owner}/{repo}/collaborators/{username}")]
public async Task<bool> Add(string owner, string name, string user, CollaboratorRequest permission)
public async Task<RepositoryInvitation> Add(string owner, string name, string user, CollaboratorRequest permission)
{
Ensure.ArgumentNotNullOrEmptyString(owner, nameof(owner));
Ensure.ArgumentNotNullOrEmptyString(name, nameof(name));
Ensure.ArgumentNotNullOrEmptyString(user, nameof(user));

try
{
var response = await Connection.Put<object>(ApiUrls.RepoCollaborator(owner, name, user), permission).ConfigureAwait(false);
return response.HttpResponse.IsTrue();
}
catch (NotFoundException)
{
return false;
}

return await ApiConnection.Put<RepositoryInvitation>(ApiUrls.RepoCollaborator(owner, name, user), permission).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -337,19 +329,11 @@ public Task Add(long repositoryId, string user)
/// <param name="permission">The permission to set. Only valid on organization-owned repositories.</param>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
[ManualRoute("PUT", "/repository/{id}/collaborators/{username}")]
public async Task<bool> Add(long repositoryId, string user, CollaboratorRequest permission)
public async Task<RepositoryInvitation> Add(long repositoryId, string user, CollaboratorRequest permission)
{
Ensure.ArgumentNotNullOrEmptyString(user, nameof(user));

try
{
var response = await Connection.Put<object>(ApiUrls.RepoCollaborator(repositoryId, user), permission).ConfigureAwait(false);
return response.HttpResponse.IsTrue();
}
catch (NotFoundException)
{
return false;
}
return await ApiConnection.Put<RepositoryInvitation>(ApiUrls.RepoCollaborator(repositoryId, user), permission).ConfigureAwait(false);
}

/// <summary>
Expand Down

0 comments on commit 0069239

Please sign in to comment.