From a3d43b636c26024c19a158d3abbe766361394f04 Mon Sep 17 00:00:00 2001 From: Ryan Gribble Date: Sat, 30 Jan 2016 15:52:44 +1000 Subject: [PATCH] Add member role filter to Members.GetAll() functions Add unit tests Added integration tests for this method (previously there were none) ... but then disabled the integration tests that use the filters, as they require the requester to be an org member (for role filter) or owner (for 2 factor filter) --- .../IObservableOrganizationMembersClient.cs | 47 +++++++++ .../ObservableOrganizationMembersClient.cs | 57 +++++++++++ .../Clients/OrganizationMembersClientTests.cs | 72 ++++++++++++++ .../Octokit.Tests.Integration.csproj | 1 + .../Clients/OrganizationMembersClientTests.cs | 99 +++++++++++++++++++ Octokit/Clients/IOrganizationMembersClient.cs | 47 +++++++++ Octokit/Clients/OrganizationMembersClient.cs | 69 +++++++++++++ Octokit/Helpers/ApiUrls.cs | 22 +++++ 8 files changed, 414 insertions(+) create mode 100644 Octokit.Tests.Integration/Clients/OrganizationMembersClientTests.cs diff --git a/Octokit.Reactive/Clients/IObservableOrganizationMembersClient.cs b/Octokit.Reactive/Clients/IObservableOrganizationMembersClient.cs index d2e6d9ca..f89cef99 100644 --- a/Octokit.Reactive/Clients/IObservableOrganizationMembersClient.cs +++ b/Octokit.Reactive/Clients/IObservableOrganizationMembersClient.cs @@ -59,6 +59,53 @@ namespace Octokit.Reactive [Obsolete("No longer supported, use GetAll(string, OrganizationMembersFilter) instead")] IObservable GetAll(string org, string filter); + /// + /// + /// List all users who are members of an organization. A member is a user that + /// belongs to at least 1 team in the organization. + /// + /// + /// If the authenticated user is also an owner of this organization then both + /// concealed and public member will be returned. + /// + /// + /// If the requester is not an owner of the organization the query will be redirected + /// to the public members list. + /// + /// + /// + /// See the API documentation + /// for more information. + /// + /// The login for the organization + /// The role filter to use when getting the users, + /// + IObservable GetAll(string org, OrganizationMembersRole role); + + /// + /// + /// List all users who are members of an organization. A member is a user that + /// belongs to at least 1 team in the organization. + /// + /// + /// If the authenticated user is also an owner of this organization then both + /// concealed and public member will be returned. + /// + /// + /// If the requester is not an owner of the organization the query will be redirected + /// to the public members list. + /// + /// + /// + /// See the API documentation + /// for more information. + /// + /// The login for the organization + /// The members filter, + /// The role filter to use when getting the users, + /// + IObservable GetAll(string org, OrganizationMembersFilter filter, OrganizationMembersRole role); + /// /// List all users who have publicized their membership of the organization. /// diff --git a/Octokit.Reactive/Clients/ObservableOrganizationMembersClient.cs b/Octokit.Reactive/Clients/ObservableOrganizationMembersClient.cs index 9eef60a0..b2b1d900 100644 --- a/Octokit.Reactive/Clients/ObservableOrganizationMembersClient.cs +++ b/Octokit.Reactive/Clients/ObservableOrganizationMembersClient.cs @@ -92,6 +92,63 @@ namespace Octokit.Reactive return _connection.GetAndFlattenAllPages(ApiUrls.Members(org, filter)); } + /// + /// + /// List all users who are members of an organization. A member is a user that + /// belongs to at least 1 team in the organization. + /// + /// + /// If the authenticated user is also an owner of this organization then both + /// concealed and public member will be returned. + /// + /// + /// If the requester is not an owner of the organization the query will be redirected + /// to the public members list. + /// + /// + /// + /// See the API documentation + /// for more information. + /// + /// The login for the organization + /// The role filter to use when getting the users, + /// + public IObservable GetAll(string org, OrganizationMembersRole role) + { + Ensure.ArgumentNotNullOrEmptyString(org, "org"); + + return _connection.GetAndFlattenAllPages(ApiUrls.Members(org, role)); + } + + /// + /// + /// List all users who are members of an organization. A member is a user that + /// belongs to at least 1 team in the organization. + /// + /// + /// If the authenticated user is also an owner of this organization then both + /// concealed and public member will be returned. + /// + /// + /// If the requester is not an owner of the organization the query will be redirected + /// to the public members list. + /// + /// + /// + /// See the API documentation + /// for more information. + /// + /// The login for the organization + /// The members filter, + /// The role filter to use when getting the users, + /// + public IObservable GetAll(string org, OrganizationMembersFilter filter, OrganizationMembersRole role) + { + Ensure.ArgumentNotNullOrEmptyString(org, "org"); + + return _connection.GetAndFlattenAllPages(ApiUrls.Members(org, filter, role)); + } + /// /// List all users who have publicized their membership of the organization. /// diff --git a/Octokit.Tests.Integration/Clients/OrganizationMembersClientTests.cs b/Octokit.Tests.Integration/Clients/OrganizationMembersClientTests.cs new file mode 100644 index 00000000..301dbce2 --- /dev/null +++ b/Octokit.Tests.Integration/Clients/OrganizationMembersClientTests.cs @@ -0,0 +1,72 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Octokit; +using Octokit.Tests.Integration; +using Octokit.Tests.Integration.Helpers; +using Xunit; + +namespace Octokit.Tests.Integration.Clients +{ + public class OrganizationMembersClientTests + { + public class TheGetAllMethod + { + private IGitHubClient _gitHub; + private string _organizationFixture; + + public TheGetAllMethod() + { + _gitHub = Helper.GetAuthenticatedClient(); + _organizationFixture = "octokit"; + } + + [IntegrationTest] + public async Task ReturnsMembers() + { + var members = _gitHub.Organization.Member.GetAll(_organizationFixture); + Assert.NotNull(members); + } + + [IntegrationTest(Skip = "TwoFactor filter can't be used unless the requester is an organization owner")] + public async Task ReturnsMembersWithFilter() + { + var no2FAMembers = await _gitHub.Organization.Member.GetAll(_organizationFixture, OrganizationMembersFilter.TwoFactorAuthenticationDisabled); + Assert.NotNull(no2FAMembers); + } + + [IntegrationTest(Skip = "Admin/Member filter doesn't work unless the requester is an organization member")] + public async Task ReturnsMembersWithRole() + { + var adminMembers = await _gitHub.Organization.Member.GetAll(_organizationFixture, OrganizationMembersRole.Admin); + Assert.NotNull(adminMembers); + + var normalMembers = await _gitHub.Organization.Member.GetAll(_organizationFixture, OrganizationMembersRole.Member); + Assert.NotNull(normalMembers); + + // There shouldnt be any members that are in both groups if the role filter works correctly + var membersInBoth = adminMembers.Select(a => a.Id).Intersect(normalMembers.Select(n => n.Id)); + Assert.True(membersInBoth.Count() == 0); + } + + [IntegrationTest(Skip = "TwoFactor filter can't be used unless the requester is an organization owner")] + public async Task ReturnsMembersWthFilterAndRole() + { + // Get count of admin/normal members + var adminCount = (await _gitHub.Organization.Member.GetAll(_organizationFixture, OrganizationMembersRole.Admin)).Count; + var memberCount = (await _gitHub.Organization.Member.GetAll(_organizationFixture, OrganizationMembersRole.Member)).Count; + + // Ensure that there are less admins with no 2 factor, than there are total admins + var adminsWithNo2FA = await _gitHub.Organization.Member.GetAll(_organizationFixture, OrganizationMembersFilter.TwoFactorAuthenticationDisabled, OrganizationMembersRole.Admin); + Assert.NotNull(adminsWithNo2FA); + Assert.True(adminsWithNo2FA.Count <= adminCount); + + // Ensure that there are less members with no 2 factor, than there are total admins + var membersWithNo2FA = await _gitHub.Organization.Member.GetAll(_organizationFixture, OrganizationMembersFilter.TwoFactorAuthenticationDisabled, OrganizationMembersRole.Member); + Assert.NotNull(membersWithNo2FA); + Assert.True(membersWithNo2FA.Count <= memberCount); + } + } + } +} diff --git a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj index b2cd9b6c..44e01fee 100644 --- a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj +++ b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj @@ -88,6 +88,7 @@ + diff --git a/Octokit.Tests/Clients/OrganizationMembersClientTests.cs b/Octokit.Tests/Clients/OrganizationMembersClientTests.cs index 38f1cba7..b35b60f7 100644 --- a/Octokit.Tests/Clients/OrganizationMembersClientTests.cs +++ b/Octokit.Tests/Clients/OrganizationMembersClientTests.cs @@ -67,6 +67,105 @@ namespace Octokit.Tests.Clients client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?filter=2fa_disabled")); } + + [Fact] + public void AllRoleFilterRequestTheCorrectUrl() + { + var client = Substitute.For(); + var orgMembersClient = new OrganizationMembersClient(client); + + orgMembersClient.GetAll("org", OrganizationMembersRole.All); + + client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?role=all")); + } + + [Fact] + public void AdminRoleFilterRequestTheCorrectUrl() + { + var client = Substitute.For(); + var orgMembersClient = new OrganizationMembersClient(client); + + orgMembersClient.GetAll("org", OrganizationMembersRole.Admin); + + client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?role=admin")); + } + + [Fact] + public void MemberRoleFilterRequestTheCorrectUrl() + { + var client = Substitute.For(); + var orgMembersClient = new OrganizationMembersClient(client); + + orgMembersClient.GetAll("org", OrganizationMembersRole.Member); + + client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?role=member")); + } + + [Fact] + public void AllFilterPlusAllRoleFilterRequestTheCorrectUrl() + { + var client = Substitute.For(); + var orgMembersClient = new OrganizationMembersClient(client); + + orgMembersClient.GetAll("org", OrganizationMembersFilter.All, OrganizationMembersRole.All); + + client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?filter=all&role=all")); + } + + [Fact] + public void AllFilterPlusAdminRoleFilterRequestTheCorrectUrl() + { + var client = Substitute.For(); + var orgMembersClient = new OrganizationMembersClient(client); + + orgMembersClient.GetAll("org", OrganizationMembersFilter.All, OrganizationMembersRole.Admin); + + client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?filter=all&role=admin")); + } + + [Fact] + public void AllFilterPlusMemberRoleFilterRequestTheCorrectUrl() + { + var client = Substitute.For(); + var orgMembersClient = new OrganizationMembersClient(client); + + orgMembersClient.GetAll("org", OrganizationMembersFilter.All, OrganizationMembersRole.Member); + + client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?filter=all&role=member")); + } + + [Fact] + public void TwoFactorFilterPlusAllRoleRequestTheCorrectUrl() + { + var client = Substitute.For(); + var orgMembersClient = new OrganizationMembersClient(client); + + orgMembersClient.GetAll("org", OrganizationMembersFilter.TwoFactorAuthenticationDisabled, OrganizationMembersRole.All); + + client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?filter=2fa_disabled&role=all")); + } + + [Fact] + public void TwoFactorFilterPlusAdminRoleRequestTheCorrectUrl() + { + var client = Substitute.For(); + var orgMembersClient = new OrganizationMembersClient(client); + + orgMembersClient.GetAll("org", OrganizationMembersFilter.TwoFactorAuthenticationDisabled, OrganizationMembersRole.Admin); + + client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?filter=2fa_disabled&role=admin")); + } + + [Fact] + public void TwoFactorFilterPlusMemberRoleRequestTheCorrectUrl() + { + var client = Substitute.For(); + var orgMembersClient = new OrganizationMembersClient(client); + + orgMembersClient.GetAll("org", OrganizationMembersFilter.TwoFactorAuthenticationDisabled, OrganizationMembersRole.Member); + + client.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/org/members?filter=2fa_disabled&role=member")); + } } public class TheGetPublicMethod diff --git a/Octokit/Clients/IOrganizationMembersClient.cs b/Octokit/Clients/IOrganizationMembersClient.cs index 1db3ffb8..3cb99a6f 100644 --- a/Octokit/Clients/IOrganizationMembersClient.cs +++ b/Octokit/Clients/IOrganizationMembersClient.cs @@ -66,6 +66,53 @@ namespace Octokit [Obsolete("No longer supported, use GetAll(string, OrganizationMembersFilter) instead")] Task> GetAll(string org, string filter); + /// + /// + /// List all users who are members of an organization. A member is a user that + /// belongs to at least 1 team in the organization. + /// + /// + /// If the authenticated user is also an owner of this organization then both + /// concealed and public member will be returned. + /// + /// + /// If the requester is not an owner of the organization the query will be redirected + /// to the public members list. + /// + /// + /// + /// See the API documentation + /// for more information. + /// + /// The login for the organization + /// The role filter to use when getting the users, + /// The users + Task> GetAll(string org, OrganizationMembersRole role); + + /// + /// + /// List all users who are members of an organization. A member is a user that + /// belongs to at least 1 team in the organization. + /// + /// + /// If the authenticated user is also an owner of this organization then both + /// concealed and public member will be returned. + /// + /// + /// If the requester is not an owner of the organization the query will be redirected + /// to the public members list. + /// + /// + /// + /// See the API documentation + /// for more information. + /// + /// The login for the organization + /// The filter to use when getting the users, + /// The role filter to use when getting the users, + /// The users + Task> GetAll(string org, OrganizationMembersFilter filter, OrganizationMembersRole role); + /// /// List all users who have publicized their membership of the organization. /// diff --git a/Octokit/Clients/OrganizationMembersClient.cs b/Octokit/Clients/OrganizationMembersClient.cs index 3ef7180d..02eb939a 100644 --- a/Octokit/Clients/OrganizationMembersClient.cs +++ b/Octokit/Clients/OrganizationMembersClient.cs @@ -26,6 +26,18 @@ namespace Octokit TwoFactorAuthenticationDisabled } + public enum OrganizationMembersRole + { + [Parameter(Value = "all")] + All, + + [Parameter(Value = "admin")] + Admin, + + [Parameter(Value = "member")] + Member + } + /// /// A client for GitHub's Organization Members API. /// @@ -112,6 +124,63 @@ namespace Octokit return ApiConnection.GetAll(ApiUrls.Members(org, filter)); } + /// + /// + /// List all users who are members of an organization. A member is a user that + /// belongs to at least 1 team in the organization. + /// + /// + /// If the authenticated user is also an owner of this organization then both + /// concealed and public member will be returned. + /// + /// + /// If the requester is not an owner of the organization the query will be redirected + /// to the public members list. + /// + /// + /// + /// See the API documentation + /// for more information. + /// + /// The login for the organization + /// The role filter to use when getting the users, + /// The users + public Task> GetAll(string org, OrganizationMembersRole role) + { + Ensure.ArgumentNotNullOrEmptyString(org, "org"); + + return ApiConnection.GetAll(ApiUrls.Members(org, role)); + } + + /// + /// + /// List all users who are members of an organization. A member is a user that + /// belongs to at least 1 team in the organization. + /// + /// + /// If the authenticated user is also an owner of this organization then both + /// concealed and public member will be returned. + /// + /// + /// If the requester is not an owner of the organization the query will be redirected + /// to the public members list. + /// + /// + /// + /// See the API documentation + /// for more information. + /// + /// The login for the organization + /// The filter to use when getting the users, + /// The role filter to use when getting the users, + /// The users + public Task> GetAll(string org, OrganizationMembersFilter filter, OrganizationMembersRole role) + { + Ensure.ArgumentNotNullOrEmptyString(org, "org"); + + return ApiConnection.GetAll(ApiUrls.Members(org, filter, role)); + } + /// /// List all users who have publicized their membership of the organization. /// diff --git a/Octokit/Helpers/ApiUrls.cs b/Octokit/Helpers/ApiUrls.cs index a1bc2dca..7e6a2796 100644 --- a/Octokit/Helpers/ApiUrls.cs +++ b/Octokit/Helpers/ApiUrls.cs @@ -384,6 +384,28 @@ namespace Octokit return "orgs/{0}/members?filter={1}".FormatUri(org, filter); } + /// + /// Returns the that returns all of the members of the organization + /// + /// The organization + /// The member filter, + /// The correct uri + public static Uri Members(string org, OrganizationMembersRole role) + { + return "orgs/{0}/members?role={1}".FormatUri(org, role.ToParameter()); + } + + /// + /// Returns the that returns all of the members of the organization + /// + /// The organization + /// The member filter, + /// The correct uri + public static Uri Members(string org, OrganizationMembersFilter filter, OrganizationMembersRole role) + { + return "orgs/{0}/members?filter={1}&role={2}".FormatUri(org, filter.ToParameter(), role.ToParameter()); + } + /// /// Returns the that returns all of the public members of the organization ///