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 PR reviews preview #352

Merged
merged 1 commit into from
Sep 8, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
77 changes: 77 additions & 0 deletions src/main/java/org/kohsuke/github/GHPullRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@

import java.io.IOException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Date;
import java.util.List;
import javax.annotation.CheckForNull;

/**
* A pull request.
Expand Down Expand Up @@ -223,6 +227,27 @@ protected void wrapUp(GHPullRequestFileDetail[] page) {
};
}

/**
* Retrieves all the reviews associated to this pull request.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice2have: since

Copy link
Collaborator

Choose a reason for hiding this comment

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

And below

public PagedIterable<GHPullRequestReview> listReviews() {
return new PagedIterable<GHPullRequestReview>() {
public PagedIterator<GHPullRequestReview> _iterator(int pageSize) {
return new PagedIterator<GHPullRequestReview>(root.retrieve()
.withPreview("application/vnd.github.black-cat-preview+json")
Copy link
Contributor

Choose a reason for hiding this comment

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

What about adding an entry in Previews class?

.asIterator(String.format("%s/reviews", getApiRoute()),
GHPullRequestReview[].class, pageSize)) {
@Override
protected void wrapUp(GHPullRequestReview[] page) {
for (GHPullRequestReview r: page) {
r.wrapUp(GHPullRequest.this);
}
}
};
}
};
}

/**
* Obtains all the review comments associated with this pull request.
*/
Expand Down Expand Up @@ -259,6 +284,34 @@ protected void wrapUp(GHPullRequestCommitDetail[] page) {
};
}

@Preview
@Deprecated
public GHPullRequestReview createReview(String body, @CheckForNull GHPullRequestReviewState event,
GHPullRequestReviewComment... comments)
throws IOException {
return createReview(body, event, Arrays.asList(comments));
}

@Preview
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

public GHPullRequestReview createReview(String body, @CheckForNull GHPullRequestReviewState event,
List<GHPullRequestReviewComment> comments)
throws IOException {
if (event == null) {
event = GHPullRequestReviewState.PENDING;
}
List<DraftReviewComment> draftComments = new ArrayList<DraftReviewComment>(comments.size());
for (GHPullRequestReviewComment c : comments) {
draftComments.add(new DraftReviewComment(c.getBody(), c.getPath(), c.getPosition()));
}
return new Requester(root).method("POST")
.with("body", body)
//.with("event", event.name())
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I'm wrong but if you aren't sending the event, then all of the reviews will be set to Pending state, right? If that's the case, why you have the event parameter in the first place?

I think we should let send it as it was specified in the method signature (2 cents opinion again)

._with("comments", draftComments)
.withPreview("application/vnd.github.black-cat-preview+json")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

.to(getApiRoute() + "/reviews", GHPullRequestReview.class).wrapUp(this);
}

public GHPullRequestReviewComment createReviewComment(String body, String sha, String path, int position) throws IOException {
return new Requester(root).method("POST")
.with("body", body)
Expand Down Expand Up @@ -300,4 +353,28 @@ private void fetchIssue() throws IOException {
fetchedIssueDetails = true;
}
}

