diff --git a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs index b00bc5ff..39542e61 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryBranchesClientTests.cs @@ -442,7 +442,7 @@ public class RepositoryBranchesClientTests var repoName = _orgRepoContext.RepositoryContext.RepositoryName; var update = new BranchProtectionSettingsUpdate( new BranchProtectionRequiredStatusChecksUpdate(false, false, new[] { "new" }), - null); + new BranchProtectionPushRestrictionsUpdate()); var protection = await _client.UpdateBranchProtection(repoOwner, repoName, "master", update); @@ -450,7 +450,8 @@ public class RepositoryBranchesClientTests Assert.False(protection.RequiredStatusChecks.Strict); Assert.Equal(1, protection.RequiredStatusChecks.Contexts.Count); - Assert.Null(protection.Restrictions); + Assert.Empty(protection.Restrictions.Teams); + Assert.Empty(protection.Restrictions.Users); } [IntegrationTest] @@ -459,7 +460,7 @@ public class RepositoryBranchesClientTests var repoId = _orgRepoContext.RepositoryContext.RepositoryId; var update = new BranchProtectionSettingsUpdate( new BranchProtectionRequiredStatusChecksUpdate(false, false, new[] { "new" }), - null); + new BranchProtectionPushRestrictionsUpdate()); var protection = await _client.UpdateBranchProtection(repoId, "master", update); @@ -467,7 +468,8 @@ public class RepositoryBranchesClientTests Assert.False(protection.RequiredStatusChecks.Strict); Assert.Equal(1, protection.RequiredStatusChecks.Contexts.Count); - Assert.Null(protection.Restrictions); + Assert.Empty(protection.Restrictions.Teams); + Assert.Empty(protection.Restrictions.Users); } public void Dispose() diff --git a/Octokit.Tests.Integration/Helpers/OrganizationRepositoryWithTeamContext.cs b/Octokit.Tests.Integration/Helpers/OrganizationRepositoryWithTeamContext.cs index 49bcedae..67600606 100644 --- a/Octokit.Tests.Integration/Helpers/OrganizationRepositoryWithTeamContext.cs +++ b/Octokit.Tests.Integration/Helpers/OrganizationRepositoryWithTeamContext.cs @@ -55,10 +55,10 @@ namespace Octokit.Tests.Integration.Helpers new RepositoryPermissionRequest(Permission.Push)); // Protect master branch - var orgUpdate = new BranchProtectionSettingsUpdate( + var protection = new BranchProtectionSettingsUpdate( new BranchProtectionRequiredStatusChecksUpdate(true, true, new[] { "build", "test" }), - new ProtectedBranchRestrictionsUpdate(new[] { contextOrgTeam.TeamName }, new string[0])); - await client.Repository.Branch.UpdateBranchProtection(contextOrgRepo.RepositoryOwner, contextOrgRepo.RepositoryName, "master", orgUpdate); + new BranchProtectionPushRestrictionsUpdate(new BranchProtectionTeamCollection { contextOrgTeam.TeamName })); + await client.Repository.Branch.UpdateBranchProtection(contextOrgRepo.RepositoryOwner, contextOrgRepo.RepositoryName, "master", protection); return new OrganizationRepositoryWithTeamContext { diff --git a/Octokit/Models/Request/BranchProtectionUpdate.cs b/Octokit/Models/Request/BranchProtectionUpdate.cs index 76ca4465..46cb2de5 100644 --- a/Octokit/Models/Request/BranchProtectionUpdate.cs +++ b/Octokit/Models/Request/BranchProtectionUpdate.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Diagnostics; using System.Globalization; using Octokit.Internal; @@ -30,7 +31,7 @@ namespace Octokit /// /// 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, ProtectedBranchRestrictionsUpdate restrictions) + public BranchProtectionSettingsUpdate(BranchProtectionRequiredStatusChecksUpdate requiredStatusChecks, BranchProtectionPushRestrictionsUpdate restrictions) { RequiredStatusChecks = requiredStatusChecks; Restrictions = restrictions; @@ -46,13 +47,16 @@ namespace Octokit /// Push access restrictions for the protected branch /// [SerializeNull] - public ProtectedBranchRestrictionsUpdate Restrictions { get; protected set; } + public BranchProtectionPushRestrictionsUpdate Restrictions { get; protected set; } internal string DebuggerDisplay { get { - return String.Format(CultureInfo.InvariantCulture, "StatusChecks: {0} Restrictions: {1}", RequiredStatusChecks.DebuggerDisplay, Restrictions.DebuggerDisplay); + return String.Format(CultureInfo.InvariantCulture, + "StatusChecks: {0} Restrictions: {1}", + RequiredStatusChecks == null ? "disabled" : RequiredStatusChecks.DebuggerDisplay, + Restrictions == null ? "disabled" : Restrictions.DebuggerDisplay); } } } @@ -101,17 +105,50 @@ namespace Octokit } /// - /// Specifies people or teams allowed to push to the protected branch. Required status checks will still prevent these people from merging if the checks fail + /// Specifies teams and/or people allowed to push to the protected branch. Required status checks will still prevent these people from merging if the checks fail /// [DebuggerDisplay("{DebuggerDisplay,nq}")] - public class ProtectedBranchRestrictionsUpdate + public class BranchProtectionPushRestrictionsUpdate { /// - /// Specify people or teams (in addition to Administrators) allowed to push to this branch. Required status checks will still prevent these people from merging if the checks fail + /// Specify only administrators are allowed to push to this branch. Required status checks will still prevent these people from merging if the checks fail /// - /// Teams allowed to push to this branch (pass empty array if no teams) - /// Users allowed to push to this branch (pass empty array if no users) - public ProtectedBranchRestrictionsUpdate(IReadOnlyList teams, IReadOnlyList users) + public BranchProtectionPushRestrictionsUpdate() + { + Teams = new BranchProtectionTeamCollection(); + Users = new BranchProtectionUserCollection(); + } + + /// + /// Specify teams (in addition to Administrators) allowed to push to this branch. Required status checks will still prevent these people from merging if the checks fail + /// + /// Teams allowed to push to this branch + public BranchProtectionPushRestrictionsUpdate(BranchProtectionTeamCollection teams) + { + Ensure.ArgumentNotNull(teams, "teams"); + + Teams = teams; + Users = new BranchProtectionUserCollection(); + } + + /// + /// Specify people (in addition to Administrators) allowed to push to this branch. Required status checks will still prevent these people from merging if the checks fail + /// + /// Users allowed to push to this branch + public BranchProtectionPushRestrictionsUpdate(BranchProtectionUserCollection users) + { + Ensure.ArgumentNotNull(users, "users"); + + Teams = new BranchProtectionTeamCollection(); + Users = users; + } + + /// + /// Specify teams and/or people (in addition to Administrators) allowed to push to this branch. Required status checks will still prevent these people from merging if the checks fail + /// + /// Teams allowed to push to this branch + /// Users allowed to push to this branch + public BranchProtectionPushRestrictionsUpdate(BranchProtectionTeamCollection teams, BranchProtectionUserCollection users) { Ensure.ArgumentNotNull(teams, "teams"); Ensure.ArgumentNotNull(users, "users"); @@ -123,18 +160,57 @@ namespace Octokit /// /// Teams allowed to push to this branch /// - public IReadOnlyList Teams { get; private set; } + public BranchProtectionTeamCollection Teams { get; private set; } /// /// Users allowed to push to this branch /// - public IReadOnlyList Users { get; private set; } + public BranchProtectionUserCollection Users { get; private set; } internal string DebuggerDisplay { get { - return String.Format(CultureInfo.InvariantCulture, "Teams: {0} Users: {1}", Teams == null ? "" : String.Join(",", Teams), Users == null ? "" : String.Join(",", Users)); + return String.Format(CultureInfo.InvariantCulture, + "Teams: {0} Users: {1}", + Teams == null ? "" : Teams.DebuggerDisplay, + Users == null ? "" : Users.DebuggerDisplay); + } + } + } + + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class BranchProtectionTeamCollection : Collection + { + public BranchProtectionTeamCollection() + { } + + public BranchProtectionTeamCollection(IList list) : base(list) + { } + + internal string DebuggerDisplay + { + get + { + return string.Format(CultureInfo.InvariantCulture, String.Join(", ", this)); + } + } + } + + [DebuggerDisplay("{DebuggerDisplay,nq}")] + public class BranchProtectionUserCollection : Collection + { + public BranchProtectionUserCollection() + { } + + public BranchProtectionUserCollection(IList list) : base(list) + { } + + internal string DebuggerDisplay + { + get + { + return string.Format(CultureInfo.InvariantCulture, String.Join(", ", this)); } } } diff --git a/Octokit/Models/Response/BranchProtection.cs b/Octokit/Models/Response/BranchProtection.cs index 98225ae6..3ef2d06f 100644 --- a/Octokit/Models/Response/BranchProtection.cs +++ b/Octokit/Models/Response/BranchProtection.cs @@ -108,7 +108,7 @@ namespace Octokit { public BranchProtectionSettings() { } - public BranchProtectionSettings(BranchProtectionRequiredStatusChecks requiredStatusChecks, ProtectedBranchRestrictions restrictions) + public BranchProtectionSettings(BranchProtectionRequiredStatusChecks requiredStatusChecks, BranchProtectionPushRestrictions restrictions) { RequiredStatusChecks = requiredStatusChecks; Restrictions = restrictions; @@ -122,7 +122,7 @@ namespace Octokit /// /// Push access restrictions for the protected branch /// - public ProtectedBranchRestrictions Restrictions { get; protected set; } + public BranchProtectionPushRestrictions Restrictions { get; protected set; } internal string DebuggerDisplay { @@ -183,11 +183,11 @@ namespace Octokit /// Specifies people or teams allowed to push to the protected branch. Required status checks will still prevent these people from merging if the checks fail /// [DebuggerDisplay("{DebuggerDisplay,nq}")] - public class ProtectedBranchRestrictions + public class BranchProtectionPushRestrictions { - public ProtectedBranchRestrictions() { } + public BranchProtectionPushRestrictions() { } - public ProtectedBranchRestrictions(IReadOnlyList teams, IReadOnlyList users) + public BranchProtectionPushRestrictions(IReadOnlyList teams, IReadOnlyList users) { Teams = teams; Users = users;