Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update models with updated permission enum #2633

Merged
merged 47 commits into from
Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
409c586
update models with updated permission enum
notauserx Dec 5, 2022
ce9df33
add suppress message attribute
notauserx Dec 6, 2022
0eed292
update integration tests
notauserx Dec 6, 2022
9b6164a
refactor: new and legacy update teams endpint
notauserx Dec 6, 2022
1ad585e
refactor: add new delete team endpoint
notauserx Dec 6, 2022
9c45e17
Merge branch 'main' into update-team-permissions-enums
kfcampbell Dec 6, 2022
3a3dbdd
use TeamPermission on NewTeam
notauserx Dec 7, 2022
754bece
use updated delete on team context dispose
notauserx Dec 7, 2022
82593bd
Merge branch 'update-team-permissions-enums' of https://github.com/no…
notauserx Dec 7, 2022
b354217
add permission enum for team response object
notauserx Dec 7, 2022
a976530
refactor: remove legacy suffix from method names
notauserx Dec 7, 2022
c22a76d
introduce permissions object on Team
notauserx Dec 7, 2022
c5f0cf4
refactor: rename enum to TeamRepositoryPermission
notauserx Dec 7, 2022
3b16170
fix formatting
notauserx Dec 7, 2022
76cfbff
change Permission to string to match api specs
notauserx Dec 8, 2022
d05c0cd
add TeamRepository
notauserx Dec 8, 2022
baee285
add CheckTeamPermission endpoint support
notauserx Dec 9, 2022
1c6313d
fix convention tests
notauserx Dec 9, 2022
a6be98d
update comments on TeamRepository props
notauserx Dec 9, 2022
d6f345b
add two new endpoints in TeamsClient
notauserx Dec 9, 2022
c01bb60
refactor: rename ApiUrl for TeamPermission
notauserx Dec 9, 2022
d0f4f0c
fix test
notauserx Dec 9, 2022
cba9141
implement methods for new endpoints
notauserx Dec 9, 2022
426abdf
add the integration tests
notauserx Dec 9, 2022
19dd149
fix spelling
notauserx Dec 9, 2022
38b0881
update comments
notauserx Dec 9, 2022
de786e0
refactor: rename method name
notauserx Dec 9, 2022
a76d81a
Merge branch 'main' into update-team-permissions-enums
notauserx Dec 9, 2022
e38ec35
Merge branch 'update-team-permissions-enums' of https://github.com/no…
notauserx Dec 9, 2022
d194b68
fix: add end tag for remarks
notauserx Dec 9, 2022
28a36d6
refactor: remove unused method param
notauserx Dec 9, 2022
662b40a
fix docstring comment
notauserx Dec 11, 2022
2a6497f
the unit tests are in finally
notauserx Dec 11, 2022
d40e981
add docs for teams api
notauserx Dec 11, 2022
f98829f
Merge branch 'main' into update-team-permissions-enums
kfcampbell Dec 12, 2022
a4deb95
split CheckTeamPermissions into two methods
notauserx Dec 13, 2022
f3d6980
Update ObservableTeamsClientTests.cs based on review
notauserx Dec 15, 2022
960382c
add cref to legacy update and delete endpoints
notauserx Dec 15, 2022
62de913
remove editorconfig file
notauserx Dec 16, 2022
7f1bd0e
Merge branch 'main' into update-team-permissions-enums
kfcampbell Jan 3, 2023
b13a43c
Merge branch 'main' into update-team-permissions-enums
kfcampbell Jan 5, 2023
4e2c833
Update Octokit.Tests/Clients/TeamsClientTests.cs
notauserx Jan 6, 2023
aa6788f
remove unused line
notauserx Jan 6, 2023
ca7de3b
rename variable based on review
notauserx Jan 6, 2023
c73545b
rename prop to match constructor param
notauserx Jan 6, 2023
2ec885d
add comment to explain TeamPermission enum values on update
notauserx Jan 7, 2023
31a2e9c
Merge branch 'main' into update-team-permissions-enums
notauserx Jan 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
root=true
[*.cs]
end_of_line = lf
6 changes: 6 additions & 0 deletions Octokit.Tests.Integration/Clients/TeamsClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public async Task CreatesTeam()
Assert.Equal(teamName, team.Name);
Assert.Equal(teamDescription, team.Description);
Assert.Equal(TeamPrivacy.Closed, team.Privacy);
Assert.Equal(TeamPermission.Pull, team.Permission);
Assert.Equal(1, team.MembersCount);
Assert.Equal(1, team.ReposCount);

