Feature/#1025 #1239

bloslo · 2021-09-24T16:10:08Z

Description

Add GHIssueQueryBuilder. Closes #1025.

I didn't use the GHIssueQueryBuilder as a base class for the GHPullRequestQueryBuilder as suggested in the issue because of the different query parameters the two endpoints list.

I was wondering should the listIssues method in GHRepository be marked for deprecation? Similar to the listPullRequests method in the same class.

codecov · 2021-09-24T16:49:32Z

Codecov Report

Merging #1239 (b309785) into main (2dc736a) will increase coverage by 0.14%.
The diff coverage is 92.30%.

@@             Coverage Diff              @@
##               main    #1239      +/-   ##
============================================
+ Coverage     77.28%   77.42%   +0.14%     
- Complexity     1889     1899      +10     
============================================
  Files           188      189       +1     
  Lines          5943     5972      +29     
  Branches        328      327       -1     
============================================
+ Hits           4593     4624      +31     
+ Misses         1152     1150       -2     
  Partials        198      198

Impacted Files	Coverage Δ
src/main/java/org/kohsuke/github/GHRepository.java	`68.08% <50.00%> (+0.11%)`	⬆️
...n/java/org/kohsuke/github/GHIssueQueryBuilder.java	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2dc736a...b309785. Read the comment docs.

bitwiseman · 2021-09-24T19:11:12Z

@bloslo
Please write your tests so that they do not reduce existing code coverage (similar to what I did with your previous PR). Also make sure you have tests covering as many of the added paths as possible. If you can't cover all of them, please add comments (on this PR, not in the code) for why those paths can't be covered.

I've invited you to the hub4j-test-org to ensure you can re-record test data as needed.

bitwiseman · 2021-09-24T19:31:37Z

I was wondering should the listIssues method in GHRepository be marked for deprecation? Similar to the listPullRequests method in the same class.

Yeah, probably.

I'm also wondering if GHQueryBuilder should extend PagedIterable. That would remove the need to call list() or end up with query.list().asList(). But that is another issue/PR.

bitwiseman

Sorry for the second round of feedback. I took a closer look this time around and saw a couple things I missed. The label change is pretty much as written in the suggestions. The class structure change, the suggested code is for clarity and there are a bunch more changes to be made.

Thanks for you patience and perseverance.

bitwiseman · 2021-09-25T18:51:22Z

src/main/java/org/kohsuke/github/GHIssueQueryBuilder.java