private static class DraftReviewComment {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's just my 2 cents opinion but why don't you make this public and add the GH prefix on it too to make it standard with the majority of the other classes in the project.

private String body;
private String path;
private int position;

public DraftReviewComment(String body, String path, int position) {
this.body = body;
this.path = path;
this.position = position;
}

public String getBody() {
return body;
}

public String getPath() {
return path;
}

public int getPosition() {
return position;
}
}
}
147 changes: 147 additions & 0 deletions src/main/java/org/kohsuke/github/GHPullRequestReview.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/*
* The MIT License
*
* Copyright (c) 2017, CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.kohsuke.github;

import java.io.IOException;
import java.net.URL;

/**
* Review to the pull request
*
* @see GHPullRequest#listReviews()
* @see GHPullRequest#createReview(String, GHPullRequestReviewState, GHPullRequestReviewComment...)
*/
public class GHPullRequestReview extends GHObject {
GHPullRequest owner;

private String body;
private GHUser user;
private String commit_id;
private GHPullRequestReviewState state;

/*package*/ GHPullRequestReview wrapUp(GHPullRequest owner) {
this.owner = owner;
return this;
}

/**
* Gets the pull request to which this review is associated.
*/
public GHPullRequest getParent() {
return owner;
}

/**
* The comment itself.
*/
public String getBody() {
return body;
}

/**
* Gets the user who posted this review.
*/
public GHUser getUser() throws IOException {
return owner.root.getUser(user.getLogin());
}

public String getCommitId() {
return commit_id;
}

public GHPullRequestReviewState getState() {
return state;
}

@Override
public URL getHtmlUrl() {
return null;
}

protected String getApiRoute() {
return owner.getApiRoute()+"/reviews/"+id;
}

/**
* Updates the comment.
*/
@Preview
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

@Deprecated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it worth adding dependency on accmod and adding @Restricted annotations instead

Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

public void submit(String body, GHPullRequestReviewState event) throws IOException {
new Requester(owner.root).method("POST")
.with("body", body)
.with("event", event.action())
.withPreview("application/vnd.github.black-cat-preview+json")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

.to(getApiRoute()+"/events",this);
this.body = body;
this.state = event;
}

/**
* Deletes this review.
*/
@Preview
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

public void delete() throws IOException {
new Requester(owner.root).method("DELETE")
.withPreview("application/vnd.github.black-cat-preview+json")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

.to(getApiRoute());
}

/**
* Dismisses this review.
*/
@Preview
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

public void dismiss(String message) throws IOException {
new Requester(owner.root).method("PUT")
.with("message", message)
.withPreview("application/vnd.github.black-cat-preview+json")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

.to(getApiRoute()+"/dismissals");
state = GHPullRequestReviewState.DISMISSED;
}

/**
* Obtains all the review comments associated with this pull request review.
*/
@Preview
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

public PagedIterable<GHPullRequestReviewComment> listReviewComments() throws IOException {
return new PagedIterable<GHPullRequestReviewComment>() {
public PagedIterator<GHPullRequestReviewComment> _iterator(int pageSize) {
return new PagedIterator<GHPullRequestReviewComment>(
owner.root.retrieve()
.withPreview("application/vnd.github.black-cat-preview+json")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no long a preview method, so you can remove this line

.asIterator(getApiRoute() + "/comments",
GHPullRequestReviewComment[].class, pageSize)) {
protected void wrapUp(GHPullRequestReviewComment[] page) {
for (GHPullRequestReviewComment c : page)
c.wrapUp(owner);
}
};
}
};
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ public class GHPullRequestReviewComment extends GHObject implements Reactable {
private int position;
private int originalPosition;

public static GHPullRequestReviewComment draft(String body, String path, int position) {
GHPullRequestReviewComment result = new GHPullRequestReviewComment();
result.body = body;
result.path = path;
result.position = position;
return result;
}

/*package*/ GHPullRequestReviewComment wrapUp(GHPullRequest owner) {
this.owner = owner;
return this;
Expand Down
19 changes: 19 additions & 0 deletions src/main/java/org/kohsuke/github/GHPullRequestReviewState.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.kohsuke.github;

public enum GHPullRequestReviewState {
PENDING(null),
APPROVED("APPROVE"),
REQUEST_CHANGES("REQUEST_CHANGES"),
COMMENTED("COMMENT"),
DISMISSED(null);

private final String _action;

GHPullRequestReviewState(String action) {
_action = action;
}

public String action() {
return _action;
}
}
13 changes: 12 additions & 1 deletion src/test/java/org/kohsuke/github/AbstractGitHubApiTestBase.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package org.kohsuke.github;

import java.io.FileInputStream;
import java.util.Properties;
import org.apache.commons.io.IOUtils;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Before;
Expand All @@ -19,9 +22,17 @@ public abstract class AbstractGitHubApiTestBase extends Assert {
public void setUp() throws Exception {
File f = new File(System.getProperty("user.home"), ".github.kohsuke2");
if (f.exists()) {
Properties props = new Properties();
FileInputStream in = null;
try {
in = new FileInputStream(f);
props.load(in);
} finally {
IOUtils.closeQuietly(in);
}
// use the non-standard credential preferentially, so that developers of this library do not have
// to clutter their event stream.
gitHub = GitHubBuilder.fromPropertyFile(f.getPath()).withRateLimitHandler(RateLimitHandler.FAIL).build();
gitHub = GitHubBuilder.fromProperties(props).withRateLimitHandler(RateLimitHandler.FAIL).build();
} else {
gitHub = GitHubBuilder.fromCredentials().withRateLimitHandler(RateLimitHandler.FAIL).build();
}
Expand Down
30 changes: 30 additions & 0 deletions src/test/java/org/kohsuke/github/PullRequestTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import java.util.Collection;
import java.util.List;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;

/**
* @author Kohsuke Kawaguchi
*/
Expand All @@ -26,6 +29,33 @@ public void createPullRequestComment() throws Exception {
p.comment("Some comment");
}

@Test
public void testPullRequestReviews() throws Exception {
String name = rnd.next();
GHPullRequest p = getRepository().createPullRequest(name, "stable", "master", "## test");
GHPullRequestReview draftReview = p.createReview("Some draft review", null,
GHPullRequestReviewComment.draft("Some niggle", "changelog.html", 1)
);
assertThat(draftReview.getState(), is(GHPullRequestReviewState.PENDING));
assertThat(draftReview.getBody(), is("Some draft review"));
assertThat(draftReview.getCommitId(), notNullValue());
List<GHPullRequestReview> reviews = p.listReviews().asList();
assertThat(reviews.size(), is(1));
GHPullRequestReview review = reviews.get(0);
assertThat(review.getState(), is(GHPullRequestReviewState.PENDING));
assertThat(review.getBody(), is("Some draft review"));
assertThat(review.getCommitId(), notNullValue());
review.submit("Some review comment", GHPullRequestReviewState.COMMENTED);
List<GHPullRequestReviewComment> comments = review.listReviewComments().asList();
assertEquals(1, comments.size());
GHPullRequestReviewComment comment = comments.get(0);
assertEquals("Some niggle", comment.getBody());
review = p.createReview("Some new review", null,
GHPullRequestReviewComment.draft("Some niggle", "changelog.html", 1)
);
review.delete();
}

@Test
public void testPullRequestReviewComments() throws Exception {
String name = rnd.next();
Expand Down