From a93a73ea62978756be27f6e2fe20536bbe4e24d7 Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Tue, 2 May 2017 13:52:43 +0300 Subject: [PATCH 01/11] Add BranchProtection.EnforceAdmins object --- .../Clients/RepositoryBranchesClientTests.cs | 38 +++++++++---------- .../OrganizationRepositoryWithTeamContext.cs | 8 ++-- .../Clients/RepositoryBranchesClientTests.cs | 12 +++--- ...ObservableRepositoryBranchesClientTests.cs | 15 +++----- .../Models/Request/BranchProtectionUpdate.cs | 31 +++++++++++---- Octokit/Models/Response/BranchProtection.cs | 37 ++++++++++++------ 6 files changed, 84 insertions(+), 57 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs index 5e300efeba..c4ebbb7f3c 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs @@ -329,7 +329,7 @@ public async Task GetsBranchProtection() var repoName = _userRepoContext.RepositoryName; var protection = await _client.GetBranchProtection(repoOwner, repoName, "master"); - Assert.True(protection.RequiredStatusChecks.IncludeAdmins); + Assert.True(protection.EnforceAdmins.Enabled); Assert.True(protection.RequiredStatusChecks.Strict); Assert.Equal(2, protection.RequiredStatusChecks.Contexts.Count); @@ -342,7 +342,7 @@ public async Task GetsBranchProtectionWithRepositoryId() var repoId = _userRepoContext.RepositoryId; var protection = await _client.GetBranchProtection(repoId, "master"); - Assert.True(protection.RequiredStatusChecks.IncludeAdmins); + Assert.True(protection.EnforceAdmins.Enabled); Assert.True(protection.RequiredStatusChecks.Strict); Assert.Equal(2, protection.RequiredStatusChecks.Contexts.Count); @@ -356,7 +356,7 @@ public async Task GetsBranchProtectionForOrgRepo() var repoName = _orgRepoContext.RepositoryContext.RepositoryName; var protection = await _client.GetBranchProtection(repoOwner, repoName, "master"); - Assert.True(protection.RequiredStatusChecks.IncludeAdmins); + Assert.True(protection.EnforceAdmins.Enabled); Assert.True(protection.RequiredStatusChecks.Strict); Assert.Equal(2, protection.RequiredStatusChecks.Contexts.Count); @@ -370,7 +370,7 @@ public async Task GetsBranchProtectionForOrgRepoWithRepositoryId() var repoId = _orgRepoContext.RepositoryContext.RepositoryId; var protection = await _client.GetBranchProtection(repoId, "master"); - Assert.True(protection.RequiredStatusChecks.IncludeAdmins); + Assert.True(protection.EnforceAdmins.Enabled); Assert.True(protection.RequiredStatusChecks.Strict); Assert.Equal(2, protection.RequiredStatusChecks.Contexts.Count); @@ -409,11 +409,11 @@ public async Task UpdatesBranchProtection() var repoOwner = _userRepoContext.RepositoryOwner; var repoName = _userRepoContext.RepositoryName; var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(false, false, new[] { "new" })); + new BranchProtectionRequiredStatusChecksUpdate(false, new[] { "new" })); var protection = await _client.UpdateBranchProtection(repoOwner, repoName, "master", update); - Assert.False(protection.RequiredStatusChecks.IncludeAdmins); + Assert.False(protection.EnforceAdmins.Enabled); Assert.False(protection.RequiredStatusChecks.Strict); Assert.Equal(1, protection.RequiredStatusChecks.Contexts.Count); @@ -425,11 +425,11 @@ public async Task UpdatesBranchProtectionWithRepositoryId() { var repoId = _userRepoContext.RepositoryId; var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(false, false, new[] { "new" })); + new BranchProtectionRequiredStatusChecksUpdate(false, new[] { "new" })); var protection = await _client.UpdateBranchProtection(repoId, "master", update); - Assert.False(protection.RequiredStatusChecks.IncludeAdmins); + Assert.False(protection.EnforceAdmins.Enabled); Assert.False(protection.RequiredStatusChecks.Strict); Assert.Equal(1, protection.RequiredStatusChecks.Contexts.Count); @@ -442,12 +442,13 @@ public async Task UpdatesBranchProtectionForOrgRepo() var repoOwner = _orgRepoContext.RepositoryContext.RepositoryOwner; var repoName = _orgRepoContext.RepositoryContext.RepositoryName; var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(false, false, new[] { "new" }), - new BranchProtectionPushRestrictionsUpdate()); + new BranchProtectionRequiredStatusChecksUpdate(false, new[] { "new" }), + new BranchProtectionPushRestrictionsUpdate(), + false); var protection = await _client.UpdateBranchProtection(repoOwner, repoName, "master", update); - Assert.False(protection.RequiredStatusChecks.IncludeAdmins); + Assert.False(protection.EnforceAdmins.Enabled); Assert.False(protection.RequiredStatusChecks.Strict); Assert.Equal(1, protection.RequiredStatusChecks.Contexts.Count); @@ -460,12 +461,13 @@ public async Task UpdatesBranchProtectionForOrgRepoWithRepositoryId() { var repoId = _orgRepoContext.RepositoryContext.RepositoryId; var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(false, false, new[] { "new" }), - new BranchProtectionPushRestrictionsUpdate()); + new BranchProtectionRequiredStatusChecksUpdate(false, new[] { "new" }), + new BranchProtectionPushRestrictionsUpdate(), + false); var protection = await _client.UpdateBranchProtection(repoId, "master", update); - Assert.False(protection.RequiredStatusChecks.IncludeAdmins); + Assert.False(protection.EnforceAdmins.Enabled); Assert.False(protection.RequiredStatusChecks.Strict); Assert.Equal(1, protection.RequiredStatusChecks.Contexts.Count); @@ -567,7 +569,6 @@ public async Task GetsRequiredStatusChecks() Assert.NotNull(requiredStatusChecks); Assert.NotNull(requiredStatusChecks.Contexts); - Assert.True(requiredStatusChecks.IncludeAdmins); Assert.True(requiredStatusChecks.Strict); Assert.Equal(2, requiredStatusChecks.Contexts.Count); } @@ -580,7 +581,6 @@ public async Task GetsRequiredStatusChecksWithRepositoryId() Assert.NotNull(requiredStatusChecks); Assert.NotNull(requiredStatusChecks.Contexts); - Assert.True(requiredStatusChecks.IncludeAdmins); Assert.True(requiredStatusChecks.Strict); Assert.Equal(2, requiredStatusChecks.Contexts.Count); } @@ -610,13 +610,12 @@ public async Task UpdateRequiredStatusChecks() { var repoOwner = _userRepoContext.RepositoryOwner; var repoName = _userRepoContext.RepositoryName; - var update = new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "new" }); + var update = new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "new" }); var requiredStatusChecks = await _client.UpdateRequiredStatusChecks(repoOwner, repoName, "master", update); Assert.NotNull(requiredStatusChecks); Assert.NotNull(requiredStatusChecks.Contexts); Assert.True(requiredStatusChecks.Contexts.Contains("new")); - Assert.True(requiredStatusChecks.IncludeAdmins); Assert.True(requiredStatusChecks.Strict); Assert.Equal(1, requiredStatusChecks.Contexts.Count); } @@ -625,13 +624,12 @@ public async Task UpdateRequiredStatusChecks() public async Task UpdatesRequiredStatusChecksWithRepositoryId() { var repoId = _userRepoContext.RepositoryId; - var update = new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "new" }); + var update = new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "new" }); var requiredStatusChecks = await _client.UpdateRequiredStatusChecks(repoId, "master", update); Assert.NotNull(requiredStatusChecks); Assert.NotNull(requiredStatusChecks.Contexts); Assert.True(requiredStatusChecks.Contexts.Contains("new")); - Assert.True(requiredStatusChecks.IncludeAdmins); Assert.True(requiredStatusChecks.Strict); Assert.Equal(1, requiredStatusChecks.Contexts.Count); } diff --git a/Octokit.Tests.Integration/Helpers/OrganizationRepositoryWithTeamContext.cs b/Octokit.Tests.Integration/Helpers/OrganizationRepositoryWithTeamContext.cs index 6760060679..4211ef936f 100644 --- a/Octokit.Tests.Integration/Helpers/OrganizationRepositoryWithTeamContext.cs +++ b/Octokit.Tests.Integration/Helpers/OrganizationRepositoryWithTeamContext.cs @@ -30,8 +30,7 @@ internal async static Task CreateRepositoryWithProtectedBranc var contextUserRepo = await client.CreateRepositoryContext(userRepo); // Protect master branch - var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "build", "test" })); + var update = new BranchProtectionSettingsUpdate(new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "build", "test" }), null, true); await client.Repository.Branch.UpdateBranchProtection(contextUserRepo.RepositoryOwner, contextUserRepo.RepositoryName, "master", update); @@ -56,8 +55,9 @@ await client.Organization.Team.AddRepository( // Protect master branch var protection = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "build", "test" }), - new BranchProtectionPushRestrictionsUpdate(new BranchProtectionTeamCollection { contextOrgTeam.TeamName })); + new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "build", "test" }), + new BranchProtectionPushRestrictionsUpdate(new BranchProtectionTeamCollection { contextOrgTeam.TeamName }), + true); await client.Repository.Branch.UpdateBranchProtection(contextOrgRepo.RepositoryOwner, contextOrgRepo.RepositoryName, "master", protection); return new OrganizationRepositoryWithTeamContext diff --git a/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs b/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs index 846a77bcbe..e02e165250 100644 --- a/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs @@ -257,7 +257,7 @@ public void RequestsTheCorrectUrl() var connection = Substitute.For(); var client = new RepositoryBranchesClient(connection); var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" })); + new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" })); const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; client.UpdateBranchProtection("owner", "repo", "branch", update); @@ -272,7 +272,7 @@ public void RequestsTheCorrectUrlWithRepositoryId() var connection = Substitute.For(); var client = new RepositoryBranchesClient(connection); var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" })); + new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" })); const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; client.UpdateBranchProtection(1, "branch", update); @@ -286,7 +286,7 @@ public async Task EnsuresNonNullArguments() { var client = new RepositoryBranchesClient(Substitute.For()); var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" })); + new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" })); await Assert.ThrowsAsync(() => client.UpdateBranchProtection(null, "repo", "branch", update)); await Assert.ThrowsAsync(() => client.UpdateBranchProtection("owner", null, "branch", update)); @@ -405,7 +405,7 @@ public void RequestsTheCorrectUrl() { var connection = Substitute.For(); var client = new RepositoryBranchesClient(connection); - var update = new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" }); + var update = new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" }); const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; client.UpdateRequiredStatusChecks("owner", "repo", "branch", update); @@ -419,7 +419,7 @@ public void RequestsTheCorrectUrlWithRepositoryId() { var connection = Substitute.For(); var client = new RepositoryBranchesClient(connection); - var update = new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" }); + var update = new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" }); const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; client.UpdateRequiredStatusChecks(1, "branch", update); @@ -432,7 +432,7 @@ public void RequestsTheCorrectUrlWithRepositoryId() public async Task EnsuresNonNullArguments() { var client = new RepositoryBranchesClient(Substitute.For()); - var update = new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" }); + var update = new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" }); await Assert.ThrowsAsync(() => client.UpdateRequiredStatusChecks(null, "repo", "branch", update)); await Assert.ThrowsAsync(() => client.UpdateRequiredStatusChecks("owner", null, "branch", update)); diff --git a/Octokit.Tests/Reactive/ObservableRepositoryBranchesClientTests.cs b/Octokit.Tests/Reactive/ObservableRepositoryBranchesClientTests.cs index a4dd6a66c7..d49f302ee0 100644 --- a/Octokit.Tests/Reactive/ObservableRepositoryBranchesClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableRepositoryBranchesClientTests.cs @@ -249,8 +249,7 @@ public void RequestsTheCorrectUrl() { var gitHubClient = Substitute.For(); var client = new ObservableRepositoryBranchesClient(gitHubClient); - var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" })); + var update = new BranchProtectionSettingsUpdate(new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" })); client.UpdateBranchProtection("owner", "repo", "branch", update); @@ -263,8 +262,7 @@ public void RequestsTheCorrectUrlWithRepositoryId() { var gitHubClient = Substitute.For(); var client = new ObservableRepositoryBranchesClient(gitHubClient); - var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" })); + var update = new BranchProtectionSettingsUpdate(new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" })); client.UpdateBranchProtection(1, "branch", update); @@ -276,8 +274,7 @@ public void RequestsTheCorrectUrlWithRepositoryId() public async Task EnsuresNonNullArguments() { var client = new ObservableRepositoryBranchesClient(Substitute.For()); - var update = new BranchProtectionSettingsUpdate( - new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" })); + var update = new BranchProtectionSettingsUpdate(new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" })); Assert.Throws(() => client.UpdateBranchProtection(null, "repo", "branch", update)); Assert.Throws(() => client.UpdateBranchProtection("owner", null, "branch", update)); @@ -392,7 +389,7 @@ public void RequestsTheCorrectUrl() { var gitHubClient = Substitute.For(); var client = new ObservableRepositoryBranchesClient(gitHubClient); - var update = new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" }); + var update = new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" }); client.UpdateRequiredStatusChecks("owner", "repo", "branch", update); @@ -405,7 +402,7 @@ public void RequestsTheCorrectUrlWithRepositoryId() { var gitHubClient = Substitute.For(); var client = new ObservableRepositoryBranchesClient(gitHubClient); - var update = new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" }); + var update = new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" }); client.UpdateRequiredStatusChecks(1, "branch", update); @@ -417,7 +414,7 @@ public void RequestsTheCorrectUrlWithRepositoryId() public async Task EnsuresNonNullArguments() { var client = new ObservableRepositoryBranchesClient(Substitute.For()); - var update = new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "test" }); + var update = new BranchProtectionRequiredStatusChecksUpdate(true, new[] { "test" }); Assert.Throws(() => client.UpdateRequiredStatusChecks(null, "repo", "branch", update)); Assert.Throws(() => client.UpdateRequiredStatusChecks("owner", null, "branch", update)); diff --git a/Octokit/Models/Request/BranchProtectionUpdate.cs b/Octokit/Models/Request/BranchProtectionUpdate.cs index d7c6ed6b7c..ecc5862be4 100644 --- a/Octokit/Models/Request/BranchProtectionUpdate.cs +++ b/Octokit/Models/Request/BranchProtectionUpdate.cs @@ -26,17 +26,34 @@ public BranchProtectionSettingsUpdate(BranchProtectionRequiredStatusChecksUpdate Restrictions = null; } + /// + /// Create a BranchProtection update request + /// + /// Specifies the requested push access restrictions (applies only to Organization owned repositories). Pass null to disable push access restrictions + public BranchProtectionSettingsUpdate(BranchProtectionPushRestrictionsUpdate restrictions) + { + RequiredStatusChecks = null; + Restrictions = restrictions; + } + /// /// Create a BranchProtection update request /// /// Specifies the requested status check settings. Pass null to disable status checks /// Specifies the requested push access restrictions (applies only to Organization owned repositories). Pass null to disable push access restrictions - public BranchProtectionSettingsUpdate(BranchProtectionRequiredStatusChecksUpdate requiredStatusChecks, BranchProtectionPushRestrictionsUpdate restrictions) + /// Specifies whether the protections applied to this branch also apply to repository admins + public BranchProtectionSettingsUpdate(BranchProtectionRequiredStatusChecksUpdate requiredStatusChecks, BranchProtectionPushRestrictionsUpdate restrictions, bool enforceAdmins) { RequiredStatusChecks = requiredStatusChecks; Restrictions = restrictions; + EnforceAdmins = enforceAdmins; } + /// + /// Specifies whether the protections applied to this branch also apply to repository admins + /// + public bool EnforceAdmins { get; set; } + /// /// Status check settings for the protected branch /// @@ -54,9 +71,10 @@ internal string DebuggerDisplay get { return string.Format(CultureInfo.InvariantCulture, - "StatusChecks: {0} Restrictions: {1}", + "StatusChecks: {0} Restrictions: {1} EnforceAdmins: {2}", RequiredStatusChecks == null ? "disabled" : RequiredStatusChecks.DebuggerDisplay, - Restrictions == null ? "disabled" : Restrictions.DebuggerDisplay); + Restrictions == null ? "disabled" : Restrictions.DebuggerDisplay, + EnforceAdmins); } } } @@ -70,12 +88,10 @@ public class BranchProtectionRequiredStatusChecksUpdate /// /// Status check settings for branch protection /// - /// Enforce required status checks for repository administrators /// Require branches to be up to date before merging /// Require status checks to pass before merging - public BranchProtectionRequiredStatusChecksUpdate(bool includeAdmins, bool strict, IReadOnlyList contexts) + public BranchProtectionRequiredStatusChecksUpdate(bool strict, IReadOnlyList contexts) { - IncludeAdmins = includeAdmins; Strict = strict; Contexts = contexts; } @@ -83,6 +99,7 @@ public BranchProtectionRequiredStatusChecksUpdate(bool includeAdmins, bool stric /// /// Enforce required status checks for repository administrators /// + [Obsolete("This property is obsolete. Use EnforceAdmins on the BranchProtectionUpdate class")] public bool IncludeAdmins { get; protected set; } /// @@ -99,7 +116,7 @@ internal string DebuggerDisplay { get { - return string.Format(CultureInfo.InvariantCulture, "IncludeAdmins: {0} Strict: {1} Contexts: {2}", IncludeAdmins, Strict, Contexts == null ? "" : String.Join(",", Contexts)); + return string.Format(CultureInfo.InvariantCulture, "Strict: {0} Contexts: {1}", Strict, Contexts == null ? "" : string.Join(",", Contexts)); } } } diff --git a/Octokit/Models/Response/BranchProtection.cs b/Octokit/Models/Response/BranchProtection.cs index a7d04d0cbb..dec5c4bfd3 100644 --- a/Octokit/Models/Response/BranchProtection.cs +++ b/Octokit/Models/Response/BranchProtection.cs @@ -78,7 +78,7 @@ internal string DebuggerDisplay /// /// The enforcement levels that are available /// - [Obsolete("This existing implementation will cease to work when the Branch Protection API preview period ends. Please see BranchProtectionRequiredStatusChecks.IncludeAdmins instead.")] + [Obsolete("This existing implementation will cease to work when the Branch Protection API preview period ends. Please see BranchProtection.EnforceAdmins instead.")] public enum EnforcementLevel { /// @@ -134,6 +134,28 @@ internal string DebuggerDisplay Restrictions == null ? "disabled" : Restrictions.DebuggerDisplay); } } + + /// + /// Specifies whether the protections applied to this branch also apply to repository admins + /// + public EnforceAdmins EnforceAdmins { get; protected set; } + } + + /// + /// Specifies whether the protections applied to this branch also apply to repository admins + /// + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class EnforceAdmins + { + public bool Enabled { get; protected set; } + + internal string DebuggerDisplay + { + get + { + return string.Format(CultureInfo.InvariantCulture, "Enabled: {0}", Enabled); + } + } } /// @@ -144,18 +166,12 @@ public class BranchProtectionRequiredStatusChecks { public BranchProtectionRequiredStatusChecks() { } - public BranchProtectionRequiredStatusChecks(bool includeAdmins, bool strict, IReadOnlyList contexts) + public BranchProtectionRequiredStatusChecks(bool strict, IReadOnlyList contexts) { - IncludeAdmins = includeAdmins; Strict = strict; Contexts = contexts; } - /// - /// Enforce required status checks for repository administrators - /// - public bool IncludeAdmins { get; protected set; } - /// /// Require branches to be up to date before merging /// @@ -171,10 +187,9 @@ internal string DebuggerDisplay get { return string.Format(CultureInfo.InvariantCulture, - "IncludeAdmins: {0} Strict: {1} Contexts: {2}", - IncludeAdmins, + "Strict: {0} Contexts: {1}", Strict, - Contexts == null ? "" : String.Join(",", Contexts)); + Contexts == null ? "" : string.Join(",", Contexts)); } } } From 6f4e4a9960da1660ea483f7cd0844387e1669c95 Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Tue, 2 May 2017 15:05:04 +0300 Subject: [PATCH 02/11] Add EnforceAdmin related methods to RepoBranch clients --- .../IObservableRepositoryBranchesClient.cs | 63 +++++++++++++++++ .../ObservableRepositoryBranchesClient.cs | 48 +++++++++++++ Octokit/Clients/IRepositoryBranchesClient.cs | 63 +++++++++++++++++ Octokit/Clients/RepositoryBranchesClient.cs | 69 +++++++++++++++++++ Octokit/Helpers/ApiUrls.cs | 21 ++++++ 5 files changed, 264 insertions(+) diff --git a/Octokit.Reactive/Clients/IObservableRepositoryBranchesClient.cs b/Octokit.Reactive/Clients/IObservableRepositoryBranchesClient.cs index f266b453b7..7f23dbb6fd 100644 --- a/Octokit.Reactive/Clients/IObservableRepositoryBranchesClient.cs +++ b/Octokit.Reactive/Clients/IObservableRepositoryBranchesClient.cs @@ -314,6 +314,69 @@ public interface IObservableRepositoryBranchesClient /// The contexts to remove IObservable DeleteRequiredStatusChecksContexts(long repositoryId, string branch, IReadOnlyList contexts); + /// + /// Get admin enforcement of protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + IObservable GetAdminEnforcement(string owner, string name, string branch); + + /// + /// Get admin enforcement of protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch + IObservable GetAdminEnforcement(long repositoryId, string branch); + + /// + /// Add admin enforcement to protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + IObservable AddAdminEnforcement(string owner, string name, string branch); + + /// + /// Add admin enforcement to protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch + IObservable AddAdminEnforcement(long repositoryId, string branch); + + /// + /// Remove admin enforcement on protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + IObservable RemoveAdminEnforcement(string owner, string name, string branch); + + /// + /// Remove admin enforcement on protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch + IObservable RemoveAdminEnforcement(long repositoryId, string branch); + /// /// Get the restrictions for the specified branch (applies only to Organization owned repositories) /// diff --git a/Octokit.Reactive/Clients/ObservableRepositoryBranchesClient.cs b/Octokit.Reactive/Clients/ObservableRepositoryBranchesClient.cs index e6cc497812..7259294881 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoryBranchesClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoryBranchesClient.cs @@ -499,6 +499,54 @@ public IObservable DeleteRequiredStatusChecksContexts(long repositoryId, return _client.DeleteRequiredStatusChecksContexts(repositoryId, branch, contexts).ToObservable().SelectMany(x => x); } + public IObservable GetAdminEnforcement(string owner, string name, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return _client.GetAdminEnforcement(owner, name, branch).ToObservable(); + } + + public IObservable GetAdminEnforcement(long repositoryId, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return _client.GetAdminEnforcement(repositoryId, branch).ToObservable(); + } + + public IObservable AddAdminEnforcement(string owner, string name, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return _client.AddAdminEnforcement(owner, name, branch).ToObservable(); + } + + public IObservable AddAdminEnforcement(long repositoryId, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return _client.AddAdminEnforcement(repositoryId, branch).ToObservable(); + } + + public IObservable RemoveAdminEnforcement(string owner, string name, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return _client.RemoveAdminEnforcement(owner, name, branch).ToObservable(); + } + + public IObservable RemoveAdminEnforcement(long repositoryId, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return _client.RemoveAdminEnforcement(repositoryId, branch).ToObservable(); + } + /// /// Get the restrictions for the specified branch (applies only to Organization owned repositories) /// diff --git a/Octokit/Clients/IRepositoryBranchesClient.cs b/Octokit/Clients/IRepositoryBranchesClient.cs index cd96d64654..551e4e7d2d 100644 --- a/Octokit/Clients/IRepositoryBranchesClient.cs +++ b/Octokit/Clients/IRepositoryBranchesClient.cs @@ -322,6 +322,69 @@ public interface IRepositoryBranchesClient /// The contexts to remove Task> DeleteRequiredStatusChecksContexts(long repositoryId, string branch, IReadOnlyList contexts); + /// + /// Get admin enforcement of protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + Task GetAdminEnforcement(string owner, string name, string branch); + + /// + /// Get admin enforcement of protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch + Task GetAdminEnforcement(long repositoryId, string branch); + + /// + /// Add admin enforcement to protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + Task AddAdminEnforcement(string owner, string name, string branch); + + /// + /// Add admin enforcement to protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch + Task AddAdminEnforcement(long repositoryId, string branch); + + /// + /// Remove admin enforcement on protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + Task RemoveAdminEnforcement(string owner, string name, string branch); + + /// + /// Remove admin enforcement on protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch + Task RemoveAdminEnforcement(long repositoryId, string branch); + /// /// Get restrictions for the specified branch (applies only to Organization owned repositories) /// diff --git a/Octokit/Clients/RepositoryBranchesClient.cs b/Octokit/Clients/RepositoryBranchesClient.cs index a2fa004965..0be10f900a 100644 --- a/Octokit/Clients/RepositoryBranchesClient.cs +++ b/Octokit/Clients/RepositoryBranchesClient.cs @@ -538,6 +538,75 @@ public Task> DeleteRequiredStatusChecksContexts(long repos return ApiConnection.Delete>(ApiUrls.RepoRequiredStatusChecksContexts(repositoryId, branch), contexts, AcceptHeaders.ProtectedBranchesApiPreview); } + public Task GetAdminEnforcement(string owner, string name, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return ApiConnection.Get(ApiUrls.RepoProtectedBranchAdminEnforcement(owner, name, branch), null, AcceptHeaders.ProtectedBranchesApiPreview); + } + + public Task GetAdminEnforcement(long repositoryId, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return ApiConnection.Get(ApiUrls.RepoProtectedBranchAdminEnforcement(repositoryId, branch), null, AcceptHeaders.ProtectedBranchesApiPreview); + + } + + public Task AddAdminEnforcement(string owner, string name, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return ApiConnection.Post(ApiUrls.RepoProtectedBranchAdminEnforcement(owner, name, branch), null, AcceptHeaders.ProtectedBranchesApiPreview); + } + + public Task AddAdminEnforcement(long repositoryId, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + return ApiConnection.Post(ApiUrls.RepoProtectedBranchAdminEnforcement(repositoryId, branch), null, AcceptHeaders.ProtectedBranchesApiPreview); + } + + public async Task RemoveAdminEnforcement(string owner, string name, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + var endpoint = ApiUrls.RepoProtectedBranchAdminEnforcement(owner, name, branch); + + try + { + var httpStatusCode = await Connection.Delete(endpoint, null, AcceptHeaders.ProtectedBranchesApiPreview).ConfigureAwait(false); + return httpStatusCode == HttpStatusCode.NoContent; + } + catch (NotFoundException) + { + return false; + } + } + + public async Task RemoveAdminEnforcement(long repositoryId, string branch) + { + Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); + + var endpoint = ApiUrls.RepoProtectedBranchAdminEnforcement(repositoryId, branch); + + try + { + var httpStatusCode = await Connection.Delete(endpoint, null, AcceptHeaders.ProtectedBranchesApiPreview).ConfigureAwait(false); + return httpStatusCode == HttpStatusCode.NoContent; + } + catch (NotFoundException) + { + return false; + } + } + /// /// Get restrictions for the specified branch (applies only to Organization owned repositories) /// diff --git a/Octokit/Helpers/ApiUrls.cs b/Octokit/Helpers/ApiUrls.cs index e9e465688d..587914da51 100644 --- a/Octokit/Helpers/ApiUrls.cs +++ b/Octokit/Helpers/ApiUrls.cs @@ -1733,6 +1733,27 @@ public static Uri RepoRequiredStatusChecksContexts(long repositoryId, string bra return "repositories/{0}/branches/{1}/protection/required_status_checks/contexts".FormatUri(repositoryId, branchName); } + /// + /// Returns the for admin enforcement for a protected branch + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch + public static Uri RepoProtectedBranchAdminEnforcement(string owner, string name, string branchName) + { + return "repos/{0}/{1}/branches/{2}/protection/enforce_admins".FormatUri(owner, name, branchName); + } + + /// + /// Returns the for admin enforcement for a protected branch + /// + /// The Id of the repository + /// The name of the branch + public static Uri RepoProtectedBranchAdminEnforcement(long repositoryId, string branchName) + { + return "repositories/{0}/branches/{1}/protection/enforce_admins".FormatUri(repositoryId, branchName); + } + /// /// Returns the for restrictions for a protected branch. /// From d50dfb851fd69c6292f1012f0cb2b9e5b0abdb31 Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Tue, 2 May 2017 16:31:47 +0300 Subject: [PATCH 03/11] Add unit tests --- .../Clients/RepositoryBranchesClientTests.cs | 141 ++++++++++++++++++ Octokit/Clients/RepositoryBranchesClient.cs | 4 +- 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs b/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs index e02e165250..258d06202b 100644 --- a/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs @@ -700,6 +700,147 @@ public async Task EnsuresNonNullArguments() } } + public class TheGetAdminEnforcementMethod + { + [Fact] + public void RequestsTheCorrectUrl() + { + var connection = Substitute.For(); + var client = new RepositoryBranchesClient(connection); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.GetAdminEnforcement("owner", "repo", "branch"); + + connection.Received() + .Get(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch/protection/enforce_admins"), null, previewAcceptsHeader); + } + + [Fact] + public void RequestsTheCorrectUrlWithRepositoryId() + { + var connection = Substitute.For(); + var client = new RepositoryBranchesClient(connection); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.GetAdminEnforcement(1, "branch"); + + connection.Received() + .Get(Arg.Is(u => u.ToString() == "repositories/1/branches/branch/protection/enforce_admins"), null, previewAcceptsHeader); + } + + [Fact] + public async Task EnsuresNonNullArguments() + { + var client = new RepositoryBranchesClient(Substitute.For()); + + await Assert.ThrowsAsync(() => client.GetAdminEnforcement(null, "repo", "branch")); + await Assert.ThrowsAsync(() => client.GetAdminEnforcement("owner", null, "branch")); + await Assert.ThrowsAsync(() => client.GetAdminEnforcement("owner", "repo", null)); + + await Assert.ThrowsAsync(() => client.GetAdminEnforcement(1, null)); + + await Assert.ThrowsAsync(() => client.GetAdminEnforcement("", "repo", "branch")); + await Assert.ThrowsAsync(() => client.GetAdminEnforcement("owner", "", "branch")); + await Assert.ThrowsAsync(() => client.GetAdminEnforcement("owner", "repo", "")); + + await Assert.ThrowsAsync(() => client.GetAdminEnforcement(1, "")); + } + } + + public class TheAddAdminEnforcement + { + [Fact] + public void RequestsTheCorrectUrl() + { + var connection = Substitute.For(); + var client = new RepositoryBranchesClient(connection); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.AddAdminEnforcement("owner", "repo", "branch"); + + connection.Received() + .Post(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch/protection/enforce_admins"), Arg.Any(), previewAcceptsHeader); + } + + [Fact] + public void RequestsTheCorrectUrlWithRepositoryId() + { + var connection = Substitute.For(); + var client = new RepositoryBranchesClient(connection); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.AddAdminEnforcement(1, "branch"); + + connection.Received() + .Post(Arg.Is(u => u.ToString() == "repositories/1/branches/branch/protection/enforce_admins"), Arg.Any(), previewAcceptsHeader); + } + + [Fact] + public async Task EnsuresNonNullArguments() + { + var client = new RepositoryBranchesClient(Substitute.For()); + + await Assert.ThrowsAsync(() => client.AddAdminEnforcement(null, "repo", "branch")); + await Assert.ThrowsAsync(() => client.AddAdminEnforcement("owner", null, "branch")); + await Assert.ThrowsAsync(() => client.AddAdminEnforcement("owner", "repo", null)); + + await Assert.ThrowsAsync(() => client.AddAdminEnforcement(1, null)); + + await Assert.ThrowsAsync(() => client.AddAdminEnforcement("", "repo", "branch")); + await Assert.ThrowsAsync(() => client.AddAdminEnforcement("owner", "", "branch")); + await Assert.ThrowsAsync(() => client.AddAdminEnforcement("owner", "repo", "")); + + await Assert.ThrowsAsync(() => client.AddAdminEnforcement(1, "")); + } + } + + public class TheRemoveAdminEnforcement + { + [Fact] + public void RequestsTheCorrectUrl() + { + var connection = Substitute.For(); + var client = new RepositoryBranchesClient(connection); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.RemoveAdminEnforcement("owner", "repo", "branch"); + + connection.Connection.Received() + .Delete(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch/protection/enforce_admins"), Arg.Any(), previewAcceptsHeader); + } + + [Fact] + public void RequestsTheCorrectUrlWithRepositoryId() + { + var connection = Substitute.For(); + var client = new RepositoryBranchesClient(connection); + const string previewAcceptsHeader = "application/vnd.github.loki-preview+json"; + + client.RemoveAdminEnforcement(1, "branch"); + + connection.Connection.Received() + .Delete(Arg.Is(u => u.ToString() == "repositories/1/branches/branch/protection/enforce_admins"), Arg.Any(), previewAcceptsHeader); + } + + [Fact] + public async Task EnsuresNonNullArguments() + { + var client = new RepositoryBranchesClient(Substitute.For()); + + await Assert.ThrowsAsync(() => client.RemoveAdminEnforcement(null, "repo", "branch")); + await Assert.ThrowsAsync(() => client.RemoveAdminEnforcement("owner", null, "branch")); + await Assert.ThrowsAsync(() => client.RemoveAdminEnforcement("owner", "repo", null)); + + await Assert.ThrowsAsync(() => client.RemoveAdminEnforcement(1, null)); + + await Assert.ThrowsAsync(() => client.RemoveAdminEnforcement("", "repo", "branch")); + await Assert.ThrowsAsync(() => client.RemoveAdminEnforcement("owner", "", "branch")); + await Assert.ThrowsAsync(() => client.RemoveAdminEnforcement("owner", "repo", "")); + + await Assert.ThrowsAsync(() => client.RemoveAdminEnforcement(1, "")); + } + } + public class TheGetProtectedBranchRestrictionsMethod { [Fact] diff --git a/Octokit/Clients/RepositoryBranchesClient.cs b/Octokit/Clients/RepositoryBranchesClient.cs index 0be10f900a..122bb34357 100644 --- a/Octokit/Clients/RepositoryBranchesClient.cs +++ b/Octokit/Clients/RepositoryBranchesClient.cs @@ -561,14 +561,14 @@ public Task AddAdminEnforcement(string owner, string name, string Ensure.ArgumentNotNullOrEmptyString(name, "name"); Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); - return ApiConnection.Post(ApiUrls.RepoProtectedBranchAdminEnforcement(owner, name, branch), null, AcceptHeaders.ProtectedBranchesApiPreview); + return ApiConnection.Post(ApiUrls.RepoProtectedBranchAdminEnforcement(owner, name, branch), new object(), AcceptHeaders.ProtectedBranchesApiPreview); } public Task AddAdminEnforcement(long repositoryId, string branch) { Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); - return ApiConnection.Post(ApiUrls.RepoProtectedBranchAdminEnforcement(repositoryId, branch), null, AcceptHeaders.ProtectedBranchesApiPreview); + return ApiConnection.Post(ApiUrls.RepoProtectedBranchAdminEnforcement(repositoryId, branch), new object(), AcceptHeaders.ProtectedBranchesApiPreview); } public async Task RemoveAdminEnforcement(string owner, string name, string branch) From 89d447fec0d662527853ffc3ca76415d52ae9fd6 Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Tue, 2 May 2017 16:46:08 +0300 Subject: [PATCH 04/11] Add unit tests for Observable client --- ...ObservableRepositoryBranchesClientTests.cs | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/Octokit.Tests/Reactive/ObservableRepositoryBranchesClientTests.cs b/Octokit.Tests/Reactive/ObservableRepositoryBranchesClientTests.cs index d49f302ee0..78e2ec2327 100644 --- a/Octokit.Tests/Reactive/ObservableRepositoryBranchesClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableRepositoryBranchesClientTests.cs @@ -672,6 +672,135 @@ public async Task EnsuresNonNullArguments() } } + public class TheGetAdminEnforcementMethod + { + [Fact] + public void RequestsTheCorrectUrl() + { + var gitHubClient = Substitute.For(); + var client = new ObservableRepositoryBranchesClient(gitHubClient); + + client.GetAdminEnforcement("owner", "repo", "branch"); + + gitHubClient.Repository.Branch.Received().GetAdminEnforcement("owner", "repo", "branch"); + } + + [Fact] + public void RequestsTheCorrectUrlWithRepositoryId() + { + var gitHubClient = Substitute.For(); + var client = new ObservableRepositoryBranchesClient(gitHubClient); + + client.GetAdminEnforcement(1, "branch"); + + gitHubClient.Repository.Branch.Received().GetAdminEnforcement(1, "branch"); + } + + [Fact] + public async Task EnsuresNonNullArguments() + { + var client = new ObservableRepositoryBranchesClient(Substitute.For()); + + Assert.Throws(() => client.GetAdminEnforcement(null, "repo", "branch")); + Assert.Throws(() => client.GetAdminEnforcement("owner", null, "branch")); + Assert.Throws(() => client.GetAdminEnforcement("owner", "repo", null)); + + Assert.Throws(() => client.GetAdminEnforcement(1, null)); + + Assert.Throws(() => client.GetAdminEnforcement("", "repo", "branch")); + Assert.Throws(() => client.GetAdminEnforcement("owner", "", "branch")); + Assert.Throws(() => client.GetAdminEnforcement("owner", "repo", "")); + + Assert.Throws(() => client.GetAdminEnforcement(1, "")); + } + } + + public class TheAddAdminEnforcement + { + [Fact] + public void RequestsTheCorrectUrl() + { + var gitHubClient = Substitute.For(); + var client = new ObservableRepositoryBranchesClient(gitHubClient); + + client.AddAdminEnforcement("owner", "repo", "branch"); + + gitHubClient.Repository.Branch.Received().AddAdminEnforcement("owner", "repo", "branch"); + } + + [Fact] + public void RequestsTheCorrectUrlWithRepositoryId() + { + var gitHubClient = Substitute.For(); + var client = new ObservableRepositoryBranchesClient(gitHubClient); + + client.AddAdminEnforcement(1, "branch"); + + gitHubClient.Repository.Branch.Received().AddAdminEnforcement(1, "branch"); + } + + [Fact] + public async Task EnsuresNonNullArguments() + { + var client = new ObservableRepositoryBranchesClient(Substitute.For()); + + Assert.Throws(() => client.AddAdminEnforcement(null, "repo", "branch")); + Assert.Throws(() => client.AddAdminEnforcement("owner", null, "branch")); + Assert.Throws(() => client.AddAdminEnforcement("owner", "repo", null)); + + Assert.Throws(() => client.AddAdminEnforcement(1, null)); + + Assert.Throws(() => client.AddAdminEnforcement("", "repo", "branch")); + Assert.Throws(() => client.AddAdminEnforcement("owner", "", "branch")); + Assert.Throws(() => client.AddAdminEnforcement("owner", "repo", "")); + + Assert.Throws(() => client.AddAdminEnforcement(1, "")); + } + } + + public class TheRemoveAdminEnforcement + { + [Fact] + public void RequestsTheCorrectUrl() + { + var gitHubClient = Substitute.For(); + var client = new ObservableRepositoryBranchesClient(gitHubClient); + + client.RemoveAdminEnforcement("owner", "repo", "branch"); + + gitHubClient.Repository.Branch.Received().RemoveAdminEnforcement("owner", "repo", "branch"); + } + + [Fact] + public void RequestsTheCorrectUrlWithRepositoryId() + { + var gitHubClient = Substitute.For(); + var client = new ObservableRepositoryBranchesClient(gitHubClient); + + client.RemoveAdminEnforcement(1, "branch"); + + gitHubClient.Repository.Branch.Received().RemoveAdminEnforcement(1, "branch"); + } + + [Fact] + public async Task EnsuresNonNullArguments() + { + var client = new ObservableRepositoryBranchesClient(Substitute.For()); + + Assert.Throws(() => client.RemoveAdminEnforcement(null, "repo", "branch")); + Assert.Throws(() => client.RemoveAdminEnforcement("owner", null, "branch")); + Assert.Throws(() => client.RemoveAdminEnforcement("owner", "repo", null)); + + Assert.Throws(() => client.RemoveAdminEnforcement(1, null)); + + Assert.Throws(() => client.RemoveAdminEnforcement("", "repo", "branch")); + Assert.Throws(() => client.RemoveAdminEnforcement("owner", "", "branch")); + Assert.Throws(() => client.RemoveAdminEnforcement("owner", "repo", "")); + + Assert.Throws(() => client.RemoveAdminEnforcement(1, "")); + } + } + public class TheGetProtectedBranchRestrictionsMethod { [Fact] From 649b11b041f050a8392f49522d993037bfb96837 Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Tue, 2 May 2017 17:14:09 +0300 Subject: [PATCH 05/11] Add integration tests for enforce admin methods --- .../Clients/RepositoryBranchesClientTests.cs | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs index c4ebbb7f3c..ec7bf0cdd8 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs @@ -846,6 +846,141 @@ public async Task DeletesRequiredStatusChecksContextsWithRepositoryId() } } + public class TheGetAdminEnforcementMethod : IDisposable + { + private readonly IRepositoryBranchesClient _client; + private readonly RepositoryContext _userRepoContext; + + public TheGetAdminEnforcementMethod() + { + var github = Helper.GetAuthenticatedClient(); + _client = github.Repository.Branch; + + _userRepoContext = github.CreateRepositoryWithProtectedBranch().Result; + } + + [IntegrationTest] + public async Task GetAdminEnforcement() + { + var repoOwner = _userRepoContext.RepositoryOwner; + var repoName = _userRepoContext.RepositoryName; + var enforceAdmins = await _client.GetAdminEnforcement(repoOwner, repoName, "master"); + + Assert.NotNull(enforceAdmins); + Assert.True(enforceAdmins.Enabled); + } + + [IntegrationTest] + public async Task GetAdminEnforcementWithRepositoryId() + { + var repoId = _userRepoContext.RepositoryId; + var enforceAdmins = await _client.GetAdminEnforcement(repoId, "master"); + + Assert.NotNull(enforceAdmins); + Assert.True(enforceAdmins.Enabled); + } + + public void Dispose() + { + if (_userRepoContext != null) + { + _userRepoContext.Dispose(); + } + } + } + + public class TheAddAdminEnforcementMethod : IDisposable + { + private readonly IRepositoryBranchesClient _client; + private readonly RepositoryContext _userRepoContext; + + public TheAddAdminEnforcementMethod() + { + var github = Helper.GetAuthenticatedClient(); + _client = github.Repository.Branch; + + _userRepoContext = github.CreateRepositoryWithProtectedBranch().Result; + } + + [IntegrationTest] + public async Task AddAdminEnforcement() + { + var repoOwner = _userRepoContext.RepositoryOwner; + var repoName = _userRepoContext.RepositoryName; + + var enforceAdmins = await _client.AddAdminEnforcement(repoOwner, repoName, "master"); + + Assert.NotNull(enforceAdmins); + Assert.True(enforceAdmins.Enabled); + } + + [IntegrationTest] + public async Task AddAdminEnforcementoWithRepositoryId() + { + var repoId = _userRepoContext.RepositoryId; + + var enforceAdmins = await _client.AddAdminEnforcement(repoId, "master"); + + Assert.NotNull(enforceAdmins); + Assert.True(enforceAdmins.Enabled); + } + + public void Dispose() + { + if (_userRepoContext != null) + { + _userRepoContext.Dispose(); + } + } + } + + public class TheRemoveAdminEnforcementMethod + { + private readonly IRepositoryBranchesClient _client; + private readonly IGitHubClient _github; + + public TheRemoveAdminEnforcementMethod() + { + _github = Helper.GetAuthenticatedClient(); + _client = _github.Repository.Branch; + } + + [IntegrationTest] + public async Task RemoveAdminEnforcement() + { + using (var context = await _github.CreateRepositoryWithProtectedBranch()) + { + var repoOwner = context.RepositoryOwner; + var repoName = context.RepositoryName; + var deleted = await _client.RemoveAdminEnforcement(repoOwner, repoName, "master"); + + Assert.True(deleted); + + var enforceAdmins = await _client.GetAdminEnforcement(repoOwner, repoName, "master"); + + Assert.NotNull(enforceAdmins); + Assert.False(enforceAdmins.Enabled); + } + } + + [IntegrationTest] + public async Task RemoveAdminEnforcementWithRepositoryId() + { + using (var context = await _github.CreateRepositoryWithProtectedBranch()) + { + var repoId = context.RepositoryId; + var deleted = await _client.RemoveAdminEnforcement(repoId, "master"); + + Assert.True(deleted); + + var enforceAdmins = await _client.GetAdminEnforcement(repoId, "master"); + + Assert.NotNull(enforceAdmins); + Assert.False(enforceAdmins.Enabled); + } + } + } + public class TheGetProtectedBranchRestrictionsMethod : IDisposable { IRepositoryBranchesClient _client; From 16fc3b55a897756061ad5bbac1bbcc4d648b58d9 Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Tue, 2 May 2017 17:24:09 +0300 Subject: [PATCH 06/11] Tweak integration test to ensure that they actually do something The `CreateRepositoryWithProtectedBranch` helper method currently sets `EnforceAdmins` as true, so delete it before adding. --- .../Clients/RepositoryBranchesClientTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs index ec7bf0cdd8..ba5ba66a9a 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs @@ -908,6 +908,7 @@ public async Task AddAdminEnforcement() var repoOwner = _userRepoContext.RepositoryOwner; var repoName = _userRepoContext.RepositoryName; + await _client.RemoveAdminEnforcement(repoOwner, repoName, "master"); var enforceAdmins = await _client.AddAdminEnforcement(repoOwner, repoName, "master"); Assert.NotNull(enforceAdmins); @@ -919,6 +920,7 @@ public async Task AddAdminEnforcementoWithRepositoryId() { var repoId = _userRepoContext.RepositoryId; + await _client.RemoveAdminEnforcement(repoId, "master"); var enforceAdmins = await _client.AddAdminEnforcement(repoId, "master"); Assert.NotNull(enforceAdmins); From 7ebfb877f5442429a98860c5d66ba6da9fe5cc37 Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Tue, 2 May 2017 18:04:27 +0300 Subject: [PATCH 07/11] add missing docs --- .../ObservableRepositoryBranchesClient.cs | 43 ++++++++++++++++ Octokit/Clients/RepositoryBranchesClient.cs | 51 +++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/Octokit.Reactive/Clients/ObservableRepositoryBranchesClient.cs b/Octokit.Reactive/Clients/ObservableRepositoryBranchesClient.cs index 7259294881..3973cda521 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoryBranchesClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoryBranchesClient.cs @@ -499,6 +499,15 @@ public IObservable DeleteRequiredStatusChecksContexts(long repositoryId, return _client.DeleteRequiredStatusChecksContexts(repositoryId, branch, contexts).ToObservable().SelectMany(x => x); } + /// + /// Get admin enforcement of protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch public IObservable GetAdminEnforcement(string owner, string name, string branch) { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); @@ -508,6 +517,14 @@ public IObservable GetAdminEnforcement(string owner, string name, return _client.GetAdminEnforcement(owner, name, branch).ToObservable(); } + /// + /// Get admin enforcement of protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch public IObservable GetAdminEnforcement(long repositoryId, string branch) { Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); @@ -515,6 +532,15 @@ public IObservable GetAdminEnforcement(long repositoryId, string return _client.GetAdminEnforcement(repositoryId, branch).ToObservable(); } + /// + /// Add admin enforcement to protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch public IObservable AddAdminEnforcement(string owner, string name, string branch) { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); @@ -531,6 +557,15 @@ public IObservable AddAdminEnforcement(long repositoryId, string return _client.AddAdminEnforcement(repositoryId, branch).ToObservable(); } + /// + /// Remove admin enforcement on protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch public IObservable RemoveAdminEnforcement(string owner, string name, string branch) { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); @@ -540,6 +575,14 @@ public IObservable RemoveAdminEnforcement(string owner, string name, strin return _client.RemoveAdminEnforcement(owner, name, branch).ToObservable(); } + /// + /// Remove admin enforcement on protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch public IObservable RemoveAdminEnforcement(long repositoryId, string branch) { Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); diff --git a/Octokit/Clients/RepositoryBranchesClient.cs b/Octokit/Clients/RepositoryBranchesClient.cs index 122bb34357..2ac509ecba 100644 --- a/Octokit/Clients/RepositoryBranchesClient.cs +++ b/Octokit/Clients/RepositoryBranchesClient.cs @@ -538,6 +538,15 @@ public Task> DeleteRequiredStatusChecksContexts(long repos return ApiConnection.Delete>(ApiUrls.RepoRequiredStatusChecksContexts(repositoryId, branch), contexts, AcceptHeaders.ProtectedBranchesApiPreview); } + /// + /// Get admin enforcement of protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch public Task GetAdminEnforcement(string owner, string name, string branch) { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); @@ -547,6 +556,14 @@ public Task GetAdminEnforcement(string owner, string name, string return ApiConnection.Get(ApiUrls.RepoProtectedBranchAdminEnforcement(owner, name, branch), null, AcceptHeaders.ProtectedBranchesApiPreview); } + /// + /// Get admin enforcement of protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch public Task GetAdminEnforcement(long repositoryId, string branch) { Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); @@ -555,6 +572,15 @@ public Task GetAdminEnforcement(long repositoryId, string branch) } + /// + /// Add admin enforcement to protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch public Task AddAdminEnforcement(string owner, string name, string branch) { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); @@ -564,6 +590,14 @@ public Task AddAdminEnforcement(string owner, string name, string return ApiConnection.Post(ApiUrls.RepoProtectedBranchAdminEnforcement(owner, name, branch), new object(), AcceptHeaders.ProtectedBranchesApiPreview); } + /// + /// Add admin enforcement to protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch public Task AddAdminEnforcement(long repositoryId, string branch) { Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); @@ -571,6 +605,15 @@ public Task AddAdminEnforcement(long repositoryId, string branch) return ApiConnection.Post(ApiUrls.RepoProtectedBranchAdminEnforcement(repositoryId, branch), new object(), AcceptHeaders.ProtectedBranchesApiPreview); } + /// + /// Remove admin enforcement on protected branch + /// + /// + /// See the API documentation for more details + /// + /// The owner of the repository + /// The name of the repository + /// The name of the branch public async Task RemoveAdminEnforcement(string owner, string name, string branch) { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); @@ -590,6 +633,14 @@ public async Task RemoveAdminEnforcement(string owner, string name, string } } + /// + /// Remove admin enforcement on protected branch + /// + /// + /// See the API documentation for more details + /// + /// The Id of the repository + /// The name of the branch public async Task RemoveAdminEnforcement(long repositoryId, string branch) { Ensure.ArgumentNotNullOrEmptyString(branch, "branch"); From e182454a4f35ab834c8b000c913eded6c695759c Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Tue, 2 May 2017 18:04:33 +0300 Subject: [PATCH 08/11] rename tests --- .../Clients/RepositoryBranchesClientTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs index ba5ba66a9a..70846d6f41 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs @@ -860,7 +860,7 @@ public TheGetAdminEnforcementMethod() } [IntegrationTest] - public async Task GetAdminEnforcement() + public async Task GetsAdminEnforcement() { var repoOwner = _userRepoContext.RepositoryOwner; var repoName = _userRepoContext.RepositoryName; @@ -871,7 +871,7 @@ public async Task GetAdminEnforcement() } [IntegrationTest] - public async Task GetAdminEnforcementWithRepositoryId() + public async Task GetsAdminEnforcementWithRepositoryId() { var repoId = _userRepoContext.RepositoryId; var enforceAdmins = await _client.GetAdminEnforcement(repoId, "master"); @@ -903,7 +903,7 @@ public TheAddAdminEnforcementMethod() } [IntegrationTest] - public async Task AddAdminEnforcement() + public async Task AddsAdminEnforcement() { var repoOwner = _userRepoContext.RepositoryOwner; var repoName = _userRepoContext.RepositoryName; @@ -916,7 +916,7 @@ public async Task AddAdminEnforcement() } [IntegrationTest] - public async Task AddAdminEnforcementoWithRepositoryId() + public async Task AddsAdminEnforcementoWithRepositoryId() { var repoId = _userRepoContext.RepositoryId; @@ -948,7 +948,7 @@ public TheRemoveAdminEnforcementMethod() } [IntegrationTest] - public async Task RemoveAdminEnforcement() + public async Task RemovesAdminEnforcement() { using (var context = await _github.CreateRepositoryWithProtectedBranch()) { @@ -966,7 +966,7 @@ public async Task RemoveAdminEnforcement() } [IntegrationTest] - public async Task RemoveAdminEnforcementWithRepositoryId() + public async Task RemovesAdminEnforcementWithRepositoryId() { using (var context = await _github.CreateRepositoryWithProtectedBranch()) { From ffb185e0b059e149ee842acabaee07bddfcd9883 Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Tue, 2 May 2017 18:05:05 +0300 Subject: [PATCH 09/11] Add missing ctor --- Octokit/Models/Request/BranchProtectionUpdate.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Octokit/Models/Request/BranchProtectionUpdate.cs b/Octokit/Models/Request/BranchProtectionUpdate.cs index ecc5862be4..0679ae0421 100644 --- a/Octokit/Models/Request/BranchProtectionUpdate.cs +++ b/Octokit/Models/Request/BranchProtectionUpdate.cs @@ -16,6 +16,17 @@ namespace Octokit [DebuggerDisplay("{DebuggerDisplay,nq}")] public class BranchProtectionSettingsUpdate { + /// + /// Create a BranchProtection update request + /// + /// Specifies whether the protections applied to this branch also apply to repository admins + public BranchProtectionSettingsUpdate(bool enforceAdmins) + { + EnforceAdmins = enforceAdmins; + RequiredStatusChecks = null; + Restrictions = null; + } + /// /// Create a BranchProtection update request /// From 969fb9e497af35a88a7366784f3c745214d2e9dc Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Thu, 4 May 2017 14:23:31 +0300 Subject: [PATCH 10/11] Remove property that is no longer supported https://developer.github.com/changes/2017-05-02-adoption-of-admin-enforced/ --- Octokit/Models/Request/BranchProtectionUpdate.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Octokit/Models/Request/BranchProtectionUpdate.cs b/Octokit/Models/Request/BranchProtectionUpdate.cs index 0679ae0421..6f9d06196a 100644 --- a/Octokit/Models/Request/BranchProtectionUpdate.cs +++ b/Octokit/Models/Request/BranchProtectionUpdate.cs @@ -107,12 +107,6 @@ public BranchProtectionRequiredStatusChecksUpdate(bool strict, IReadOnlyList - /// Enforce required status checks for repository administrators - /// - [Obsolete("This property is obsolete. Use EnforceAdmins on the BranchProtectionUpdate class")] - public bool IncludeAdmins { get; protected set; } - /// /// Require branches to be up to date before merging /// From dc9ebfb73e7a939968700e0dc6883e6fa407e499 Mon Sep 17 00:00:00 2001 From: Mordechai Zuber Date: Thu, 4 May 2017 14:25:15 +0300 Subject: [PATCH 11/11] Fix failing unit tests --- Octokit.Tests/Clients/RepositoryBranchesClientTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs b/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs index 258d06202b..bcec09a252 100644 --- a/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoryBranchesClientTests.cs @@ -806,7 +806,7 @@ public void RequestsTheCorrectUrl() client.RemoveAdminEnforcement("owner", "repo", "branch"); connection.Connection.Received() - .Delete(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch/protection/enforce_admins"), Arg.Any(), previewAcceptsHeader); + .Delete(Arg.Is(u => u.ToString() == "repos/owner/repo/branches/branch/protection/enforce_admins"), Arg.Any(), previewAcceptsHeader); } [Fact] @@ -819,7 +819,7 @@ public void RequestsTheCorrectUrlWithRepositoryId() client.RemoveAdminEnforcement(1, "branch"); connection.Connection.Received() - .Delete(Arg.Is(u => u.ToString() == "repositories/1/branches/branch/protection/enforce_admins"), Arg.Any(), previewAcceptsHeader); + .Delete(Arg.Is(u => u.ToString() == "repositories/1/branches/branch/protection/enforce_admins"), Arg.Any(), previewAcceptsHeader); } [Fact]