Expand Down Expand Up @@ -445,10 +446,14 @@ public async Task UpdatesTeam()
{
var teamName = Helper.MakeNameWithTimestamp("updated-team");
var teamDescription = Helper.MakeNameWithTimestamp("updated description");

// setting TeamPermission.Admin fails with Octokit.ApiValidationException : Setting team permission to admin is no longer supported
var teamPermission = TeamPermission.Push;
Copy link
Contributor Author

@notauserx notauserx Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that admin is not being accepted as a permission value, even though the docs say it can be.

I can get this test passing with Pull and Push values, but passing admin just fails with

setting TeamPermission.Admin fails with Octokit.ApiValidationException : Setting team permission to admin is no longer supported

curl also gets me the same error

{
  "message": "Setting team permission to admin is no longer supported",
  "documentation_url": "https://docs.github.com/rest/reference/teams/#update-a-team-legacy"
}

var update = new UpdateTeam(teamName)
{
Description = teamDescription,
Privacy = TeamPrivacy.Closed,
Permission = teamPermission,
ParentTeamId = parentTeamContext.TeamId
};

Expand All @@ -457,6 +462,7 @@ public async Task UpdatesTeam()
Assert.Equal(teamName, team.Name);
Assert.Equal(teamDescription, team.Description);
Assert.Equal(TeamPrivacy.Closed, team.Privacy);
Assert.Equal(teamPermission, team.Permission);
Assert.Equal(parentTeamContext.TeamId, team.Parent.Id);
}
}
Expand Down
4 changes: 2 additions & 2 deletions Octokit.Tests/SimpleJsonSerializerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -472,12 +472,12 @@ public void ShouldDeserializeParentTeamWithNullPermission()
var result = new SimpleJsonSerializer().Deserialize<Team>(teamJson);

// original value works as expected
Assert.Equal(PermissionLevel.Admin, result.Permission.Value);
Assert.Equal(TeamPermission.Admin, result.Permission.Value);
Assert.Equal("admin", result.Permission.StringValue);

// parent permission is marked as null and cannot be parsed
Assert.Equal("null", result.Parent.Permission.StringValue);
PermissionLevel value;
TeamPermission value;
Assert.False(result.Parent.Permission.TryParse(out value));
}
}
Expand Down
2 changes: 1 addition & 1 deletion Octokit/Models/Request/NewTeam.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public NewTeam(string name)
/// <summary>
/// The permission that new repositories will be added to the team with when none is specified (default: Pull)
/// </summary>
public Permission? Permission { get; set; }
public NewTeamPermission? Permission { get; set; }

/// <summary>
/// Id of a team to set as the parent team
Expand Down
48 changes: 48 additions & 0 deletions Octokit/Models/Request/Permission.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,52 @@ public enum Permission
[Parameter(Value = "pull")]
Pull
}

/// <summary>
/// Deprecated. The permission that new repositories will be added to the team with when none is specified
/// Default: pull
/// Can be one of: pull, push
/// </summary>
[SuppressMessage("Microsoft.Naming", "CA1711:IdentifiersShouldNotHaveIncorrectSuffix")]
public enum NewTeamPermission
{
/// <summary>
/// team members can pull, but not push to or administer these repositories
/// </summary>
[Parameter(Value = "pull")]
Pull,

/// <summary>
/// team members can pull and push, but not administer these repositories
/// </summary>
[Parameter(Value = "push")]
Push
}

/// <summary>
/// Deprecated. The permission that new repositories will be added to the team with when none is specified.
/// Default: pull
/// Can be one of: pull, push, admin
/// </summary>
[SuppressMessage("Microsoft.Naming", "CA1711:IdentifiersShouldNotHaveIncorrectSuffix")]
public enum TeamPermission
{
/// <summary>
/// team members can pull, but not push to or administer these repositories
/// </summary>
[Parameter(Value = "pull")]
Pull,

/// <summary>
/// team members can pull and push, but not administer these repositories
/// </summary>
[Parameter(Value = "push")]
Push,

/// <summary>
/// team members can pull, push and administer these repositories.
/// </summary>
[Parameter(Value = "admin")]
Admin,
}
}
2 changes: 1 addition & 1 deletion Octokit/Models/Request/UpdateTeam.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public UpdateTeam(string name)
/// <summary>
/// The permission that new repositories will be added to the team with when none is specified (default: Pull)
/// </summary>
public Permission? Permission { get; set; }
public TeamPermission? Permission { get; set; }
kfcampbell marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Id of a team to set as the parent team
Expand Down
4 changes: 2 additions & 2 deletions Octokit/Models/Response/Team.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class Team
{
public Team() { }

public Team(string url, string htmlUrl, int id, string nodeId, string slug, string name, string description, TeamPrivacy privacy, PermissionLevel permission, int membersCount, int reposCount, Organization organization, Team parent, string ldapDistinguishedName)
public Team(string url, string htmlUrl, int id, string nodeId, string slug, string name, string description, TeamPrivacy privacy, TeamPermission permission, int membersCount, int reposCount, Organization organization, Team parent, string ldapDistinguishedName)
{
Url = url;
HtmlUrl = htmlUrl;
Expand Down Expand Up @@ -73,7 +73,7 @@ public Team(string url, string htmlUrl, int id, string nodeId, string slug, stri
/// <summary>
/// permission attached to this team
/// </summary>
public StringEnum<PermissionLevel> Permission { get; private set; }
public StringEnum<TeamPermission> Permission { get; private set; }

/// <summary>
/// how many members in this team
Expand Down