+
+public class GHIssueQueryBuilder extends GHQueryBuilder<GHIssue> {
+    private final GHRepository repo;
+    private List<String> labels = new ArrayList<>();


Suggested change

private List<String> labels = new ArrayList<>();

private final List<String> labels = new ArrayList<>();

bitwiseman · 2021-09-25T18:53:01Z

src/main/java/org/kohsuke/github/GHIssueQueryBuilder.java

+        return req.with("labels", labels.stream().collect(Collectors.joining(",")))
+                .withUrlPath(repo.getApiTailUrl("issues"))


Suggested change

return req.with("labels", labels.stream().collect(Collectors.joining(",")))

.withUrlPath(repo.getApiTailUrl("issues"))

return req.withUrlPath(repo.getApiTailUrl("issues"))

bitwiseman · 2021-09-25T18:53:47Z

.../kohsuke/github/AppTest/wiremock/testGetIssues/mappings/repos_hub4j_github-api_issues-4.json

@@ -2,7 +2,7 @@
  "id": "a70d05ac-5820-4749-8318-e422f0f6eaf5",
  "name": "repos_hub4j_github-api_issues",
  "request": {
-    "url": "/repos/hub4j/github-api/issues?state=closed",
+    "url": "/repos/hub4j/github-api/issues?state=closed&labels=",


&labels= is a no-op parameter and should not be present.

Suggested change

"url": "/repos/hub4j/github-api/issues?state=closed&labels=",

"url": "/repos/hub4j/github-api/issues?state=closed",

bitwiseman · 2021-09-25T18:53:59Z

...kohsuke/github/AppTest/wiremock/testListIssues/mappings/repos_hub4j_github-api_issues-4.json

@@ -2,7 +2,7 @@
  "id": "b87318fd-8120-4fad-8c4d-553dafde3319",
  "name": "repos_hub4j_github-api_issues",
  "request": {
-    "url": "/repos/hub4j/github-api/issues?state=closed",
+    "url": "/repos/hub4j/github-api/issues?state=closed&labels=",


Suggested change

"url": "/repos/hub4j/github-api/issues?state=closed&labels=",

"url": "/repos/hub4j/github-api/issues?state=closed",

bitwiseman · 2021-09-25T18:54:24Z

...ppings/repos_hub4j-test-org_testqueryissues_issues-92abb44d-1298-496e-a8e9-ae447ffc68a1.json

+  "id": "92abb44d-1298-496e-a8e9-ae447ffc68a1",
+  "name": "repos_hub4j-test-org_testqueryissues_issues",
+  "request": {
+    "url": "/repos/hub4j-test-org/testQueryIssues/issues?state=all&mentioned=bloslo&since=1970-01-19T21%3A26%3A51Z&sort=comments&direction=asc&labels=",


Suggested change

"url": "/repos/hub4j-test-org/testQueryIssues/issues?state=all&mentioned=bloslo&since=1970-01-19T21%3A26%3A51Z&sort=comments&direction=asc&labels=",

"url": "/repos/hub4j-test-org/testQueryIssues/issues?state=all&mentioned=bloslo&since=1970-01-19T21%3A26%3A51Z&sort=comments&direction=asc",

bitwiseman · 2021-09-25T20:37:39Z

src/test/java/org/kohsuke/github/AppTest.java

+        List<GHIssue> openBugIssues = gitHub.getOrganization("hub4j-test-org")
+                .getRepository("testQueryIssues")


These don't need to request the repo again each time. Once you make this change, re-record to test data.

Suggested change

List<GHIssue> openBugIssues = gitHub.getOrganization("hub4j-test-org")

.getRepository("testQueryIssues")

final GHRepository repo = gitHub.getOrganization("hub4j-test-org")

.getRepository("testQueryIssues");

List<GHIssue> openBugIssues = repo

src/test/java/org/kohsuke/github/AppTest.java

bitwiseman · 2021-09-25T20:42:06Z

src/test/java/org/kohsuke/github/AppTest.java

+        List<GHIssue> openIssuesWithAssignee = gitHub.getOrganization("hub4j-test-org")
+                .getRepository("testQueryIssues")


Suggested change

List<GHIssue> openIssuesWithAssignee = gitHub.getOrganization("hub4j-test-org")

.getRepository("testQueryIssues")

List<GHIssue> openIssuesWithAssignee = repo

bitwiseman · 2021-09-25T20:42:29Z

src/test/java/org/kohsuke/github/AppTest.java

+        List<GHIssue> allIssuesSince = gitHub.getOrganization("hub4j-test-org")
+                .getRepository("testQueryIssues")


Suggested change

List<GHIssue> allIssuesSince = gitHub.getOrganization("hub4j-test-org")

.getRepository("testQueryIssues")

List<GHIssue> allIssuesSince = repo

…o feature/hub4j#1025

…ory` as suggested.

bitwiseman

Excellent!

bloslo added 5 commits September 24, 2021 17:59

Add GHIssueQueryBuilder.

507c947

Update test data files.

9b0d831

Add test for querying issues.

e4be647

Merge branch 'main' into feature/hub4j#1025

b179461

Add test for the GHIssueQueryBuilder.Sort enum.

6b8d46c

bloslo added 3 commits September 24, 2021 23:42

Deprecate GHRepository listIssues method

dd38e90

Replace call to listIssues with queryIssues.

0a03bea

Update tests to fix code coverage.

ec9e12b

bitwiseman requested changes Sep 25, 2021

View reviewed changes

bloslo and others added 12 commits September 25, 2021 22:47

Make labels final.

485b74e

Modify how the label parameter is added to the request.

5745bfd

Update JavaDoc for the milestone parameter.

098a577

Merge branch 'main' into feature/hub4j#1025

8d19611

Merge branch 'main' into feature/hub4j#1025

0ebfb1d

Merge branch 'feature/hub4j#1025' of github.com:bloslo/github-api int…

0e43206

…o feature/hub4j#1025

Refactor GHIssueQueryBuilder and add `GHIssueQueryBuilderForReposit…

bfe456d

…ory` as suggested.

Refactor GHIssueQueryBuilderForRepository into a static nested class.

29a0d0d

Request test repo once.

9f55f7c

Add more assertions.

b8a8522

Change how the labels list is transformed into a comma separated string.

e1b0249

Extend AppTest testQueryIssues method as per suggestions.

b309785

bloslo requested a review from bitwiseman September 28, 2021 15:55

bitwiseman approved these changes Sep 30, 2021

View reviewed changes

bitwiseman merged commit 996170a into hub4j:main Sep 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/#1025 #1239

Feature/#1025 #1239

bloslo commented Sep 24, 2021

codecov bot commented Sep 24, 2021 •

edited

Loading

bitwiseman commented Sep 24, 2021 •

edited

Loading

bitwiseman commented Sep 24, 2021

bitwiseman left a comment

bitwiseman Sep 25, 2021

bitwiseman Sep 25, 2021

bitwiseman Sep 25, 2021

bitwiseman Sep 25, 2021

bitwiseman Sep 25, 2021

bitwiseman Sep 25, 2021 •

edited

Loading

bitwiseman Sep 25, 2021

bitwiseman Sep 25, 2021

bitwiseman left a comment

	private List<String> labels = new ArrayList<>();
	private final List<String> labels = new ArrayList<>();

		return req.with("labels", labels.stream().collect(Collectors.joining(",")))
		.withUrlPath(repo.getApiTailUrl("issues"))

	return req.with("labels", labels.stream().collect(Collectors.joining(",")))
	.withUrlPath(repo.getApiTailUrl("issues"))
	return req.withUrlPath(repo.getApiTailUrl("issues"))

	"url": "/repos/hub4j/github-api/issues?state=closed&labels=",
	"url": "/repos/hub4j/github-api/issues?state=closed",

	"url": "/repos/hub4j-test-org/testQueryIssues/issues?state=all&mentioned=bloslo&since=1970-01-19T21%3A26%3A51Z&sort=comments&direction=asc&labels=",
	"url": "/repos/hub4j-test-org/testQueryIssues/issues?state=all&mentioned=bloslo&since=1970-01-19T21%3A26%3A51Z&sort=comments&direction=asc",

		List<GHIssue> openBugIssues = gitHub.getOrganization("hub4j-test-org")
		.getRepository("testQueryIssues")

-        List<GHIssue> openBugIssues = gitHub.getOrganization("hub4j-test-org")
-                .getRepository("testQueryIssues")
+        final GHRepository repo = gitHub.getOrganization("hub4j-test-org")
+                .getRepository("testQueryIssues");
+        List<GHIssue> openBugIssues = repo

		List<GHIssue> openIssuesWithAssignee = gitHub.getOrganization("hub4j-test-org")
		.getRepository("testQueryIssues")

	List<GHIssue> openIssuesWithAssignee = gitHub.getOrganization("hub4j-test-org")
	.getRepository("testQueryIssues")
	List<GHIssue> openIssuesWithAssignee = repo

		List<GHIssue> allIssuesSince = gitHub.getOrganization("hub4j-test-org")
		.getRepository("testQueryIssues")

	List<GHIssue> allIssuesSince = gitHub.getOrganization("hub4j-test-org")
	.getRepository("testQueryIssues")
	List<GHIssue> allIssuesSince = repo

Feature/#1025 #1239

Feature/#1025 #1239

Conversation

bloslo commented Sep 24, 2021

Description

codecov bot commented Sep 24, 2021 • edited Loading

Codecov Report

bitwiseman commented Sep 24, 2021 • edited Loading

bitwiseman commented Sep 24, 2021

bitwiseman left a comment

Choose a reason for hiding this comment

bitwiseman Sep 25, 2021

Choose a reason for hiding this comment

bitwiseman Sep 25, 2021

Choose a reason for hiding this comment

bitwiseman Sep 25, 2021

Choose a reason for hiding this comment

bitwiseman Sep 25, 2021

Choose a reason for hiding this comment

bitwiseman Sep 25, 2021

Choose a reason for hiding this comment

bitwiseman Sep 25, 2021 • edited Loading

Choose a reason for hiding this comment

bitwiseman Sep 25, 2021

Choose a reason for hiding this comment

bitwiseman Sep 25, 2021

Choose a reason for hiding this comment

bitwiseman left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 24, 2021 •

edited

Loading

bitwiseman commented Sep 24, 2021 •

edited

Loading

bitwiseman Sep 25, 2021 •

edited

Loading