From 4850b839067b39dc10a500812fded54b75d6bc98 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sat, 4 Jun 2016 23:35:23 +1000 Subject: [PATCH 1/2] Add Permissions attribute to User Update integration tests to ensure Permissions field is populated Add Accept header for this functionality, so it works on GHE 2.5 Fix impacted URL unit tests --- .../Clients/RepositoryCollaboratorClientTests.cs | 2 ++ .../Reactive/ObservableRepositoryCollaboratorClientTests.cs | 2 ++ Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs | 4 ++-- Octokit/Clients/RepoCollaboratorsClient.cs | 2 +- Octokit/Helpers/AcceptHeaders.cs | 4 +++- Octokit/Models/Response/User.cs | 5 ++++- 6 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/RepositoryCollaboratorClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryCollaboratorClientTests.cs index abfff650..1035ab35 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryCollaboratorClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryCollaboratorClientTests.cs @@ -24,6 +24,8 @@ public class RepositoryCollaboratorClientTests var collaborators = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName); Assert.NotNull(collaborators); Assert.Equal(2, collaborators.Count); + Assert.NotNull(collaborators[0].Permissions); + Assert.NotNull(collaborators[1].Permissions); } } diff --git a/Octokit.Tests.Integration/Reactive/ObservableRepositoryCollaboratorClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableRepositoryCollaboratorClientTests.cs index c6fc5464..f26eff9a 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableRepositoryCollaboratorClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableRepositoryCollaboratorClientTests.cs @@ -26,6 +26,8 @@ public class ObservableRepositoryCollaboratorClientTests var collaborators = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName).ToList(); Assert.NotNull(collaborators); Assert.Equal(2, collaborators.Count); + Assert.NotNull(collaborators[0].Permissions); + Assert.NotNull(collaborators[1].Permissions); } } diff --git a/Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs b/Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs index 5c9921d4..d79c82a8 100644 --- a/Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs +++ b/Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs @@ -32,7 +32,7 @@ namespace Octokit.Tests.Clients var client = new RepoCollaboratorsClient(connection); client.GetAll("owner", "test"); - connection.Received().GetAll(Arg.Is(u => u.ToString() == "repos/owner/test/collaborators"), Args.ApiOptions); + connection.Received().GetAll(Arg.Is(u => u.ToString() == "repos/owner/test/collaborators"), null, "application/vnd.github.ironman-preview+json", Args.ApiOptions); } [Fact] @@ -51,7 +51,7 @@ namespace Octokit.Tests.Clients client.GetAll("owner", "test", options); connection.Received() - .GetAll(Arg.Is(u => u.ToString() == "repos/owner/test/collaborators"), options); + .GetAll(Arg.Is(u => u.ToString() == "repos/owner/test/collaborators"), null, "application/vnd.github.ironman-preview+json", options); } [Fact] diff --git a/Octokit/Clients/RepoCollaboratorsClient.cs b/Octokit/Clients/RepoCollaboratorsClient.cs index d72ea217..cf1f5a35 100644 --- a/Octokit/Clients/RepoCollaboratorsClient.cs +++ b/Octokit/Clients/RepoCollaboratorsClient.cs @@ -57,7 +57,7 @@ namespace Octokit Ensure.ArgumentNotNullOrEmptyString(repo, "repo"); Ensure.ArgumentNotNull(options, "options"); - return ApiConnection.GetAll(ApiUrls.RepoCollaborators(owner, repo), options); + return ApiConnection.GetAll(ApiUrls.RepoCollaborators(owner, repo), null, AcceptHeaders.OrganizationPermissionsPreview, options); } /// diff --git a/Octokit/Helpers/AcceptHeaders.cs b/Octokit/Helpers/AcceptHeaders.cs index 3d448fb7..f08bc8e5 100644 --- a/Octokit/Helpers/AcceptHeaders.cs +++ b/Octokit/Helpers/AcceptHeaders.cs @@ -8,6 +8,8 @@ public const string RedirectsPreviewThenStableVersionJson = "application/vnd.github.quicksilver-preview+json; charset=utf-8, application/vnd.github.v3+json; charset=utf-8"; + public const string OrganizationPermissionsPreview = "application/vnd.github.ironman-preview+json"; + public const string LicensesApiPreview = "application/vnd.github.drax-preview+json"; public const string ProtectedBranchesApiPreview = "application/vnd.github.loki-preview+json"; @@ -20,6 +22,6 @@ public const string SquashCommitPreview = "application/vnd.github.polaris-preview+json"; - public const string MigrationsApiPreview = " application/vnd.github.wyandotte-preview+json"; + public const string MigrationsApiPreview = "application/vnd.github.wyandotte-preview+json"; } } diff --git a/Octokit/Models/Response/User.cs b/Octokit/Models/Response/User.cs index 5c7c515e..2cd22753 100644 --- a/Octokit/Models/Response/User.cs +++ b/Octokit/Models/Response/User.cs @@ -13,14 +13,17 @@ namespace Octokit { public User() { } - public User(string avatarUrl, string bio, string blog, int collaborators, string company, DateTimeOffset createdAt, int diskUsage, string email, int followers, int following, bool? hireable, string htmlUrl, int totalPrivateRepos, int id, string location, string login, string name, int ownedPrivateRepos, Plan plan, int privateGists, int publicGists, int publicRepos, string url, bool siteAdmin, string ldapDistinguishedName, DateTimeOffset? suspendedAt) + public User(string avatarUrl, string bio, string blog, int collaborators, string company, DateTimeOffset createdAt, int diskUsage, string email, int followers, int following, bool? hireable, string htmlUrl, int totalPrivateRepos, int id, string location, string login, string name, int ownedPrivateRepos, Plan plan, int privateGists, int publicGists, int publicRepos, string url, RepositoryPermissions permissions, bool siteAdmin, string ldapDistinguishedName, DateTimeOffset? suspendedAt) : base(avatarUrl, bio, blog, collaborators, company, createdAt, diskUsage, email, followers, following, hireable, htmlUrl, totalPrivateRepos, id, location, login, name, ownedPrivateRepos, plan, privateGists, publicGists, publicRepos, AccountType.User, url) { + Permissions = permissions; SiteAdmin = siteAdmin; LdapDistinguishedName = ldapDistinguishedName; SuspendedAt = suspendedAt; } + public RepositoryPermissions Permissions { get; protected set; } + /// /// Whether or not the user is an administrator of the site /// From d02007d2b6517a9536ad44249224d79660ef6c5a Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sat, 4 Jun 2016 23:40:05 +1000 Subject: [PATCH 2/2] Permissions attribute was already present on Repository Ensure Teams.GetAllRepositories() calls pass the preview header and add integration tests to assert the permissions property is populated --- .../Clients/ObservableTeamsClient.cs | 2 +- .../Helpers/ConnectionExtensions.cs | 9 ++++ .../Clients/TeamsClientTests.cs | 33 ++++++++++++++- .../Reactive/ObservableTeamsClientTests.cs | 42 ++++++++++++++++--- Octokit.Tests/Clients/TeamsClientTests.cs | 4 +- Octokit/Clients/TeamsClient.cs | 17 +------- 6 files changed, 82 insertions(+), 25 deletions(-) diff --git a/Octokit.Reactive/Clients/ObservableTeamsClient.cs b/Octokit.Reactive/Clients/ObservableTeamsClient.cs index 31f9b7aa..be061f3c 100644 --- a/Octokit.Reactive/Clients/ObservableTeamsClient.cs +++ b/Octokit.Reactive/Clients/ObservableTeamsClient.cs @@ -224,7 +224,7 @@ namespace Octokit.Reactive { Ensure.ArgumentNotNull(options, "options"); - return _connection.GetAndFlattenAllPages(ApiUrls.TeamRepositories(id), options); + return _connection.GetAndFlattenAllPages(ApiUrls.TeamRepositories(id), null, AcceptHeaders.OrganizationPermissionsPreview, options); } /// diff --git a/Octokit.Reactive/Helpers/ConnectionExtensions.cs b/Octokit.Reactive/Helpers/ConnectionExtensions.cs index 5f2aec15..c60ee366 100644 --- a/Octokit.Reactive/Helpers/ConnectionExtensions.cs +++ b/Octokit.Reactive/Helpers/ConnectionExtensions.cs @@ -37,6 +37,15 @@ namespace Octokit.Reactive.Internal return GetPages(url, parameters, (pageUrl, pageParams) => connection.Get>(pageUrl, pageParams, accepts).ToObservable()); } + public static IObservable GetAndFlattenAllPages(this IConnection connection, Uri url, IDictionary parameters, string accepts, ApiOptions options) + { + return GetPagesWithOptions(url, parameters, options, (pageUrl, pageParams, o) => + { + var passingParameters = Pagination.Setup(parameters, options); + return connection.Get>(pageUrl, passingParameters, accepts).ToObservable(); + }); + } + static IObservable GetPages(Uri uri, IDictionary parameters, Func, IObservable>>> getPageFunc) { diff --git a/Octokit.Tests.Integration/Clients/TeamsClientTests.cs b/Octokit.Tests.Integration/Clients/TeamsClientTests.cs index a80a6da0..1a1609b7 100644 --- a/Octokit.Tests.Integration/Clients/TeamsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/TeamsClientTests.cs @@ -4,6 +4,7 @@ using System.Net; using System.Threading.Tasks; using Octokit; using Octokit.Tests.Integration; +using Octokit.Tests.Integration.Helpers; using Xunit; public class TeamsClientTests @@ -98,11 +99,11 @@ public class TeamsClientTests } } - public class TheGetMembersMethod + public class TheGetAllMembersMethod { readonly Team team; - public TheGetMembersMethod() + public TheGetAllMembersMethod() { var github = Helper.GetAuthenticatedClient(); @@ -119,4 +120,32 @@ public class TeamsClientTests Assert.Contains(Helper.UserName, members.Select(u => u.Login)); } } + + public class TheGetAllRepositoriesMethod + { + readonly Team _team; + + public TheGetAllRepositoriesMethod() + { + var github = Helper.GetAuthenticatedClient(); + + _team = github.Organization.Team.GetAll(Helper.Organization).Result.First(); + } + + [OrganizationTest] + public async Task GetsAllRepositories() + { + var github = Helper.GetAuthenticatedClient(); + + using (var repositoryContext = await github.CreateRepositoryContext(Helper.Organization, new NewRepository(Helper.MakeNameWithTimestamp("teamrepo")))) + { + github.Organization.Team.AddRepository(_team.Id, Helper.Organization, repositoryContext.RepositoryName); + + var repos = await github.Organization.Team.GetAllRepositories(_team.Id); + + Assert.True(repos.Count > 0); + Assert.NotNull(repos[0].Permissions); + } + } + } } diff --git a/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs index d6743e8c..c6b9969d 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs @@ -4,15 +4,16 @@ using System.Threading.Tasks; using Octokit; using Octokit.Reactive; using Octokit.Tests.Integration; +using Octokit.Tests.Integration.Helpers; using Xunit; public class ObservableTeamsClientTests { - public class TheGetMembersMethod + public class TheGetAllMembersMethod { readonly Team _team; - public TheGetMembersMethod() + public TheGetAllMembersMethod() { var github = Helper.GetAuthenticatedClient(); @@ -23,12 +24,43 @@ public class ObservableTeamsClientTests public async Task GetsAllMembersWhenAuthenticated() { var github = Helper.GetAuthenticatedClient(); - var client = new ObservableTeamsClient(github); - var member = await client.GetAllMembers(_team.Id, ApiOptions.None); + var observable = client.GetAllMembers(_team.Id, ApiOptions.None); + var members = await observable.ToList(); - Assert.Equal(Helper.UserName, member.Login); + Assert.True(members.Count > 0); + Assert.True(members.Any(x => x.Login == Helper.UserName)); + } + } + + public class TheGetAllRepositoriesMethod + { + readonly Team _team; + + public TheGetAllRepositoriesMethod() + { + var github = Helper.GetAuthenticatedClient(); + + _team = github.Organization.Team.GetAll(Helper.Organization).Result.First(); + } + + [OrganizationTest] + public async Task GetsAllRepositories() + { + var github = Helper.GetAuthenticatedClient(); + var client = new ObservableTeamsClient(github); + + using (var repositoryContext = await github.CreateRepositoryContext(Helper.Organization, new NewRepository(Helper.MakeNameWithTimestamp("teamrepo")))) + { + client.AddRepository(_team.Id, Helper.Organization, repositoryContext.RepositoryName); + + var observable = client.GetAllRepositories(_team.Id, ApiOptions.None); + var repos = await observable.ToList(); + + Assert.True(repos.Count() > 0); + Assert.NotNull(repos[0].Permissions); + } } } } diff --git a/Octokit.Tests/Clients/TeamsClientTests.cs b/Octokit.Tests/Clients/TeamsClientTests.cs index f41c3abe..77930744 100644 --- a/Octokit.Tests/Clients/TeamsClientTests.cs +++ b/Octokit.Tests/Clients/TeamsClientTests.cs @@ -220,7 +220,7 @@ namespace Octokit.Tests.Clients } } - public class TheRRemoveMembershipMethod + public class TheRemoveMembershipMethod { [Fact] public void RequestsTheCorrectUrl() @@ -255,6 +255,8 @@ namespace Octokit.Tests.Clients connection.Received().GetAll( Arg.Is(u => u.ToString() == "teams/1/repos"), + null, + "application/vnd.github.ironman-preview+json", Args.ApiOptions); } } diff --git a/Octokit/Clients/TeamsClient.cs b/Octokit/Clients/TeamsClient.cs index 3ec477ac..f1388d0c 100644 --- a/Octokit/Clients/TeamsClient.cs +++ b/Octokit/Clients/TeamsClient.cs @@ -276,22 +276,7 @@ namespace Octokit var endpoint = ApiUrls.TeamRepositories(id); - return ApiConnection.GetAll(endpoint, options); - } - - /// - /// Returns all (ies) associated with the given team. - /// - /// The team identifier - /// - /// See the API documentation for more information. - /// - /// A list of the team's (ies). - public Task> GetRepositories(int id) - { - var endpoint = ApiUrls.TeamRepositories(id); - - return ApiConnection.GetAll(endpoint); + return ApiConnection.GetAll(endpoint, null, AcceptHeaders.OrganizationPermissionsPreview, options); } ///