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

Add support for all create team parameters #683

Merged
merged 8 commits into from
Jan 27, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
92 changes: 92 additions & 0 deletions src/main/java/org/kohsuke/github/GHCreateTeamBuilder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package org.kohsuke.github;

import java.io.IOException;
import java.util.List;

/**
* Creates a team.
Copy link
Member

Choose a reason for hiding this comment

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

Add a link to the github api documentation for create team

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

*/
public class GHCreateTeamBuilder {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public class GHCreateTeamBuilder {
public class GHTeamBuilder {

Then we could also have a GHTeamUpdater.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done


private final GitHub root;
protected final Requester builder;
private final String apiUrlTail;

public GHCreateTeamBuilder(GitHub root, String apiUrlTail, String name) {
this.root = root;
this.apiUrlTail = apiUrlTail;
this.builder = root.createRequest();
this.builder.with("name", name);
}

/**
* Description for this team.
*
* @param description
* description of team
* @return a builder to continue with building
*/
public GHCreateTeamBuilder description(String description) {
this.builder.with("description", description);
return this;
}

/**
* Maintainers for this team.
*
* @param maintainers
* maintainers of team
* @return a builder to continue with building
*/
public GHCreateTeamBuilder maintainers(List<String> maintainers) {
this.builder.with("maintainers", maintainers);
return this;
}

/**
* Repo names to add this team to.
*
* @param repoNames
* repoNames to add team to
* @return a builder to continue with building
*/
public GHCreateTeamBuilder repoNames(List<String> repoNames) {
this.builder.with("repo_names", repoNames);
return this;
}

/**
* Description for this team
*
* @param privacy
* privacy of team
* @return a builder to continue with building
*/
public GHCreateTeamBuilder privacy(GHTeam.Privacy privacy) {
this.builder.with("privacy", privacy);
return this;
}

/**
* Parent team id for this team
*
* @param parentTeamId
* parentTeamId of team
* @return a builder to continue with building
*/
public GHCreateTeamBuilder parentTeamId(int parentTeamId) {
this.builder.with("parent_team_id", parentTeamId);
return this;
}

/**
* Creates a team with all the parameters.
*
* @return the gh team
* @throws IOException
* if team cannot be created
*/
public GHTeam create() throws IOException {
return builder.method("POST").withUrlPath(apiUrlTail).fetch(GHTeam.class).wrapUp(root);
}
}
17 changes: 16 additions & 1 deletion src/main/java/org/kohsuke/github/GHOrganization.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public GHRepository createRepository(String name,
*
* <p>
* You use the returned builder to set various properties, then call {@link GHCreateRepositoryBuilder#create()} to
* finally createa repository.
* finally create a repository.
*
* @param name
* the name
Expand Down Expand Up @@ -455,6 +455,21 @@ public GHTeam createTeam(String name, GHRepository... repositories) throws IOExc
return createTeam(name, Arrays.asList(repositories));
}

/**
* Starts a builder that creates a new team.
*
* <p>
* You use the returned builder to set various properties, then call {@link GHCreateTeamBuilder#create()} to finally
* create a team.
*
* @param name
* the name
* @return the gh create repository builder
*/
public GHCreateTeamBuilder createTeam(String name) {
Copy link
Member

Choose a reason for hiding this comment

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

I think the other createTeam methods should be marked as @Deprecated. Do you agree?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, wanted your input on it first before I did it, done.

return new GHCreateTeamBuilder(root, "/orgs/" + login + "/teams", name);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return new GHCreateTeamBuilder(root, "/orgs/" + login + "/teams", name);
return new GHCreateTeamBuilder(root, login, name);

Or maybe even this (and add name to builder):

Suggested change
return new GHCreateTeamBuilder(root, "/orgs/" + login + "/teams", name);
return new GHCreateTeamBuilder(root);

Is a team always created attached to a login? As me, could I create a team that your login owns?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did this for consistency with the create repo builder, I've made it take an org name now and not the 'apiTailUrl'.

I've kept the name as it's the only mandatory parameter for the builder

}

/**
* List up repositories that has some open pull requests.
* <p>
Expand Down
33 changes: 32 additions & 1 deletion src/main/java/org/kohsuke/github/GHTeam.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,22 @@
* @author Kohsuke Kawaguchi
*/
public class GHTeam implements Refreshable {
private String name, permission, slug, description;
private String name;
private String permission;
private String slug;
private String description;
private Privacy privacy;

private int id;
private GHOrganization organization; // populated by GET /user/teams where Teams+Orgs are returned together

protected /* final */ GitHub root;

public enum Privacy {
SECRET, // only visible to organization owners and members of this team.
CLOSED // visible to all members of this organization.
}

/**
* Member's role in a team
*/
Expand Down Expand Up @@ -94,6 +104,15 @@ public String getDescription() {
return description;
}

/**
* Gets the privacy state.
*
* @return the privacy state.
*/
public Privacy getPrivacy() {
return privacy;
}

/**
* Sets description.
*
Expand All @@ -106,6 +125,18 @@ public void setDescription(String description) throws IOException {
root.createRequest().method("PATCH").with("description", description).withUrlPath(api("")).send();
}

/**
* Sets privacy.
*
* @param privacy
* the privacy
* @throws IOException
* the io exception
*/
public void setPrivacy(Privacy privacy) throws IOException {
root.createRequest().method("PATCH").with("privacy", privacy).withUrlPath(api("")).send();
Copy link
Member

Choose a reason for hiding this comment

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

@timja
This matches the existing behavior of other set methods on this class. There have been issues filed describing this is not great.

This is not a blocker for this PR, but I'd like your thoughts on #589.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agree, changed to update.
I didn't like the name but I followed the other method for consistency in the class

Copy link
Member

Choose a reason for hiding this comment

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

Glad you agree. But leave this for now. I don't want to do this unevenly (some update some set). 😄

}

/**
* Gets id.
*
Expand Down
30 changes: 28 additions & 2 deletions src/test/java/org/kohsuke/github/GHOrganizationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

import java.io.IOException;

import static java.util.Collections.singletonList;

public class GHOrganizationTest extends AbstractGitHubWireMockTest {

public static final String GITHUB_API_TEST = "github-api-test";
Expand Down Expand Up @@ -86,7 +88,7 @@ public void testCreateTeamWithRepoAccess() throws IOException {
// Create team with access to repository. Check access was granted.
GHTeam team = org.createTeam(TEAM_NAME_CREATE, GHOrganization.Permission.PUSH, repo);
Assert.assertTrue(team.getRepositories().containsKey(REPO_NAME));
Assert.assertEquals(Permission.PUSH.toString().toLowerCase(), team.getPermission());
assertEquals(Permission.PUSH.toString().toLowerCase(), team.getPermission());
}

@Test
Expand All @@ -100,6 +102,30 @@ public void testCreateTeam() throws IOException {
// Create team with no permission field. Verify that default permission is pull
GHTeam team = org.createTeam(TEAM_NAME_CREATE, repo);
Assert.assertTrue(team.getRepositories().containsKey(REPO_NAME));
Assert.assertEquals(DEFAULT_PERMISSION, team.getPermission());
assertEquals(DEFAULT_PERMISSION, team.getPermission());
}

@Test
public void testCreateVisibleTeam() throws IOException {
GHOrganization org = gitHub.getOrganization(GITHUB_API_TEST_ORG);

GHTeam team = org.createTeam(TEAM_NAME_CREATE).privacy(GHTeam.Privacy.CLOSED).create();
assertEquals(GHTeam.Privacy.CLOSED, team.getPrivacy());
}

@Test
public void testCreateAllArgsTeam() throws IOException {
String REPO_NAME = "github-api";
GHOrganization org = gitHub.getOrganization(GITHUB_API_TEST_ORG);

GHTeam team = org.createTeam(TEAM_NAME_CREATE)
.description("Team description")
.maintainers(singletonList("bitwiseman"))
.repoNames(singletonList(REPO_NAME))
.privacy(GHTeam.Privacy.CLOSED)
.parentTeamId(3617900)
.create();
assertEquals("Team description", team.getDescription());
assertEquals(GHTeam.Privacy.CLOSED, team.getPrivacy());
}
}
24 changes: 24 additions & 0 deletions src/test/java/org/kohsuke/github/GHTeamTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.kohsuke.github;

import org.junit.Test;
import org.kohsuke.github.GHTeam.Privacy;

import java.io.IOException;

Expand Down Expand Up @@ -30,4 +31,27 @@ public void testSetDescription() throws IOException {
assertEquals(description, team.getDescription());
}

@Test
public void testSetPrivacy() throws IOException {
String teamSlug = "dummy-team";
Privacy privacy = Privacy.CLOSED;

// Set the privacy.
GHTeam team = gitHub.getOrganization(GITHUB_API_TEST_ORG).getTeamBySlug(teamSlug);
team.setPrivacy(privacy);

// Check that it was set correctly.
team = gitHub.getOrganization(GITHUB_API_TEST_ORG).getTeamBySlug(teamSlug);
assertEquals(privacy, team.getPrivacy());

privacy = Privacy.SECRET;

// Set the privacy.
team.setPrivacy(privacy);

// Check that it was set correctly.
team = gitHub.getOrganization(GITHUB_API_TEST_ORG).getTeamBySlug(teamSlug);
assertEquals(privacy, team.getPrivacy());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"login": "github-api-test-org",
"id": 7544739,
"node_id": "MDEyOk9yZ2FuaXphdGlvbjc1NDQ3Mzk=",
"url": "https://api.github.com/orgs/github-api-test-org",
"repos_url": "https://api.github.com/orgs/github-api-test-org/repos",
"events_url": "https://api.github.com/orgs/github-api-test-org/events",
"hooks_url": "https://api.github.com/orgs/github-api-test-org/hooks",
"issues_url": "https://api.github.com/orgs/github-api-test-org/issues",
"members_url": "https://api.github.com/orgs/github-api-test-org/members{/member}",
"public_members_url": "https://api.github.com/orgs/github-api-test-org/public_members{/member}",
"avatar_url": "https://avatars3.githubusercontent.com/u/7544739?v=4",
"description": null,
"is_verified": false,
"has_organization_projects": true,
"has_repository_projects": true,
"public_repos": 10,
"public_gists": 0,
"followers": 0,
"following": 0,
"html_url": "https://github.com/github-api-test-org",
"created_at": "2014-05-10T19:39:11Z",
"updated_at": "2015-04-20T00:42:30Z",
"type": "Organization",
"total_private_repos": 0,
"owned_private_repos": 0,
"private_gists": 0,
"disk_usage": 132,
"collaborators": 0,
"billing_email": "kk@kohsuke.org",
"default_repository_permission": "none",
"members_can_create_repositories": false,
"two_factor_requirement_enabled": false,
"plan": {
"name": "free",
"space": 976562499,
"private_repos": 0,
"filled_seats": 7,
"seats": 0
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
{
"name": "create-team-test",
"id": 3618001,
"node_id": "MDQ6VGVhbTM2MTgwMDE=",
"slug": "create-team-test",
"description": "Team description",
"privacy": "closed",
"url": "https://api.github.com/organizations/49127317/team/3618001",
"html_url": "https://github.com/orgs/github-api-test-org/teams/create-team-test",
"members_url": "https://api.github.com/organizations/49127317/team/3618001/members{/member}",
"repositories_url": "https://api.github.com/organizations/49127317/team/3618001/repos",
"permission": "pull",
"parent": {
"name": "Core Developers",
"id": 3617900,
"node_id": "MDQ6VGVhbTM2MTc5MDA=",
"slug": "core-developers",
"description": "",
"privacy": "closed",
"url": "https://api.github.com/organizations/49127317/team/3617900",
"html_url": "https://github.com/orgs/github-api-test-org/teams/core-developers",
"members_url": "https://api.github.com/organizations/49127317/team/3617900/members{/member}",
"repositories_url": "https://api.github.com/organizations/49127317/team/3617900/repos",
"permission": "pull"
},
"created_at": "2020-01-25T19:41:44Z",
"updated_at": "2020-01-25T19:41:44Z",
"members_count": 1,
"repos_count": 1,
"organization": {
"login": "github-api-test-org",
"id": 49127317,
"node_id": "MDEyOk9yZ2FuaXphdGlvbjQ5MTI3MzE3",
"url": "https://api.github.com/orgs/github-api-test-org",
"repos_url": "https://api.github.com/orgs/github-api-test-org/repos",
"events_url": "https://api.github.com/orgs/github-api-test-org/events",
"hooks_url": "https://api.github.com/orgs/github-api-test-org/hooks",
"issues_url": "https://api.github.com/orgs/github-api-test-org/issues",
"members_url": "https://api.github.com/orgs/github-api-test-org/members{/member}",
"public_members_url": "https://api.github.com/orgs/github-api-test-org/public_members{/member}",
"avatar_url": "https://avatars3.githubusercontent.com/u/49127317?v=4",
"description": null,
"is_verified": false,
"has_organization_projects": true,
"has_repository_projects": true,
"public_repos": 4,
"public_gists": 0,
"followers": 0,
"following": 0,
"html_url": "https://github.com/github-api-test-org",
"created_at": "2019-03-31T17:42:10Z",
"updated_at": "2019-10-07T20:06:18Z",
"type": "Organization"
}
}
Loading