Update dashboard (#1743)
Check in bunch of fixes/updates:
- Sort repo by names when displaying them on the sidebar.
- Use JDK20.
- Remove duplicated `team_owner` field in `github_team_table`. Make it
easier to change owners of repo.
- Use case-insensitive text for labels to match the search result on
Github.
- Fix a bug that sometimes action owners for an issue are not correctly
computed.
- Update the logic for determining whether an issue/PR is triaged
according to today's triaging process.
- Update nagging emails to have a separated PRs section.
- Improve sync process.
Fixes #1732.
diff --git a/dashboard/server/Dockerfile b/dashboard/server/Dockerfile
index e0179b3..853cf0a 100644
--- a/dashboard/server/Dockerfile
+++ b/dashboard/server/Dockerfile
@@ -1,4 +1,4 @@
-FROM azul/zulu-openjdk-alpine:19
+FROM azul/zulu-openjdk-alpine:20
ARG DEPENDENCY=dependency
COPY ${DEPENDENCY}/BOOT-INF/lib /app/lib
COPY ${DEPENDENCY}/META-INF /app/META-INF
diff --git a/dashboard/server/database.sql b/dashboard/server/database.sql
index c15cc4b..9c548b5 100644
--- a/dashboard/server/database.sql
+++ b/dashboard/server/database.sql
@@ -1,3 +1,5 @@
+CREATE EXTENSION IF NOT EXISTS citext;
+
CREATE TABLE json_state
(
key TEXT PRIMARY KEY,
@@ -55,7 +57,7 @@
title TEXT NOT NULL,
body TEXT NOT NULL,
milestone TEXT NOT NULL,
- labels TEXT[] NOT NULL,
+ labels CITEXT[] NOT NULL,
created_at TIMESTAMPTZ NOT NULL,
updated_at TIMESTAMPTZ,
closed_at TIMESTAMPTZ,
@@ -204,7 +206,6 @@
created_at TIMESTAMPTZ NOT NULL,
updated_at TIMESTAMPTZ NOT NULL,
name TEXT NOT NULL,
- none_team_owner TEXT NOT NULL DEFAULT '',
PRIMARY KEY (owner, repo, id)
);
@@ -278,6 +279,20 @@
);
--
+-- Table that stores the raw pull requests data fetched from Github
+--
+CREATE TABLE github_pull_request_data
+(
+ owner TEXT NOT NULL,
+ repo TEXT NOT NULL,
+ issue_number INTEGER NOT NULL,
+ timestamp TIMESTAMPTZ NOT NULL,
+ etag TEXT NOT NULL,
+ data JSONB NOT NULL,
+ PRIMARY KEY (owner, repo, issue_number)
+);
+
+--
-- Table that stores the raw builds data fetched from Buildkite
--
CREATE TABLE buildkite_build_data
diff --git a/dashboard/server/pom.xml b/dashboard/server/pom.xml
index 442a165..2b7b643 100644
--- a/dashboard/server/pom.xml
+++ b/dashboard/server/pom.xml
@@ -15,7 +15,7 @@
<description>The server for the dashboard</description>
<properties>
- <java.version>19</java.version>
+ <java.version>20</java.version>
<project.version>DEV</project.version>
</properties>
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/api/FetchPullRequestRequest.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/api/FetchPullRequestRequest.java
new file mode 100644
index 0000000..e287376
--- /dev/null
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/api/FetchPullRequestRequest.java
@@ -0,0 +1,3 @@
+package build.bazel.dashboard.github.api;
+
+public record FetchPullRequestRequest(String owner, String repo, int issueNumber, String etag) {}
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/api/GithubApi.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/api/GithubApi.java
index 07b47a9..dda107e 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/api/GithubApi.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/api/GithubApi.java
@@ -11,6 +11,8 @@
RestApiResponse fetchIssue(FetchIssueRequest request);
+ RestApiResponse fetchPullRequest(FetchPullRequestRequest request);
+
RestApiResponse listIssueComments(ListIssueCommentsRequest request);
RestApiResponse searchIssues(SearchIssuesRequest request);
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/api/WebClientGithubApi.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/api/WebClientGithubApi.java
index 30c693b..a679436 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/api/WebClientGithubApi.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/api/WebClientGithubApi.java
@@ -105,6 +105,24 @@
}
@Override
+ public RestApiResponse fetchPullRequest(FetchPullRequestRequest request) {
+ log.debug("{}", request);
+
+ WebClient.RequestHeadersSpec<?> spec =
+ get(
+ uriBuilder ->
+ uriBuilder
+ .pathSegment(
+ "repos",
+ request.owner(),
+ request.repo(),
+ "pulls",
+ String.valueOf(request.issueNumber()))
+ .build(), request.etag());
+ return exchange(spec);
+ }
+
+ @Override
public RestApiResponse listIssueComments(ListIssueCommentsRequest request) {
log.debug("{}", request);
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubIssue.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubIssue.java
index 390f342..21d174e 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubIssue.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubIssue.java
@@ -5,6 +5,8 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
import com.fasterxml.jackson.databind.annotation.JsonNaming;
+import com.fasterxml.jackson.databind.node.NullNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
import java.time.Instant;
import java.util.List;
import javax.annotation.Nullable;
@@ -21,13 +23,15 @@
String etag;
JsonNode data;
- public static GithubIssue empty(String owner, String repo, int issueNumber) {
+ public static GithubIssue empty(
+ String owner, String repo, int issueNumber, ObjectMapper objectMapper) {
return GithubIssue.builder()
.owner(owner)
.repo(repo)
.issueNumber(issueNumber)
.timestamp(Instant.EPOCH)
.etag("")
+ .data(objectMapper.createObjectNode())
.build();
}
@@ -48,9 +52,14 @@
String title;
String state;
List<Label> labels;
- @Nullable User assignee;
+ List<User> assignees;
Instant createdAt;
Instant updatedAt;
+ @Nullable JsonNode pullRequest;
+
+ public boolean isPullRequest() {
+ return pullRequest != null;
+ }
}
@Builder
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubIssueService.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubIssueService.java
index 06e9f35..77860b7 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubIssueService.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubIssueService.java
@@ -1,12 +1,15 @@
package build.bazel.dashboard.github.issue;
import build.bazel.dashboard.github.api.FetchIssueRequest;
+import build.bazel.dashboard.github.api.FetchPullRequestRequest;
import build.bazel.dashboard.github.api.GithubApi;
import build.bazel.dashboard.github.issuestatus.GithubIssueStatus;
import build.bazel.dashboard.github.issuestatus.GithubIssueStatusService;
+import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.IOException;
import java.time.Instant;
import java.util.Optional;
+import javax.annotation.Nullable;
import lombok.Builder;
import lombok.RequiredArgsConstructor;
import lombok.Value;
@@ -20,12 +23,15 @@
private final GithubApi githubApi;
private final GithubIssueRepo githubIssueRepo;
+ private final GithubPullRequestRepo githubPullRequestRepo;
private final GithubIssueStatusService githubIssueStatusService;
+ private final ObjectMapper objectMapper;
@Builder
@Value
public static class FetchResult {
GithubIssue issue;
+ @Nullable GithubPullRequest pullRequest;
GithubIssueStatus status;
boolean added;
boolean updated;
@@ -33,8 +39,17 @@
Throwable error;
static FetchResult create(
- GithubIssue result, GithubIssueStatus status, boolean added, boolean updated, boolean deleted, Throwable error) {
+ GithubIssue result,
+ GithubPullRequest pullRequest,
+ GithubIssueStatus status,
+ boolean added,
+ boolean updated,
+ boolean deleted,
+ Throwable error) {
FetchResultBuilder builder = FetchResult.builder().issue(result).status(status);
+ if (pullRequest != null) {
+ builder.pullRequest(pullRequest);
+ }
if (error != null) {
builder.error(error);
} else if (added) {
@@ -56,7 +71,7 @@
var existed =
githubIssueRepo
.findOne(owner, repo, issueNumber)
- .orElse(GithubIssue.empty(owner, repo, issueNumber));
+ .orElse(GithubIssue.empty(owner, repo, issueNumber, objectMapper));
FetchIssueRequest request =
FetchIssueRequest.builder()
@@ -79,27 +94,32 @@
.build();
try {
githubIssueRepo.save(githubIssue);
- var status = githubIssueStatusService.check(githubIssue, Instant.now());
- return FetchResult.create(githubIssue, status.orElse(null), !exists, exists, false, null);
+ var pullRequest = fetchAndSavePullRequest(githubIssue);
+ var status = githubIssueStatusService.check(githubIssue, pullRequest, Instant.now());
+ return FetchResult.create(
+ githubIssue, pullRequest, status.orElse(null), !exists, exists, false, null);
} catch (IOException e) {
- return FetchResult.create(githubIssue, null, !exists, exists, false, e);
+ return FetchResult.create(githubIssue, null, null, !exists, exists, false, e);
}
} else if (response.getStatus().value() == 304) {
// Not modified
try {
- var status = githubIssueStatusService.check(existed, Instant.now());
- return FetchResult.create(existed, status.orElse(null), false, false, false, null);
+ var pullRequest = fetchAndSavePullRequest(existed);
+ var status = githubIssueStatusService.check(existed, pullRequest, Instant.now());
+ return FetchResult.create(
+ existed, pullRequest, status.orElse(null), false, false, false, null);
} catch (IOException e) {
- return FetchResult.create(existed, null, false, false, false, e);
+ return FetchResult.create(existed, null, null, false, false, false, e);
}
} else if (response.getStatus().value() == 301
|| response.getStatus().value() == 404
|| response.getStatus().value() == 410) {
// Transferred or deleted
githubIssueRepo.delete(owner, repo, issueNumber);
+ githubPullRequestRepo.delete(owner, repo, issueNumber);
// Mark existing status to DELETED
githubIssueStatusService.markDeleted(owner, repo, issueNumber);
- return FetchResult.create(existed, null, false, false, true, null);
+ return FetchResult.create(existed, null, null, false, false, true, null);
} else {
log.error(
"Failed to fetch {}/{}/issues/{}: {}",
@@ -108,11 +128,57 @@
issueNumber,
response.getStatus().toString());
return FetchResult.create(
- existed, null, false, false, false, new IOException(response.getStatus().toString()));
+ existed,
+ null,
+ null,
+ false,
+ false,
+ false,
+ new IOException(response.getStatus().toString()));
}
}
public Integer findMaxIssueNumber(String owner, String repo) {
return githubIssueRepo.findMaxIssueNumber(owner, repo);
}
+
+ @Nullable
+ private GithubPullRequest fetchAndSavePullRequest(GithubIssue issue) throws IOException {
+ if (!issue.parseData(objectMapper).isPullRequest()) {
+ return null;
+ }
+
+ var existed =
+ githubPullRequestRepo
+ .findOne(issue.getOwner(), issue.getRepo(), issue.getIssueNumber())
+ .orElse(
+ GithubPullRequest.empty(
+ issue.getOwner(), issue.getRepo(), issue.getIssueNumber(), objectMapper));
+
+ var request =
+ new FetchPullRequestRequest(
+ existed.owner(), existed.repo(), existed.issueNumber(), existed.etag());
+
+ var response = githubApi.fetchPullRequest(request);
+ if (response.getStatus().is2xxSuccessful()) {
+ var pullRequest =
+ new GithubPullRequest(
+ existed.owner(),
+ existed.repo(),
+ existed.issueNumber(),
+ Instant.now(),
+ response.getEtag(),
+ response.getBody());
+ githubPullRequestRepo.save(pullRequest);
+ return pullRequest;
+ } else if (response.getStatus().value() == 304) {
+ // Not modified
+ return existed;
+ } else {
+ throw new IOException(
+ String.format(
+ "Failed to fetch %s/%s/pulls/%s: %s",
+ existed.owner(), existed.repo(), existed.issueNumber(), response.getStatus()));
+ }
+ }
}
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubPullRequest.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubPullRequest.java
new file mode 100644
index 0000000..65181f5
--- /dev/null
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubPullRequest.java
@@ -0,0 +1,27 @@
+package build.bazel.dashboard.github.issue;
+
+import build.bazel.dashboard.github.issue.GithubIssue.User;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.PropertyNamingStrategies;
+import com.fasterxml.jackson.databind.annotation.JsonNaming;
+import java.time.Instant;
+import java.util.List;
+
+public record GithubPullRequest(
+ String owner, String repo, int issueNumber, Instant timestamp, String etag, JsonNode data) {
+
+ public static GithubPullRequest empty(
+ String owner, String repo, int issueNumber, ObjectMapper objectMapper) {
+ return new GithubPullRequest(
+ owner, repo, issueNumber, Instant.EPOCH, "", objectMapper.createObjectNode());
+ }
+
+ public Data parseData(ObjectMapper objectMapper) throws JsonProcessingException {
+ return objectMapper.treeToValue(data, Data.class);
+ }
+
+ @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
+ public record Data(List<User> requestedReviewers) {}
+}
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubPullRequestRepo.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubPullRequestRepo.java
new file mode 100644
index 0000000..89f90d8
--- /dev/null
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/issue/GithubPullRequestRepo.java
@@ -0,0 +1,78 @@
+package build.bazel.dashboard.github.issue;
+
+import static build.bazel.dashboard.utils.PgJson.toPgJson;
+import static java.util.Objects.requireNonNull;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import io.r2dbc.postgresql.codec.Json;
+import io.r2dbc.spi.Readable;
+import java.io.IOException;
+import java.time.Instant;
+import java.util.Optional;
+import lombok.RequiredArgsConstructor;
+import org.springframework.r2dbc.core.DatabaseClient;
+import org.springframework.stereotype.Repository;
+
+@Repository
+@RequiredArgsConstructor
+public class GithubPullRequestRepo {
+ private final DatabaseClient databaseClient;
+ private final ObjectMapper objectMapper;
+
+ public void save(GithubPullRequest pullRequest) {
+ databaseClient
+ .sql(
+ "INSERT INTO github_pull_request_data (owner, repo, issue_number, timestamp, etag, data)"
+ + " VALUES (:owner, :repo, :issue_number, :timestamp, :etag, :data) ON"
+ + " CONFLICT (owner, repo, issue_number) DO UPDATE SET etag = excluded.etag,"
+ + " timestamp = excluded.timestamp, data = excluded.data")
+ .bind("owner", pullRequest.owner())
+ .bind("repo", pullRequest.repo())
+ .bind("issue_number", pullRequest.issueNumber())
+ .bind("timestamp", pullRequest.timestamp())
+ .bind("etag", pullRequest.etag())
+ .bind("data", toPgJson(objectMapper, pullRequest.data()))
+ .then()
+ .block();
+ }
+
+ public void delete(String owner, String repo, int issueNumber) {
+ databaseClient
+ .sql(
+ "DELETE FROM github_pull_request_data WHERE owner = :owner AND repo = :repo AND"
+ + " issue_number = :issue_number")
+ .bind("owner", owner)
+ .bind("repo", repo)
+ .bind("issue_number", issueNumber)
+ .then()
+ .block();
+ }
+
+ public Optional<GithubPullRequest> findOne(String owner, String repo, int issueNumber) {
+ return Optional.ofNullable(
+ databaseClient
+ .sql(
+ "SELECT owner, repo, issue_number, timestamp, etag, data FROM github_pull_request_data "
+ + "WHERE owner=:owner AND repo=:repo AND issue_number=:issue_number")
+ .bind("owner", owner)
+ .bind("repo", repo)
+ .bind("issue_number", issueNumber)
+ .map(this::toPullRequest)
+ .one()
+ .block());
+ }
+
+ private GithubPullRequest toPullRequest(Readable row) {
+ try {
+ return new GithubPullRequest(
+ requireNonNull(row.get("owner", String.class)),
+ requireNonNull(row.get("repo", String.class)),
+ requireNonNull(row.get("issue_number", Integer.class)),
+ requireNonNull(row.get("timestamp", Instant.class)),
+ requireNonNull(row.get("etag", String.class)),
+ objectMapper.readTree((requireNonNull(row.get("data", Json.class))).asArray()));
+ } catch (IOException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+}
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/issuelist/GithubIssueListRepoPg.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/issuelist/GithubIssueListRepoPg.java
index 017134d..ac965e8 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/issuelist/GithubIssueListRepoPg.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/issuelist/GithubIssueListRepoPg.java
@@ -8,6 +8,8 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import io.r2dbc.postgresql.codec.Json;
+import io.r2dbc.postgresql.codec.PostgresqlObjectId;
+import io.r2dbc.spi.Parameters;
import io.r2dbc.spi.Readable;
import io.reactivex.rxjava3.core.Flowable;
import io.reactivex.rxjava3.core.Single;
@@ -76,7 +78,9 @@
if (params.getLabels() != null && !params.getLabels().isEmpty()) {
where.append(" AND gi.labels @> :labels");
- bindings.put("labels", params.getLabels().toArray(new String[0]));
+ bindings.put(
+ "labels",
+ Parameters.in(PostgresqlObjectId.UNSPECIFIED, params.getLabels().toArray(new String[0])));
}
if (where.length() > 0) {
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/issuequery/GithubIssueQueryExecutorPg.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/issuequery/GithubIssueQueryExecutorPg.java
index 9c0a56a..23ec7ab 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/issuequery/GithubIssueQueryExecutorPg.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/issuequery/GithubIssueQueryExecutorPg.java
@@ -5,8 +5,8 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import io.r2dbc.postgresql.codec.Json;
-import io.reactivex.rxjava3.core.Flowable;
-import io.reactivex.rxjava3.core.Single;
+import io.r2dbc.postgresql.codec.PostgresqlObjectId;
+import io.r2dbc.spi.Parameters;
import lombok.Builder;
import lombok.RequiredArgsConstructor;
import lombok.Value;
@@ -129,12 +129,19 @@
if (!parsedQuery.getLabels().isEmpty()) {
and(condition, "labels @> :labels");
- bindings.put("labels", parsedQuery.getLabels().toArray(new String[0]));
+ bindings.put(
+ "labels",
+ Parameters.in(
+ PostgresqlObjectId.UNSPECIFIED, parsedQuery.getLabels().toArray(new String[0])));
}
if (!parsedQuery.getExcludeLabels().isEmpty()) {
and(condition, "NOT labels && :excluded_labels");
- bindings.put("excluded_labels", parsedQuery.getExcludeLabels().toArray(new String[0]));
+ bindings.put(
+ "excluded_labels",
+ Parameters.in(
+ PostgresqlObjectId.UNSPECIFIED,
+ parsedQuery.getExcludeLabels().toArray(new String[0])));
}
return SqlCondition.builder().condition(condition.toString()).bindings(bindings).build();
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/issuestatus/GithubIssueStatusRestController.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/issuestatus/GithubIssueStatusRestController.java
index ab342ce..e942749 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/issuestatus/GithubIssueStatusRestController.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/issuestatus/GithubIssueStatusRestController.java
@@ -3,6 +3,7 @@
import static build.bazel.dashboard.utils.RxJavaVirtualThread.completable;
import build.bazel.dashboard.github.issue.GithubIssueService;
+import build.bazel.dashboard.github.issue.GithubPullRequestRepo;
import io.reactivex.rxjava3.core.Completable;
import java.time.Instant;
import lombok.RequiredArgsConstructor;
@@ -17,6 +18,7 @@
@RequiredArgsConstructor
public class GithubIssueStatusRestController {
private final GithubIssueService githubIssueService;
+ private final GithubPullRequestRepo githubPullRequestRepo;
private final GithubIssueStatusService githubIssueStatusService;
@PutMapping("/internal/github/{owner}/{repo}/issues/status")
@@ -29,8 +31,9 @@
() -> {
for (var number = start; number < start + count; ++number) {
var issue = githubIssueService.findOne(owner, repo, number);
+ var pullRequest = githubPullRequestRepo.findOne(owner, repo, number).orElse(null);
if (issue.isPresent()) {
- githubIssueStatusService.check(issue.get(), Instant.now());
+ githubIssueStatusService.check(issue.get(), pullRequest, Instant.now());
}
}
});
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/issuestatus/GithubIssueStatusService.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/issuestatus/GithubIssueStatusService.java
index 6abc854..50bc2c7 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/issuestatus/GithubIssueStatusService.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/issuestatus/GithubIssueStatusService.java
@@ -5,17 +5,13 @@
import build.bazel.dashboard.github.issue.GithubIssue;
import build.bazel.dashboard.github.issue.GithubIssue.Label;
import build.bazel.dashboard.github.issue.GithubIssue.User;
-import build.bazel.dashboard.github.issuestatus.GithubIssueStatus.GithubIssueStatusBuilder;
+import build.bazel.dashboard.github.issue.GithubPullRequest;
import build.bazel.dashboard.github.issuestatus.GithubIssueStatus.Status;
import build.bazel.dashboard.github.repo.GithubRepo;
import build.bazel.dashboard.github.repo.GithubRepoService;
-import build.bazel.dashboard.github.team.GithubTeam;
import build.bazel.dashboard.github.team.GithubTeamService;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
-import io.reactivex.rxjava3.core.Completable;
-import io.reactivex.rxjava3.core.Maybe;
-import io.reactivex.rxjava3.core.Single;
import java.io.IOException;
import java.time.Instant;
import java.util.List;
@@ -38,7 +34,8 @@
return githubIssueStatusRepo.findOne(owner, repo, issueNumber);
}
- public Optional<GithubIssueStatus> check(GithubIssue issue, Instant now) throws IOException {
+ public Optional<GithubIssueStatus> check(
+ GithubIssue issue, @Nullable GithubPullRequest pullRequest, Instant now) throws IOException {
String owner = issue.getOwner();
String repo = issue.getRepo();
@@ -50,7 +47,8 @@
var githubRepo = githubRepoOptional.get();
var existingIssueStatus = githubIssueStatusRepo.findOne(owner, repo, issue.getIssueNumber());
- var newIssueStatus = buildStatus(githubRepo, existingIssueStatus.orElse(null), issue, now);
+ var newIssueStatus =
+ buildStatus(githubRepo, existingIssueStatus.orElse(null), issue, pullRequest, now);
githubIssueStatusRepo.save(newIssueStatus);
return Optional.of(newIssueStatus);
}
@@ -64,9 +62,17 @@
private GithubIssueStatus buildStatus(
GithubRepo repo,
- @Nullable GithubIssueStatus oldStatus, GithubIssue issue, Instant now) throws IOException {
+ @Nullable GithubIssueStatus oldStatus,
+ GithubIssue issue,
+ @Nullable GithubPullRequest pullRequest,
+ Instant now)
+ throws IOException {
var data = issue.parseData(objectMapper);
- Status newStatus = newStatus(repo, data);
+ GithubPullRequest.Data pullRequestData = null;
+ if (pullRequest != null) {
+ pullRequestData = pullRequest.parseData(objectMapper);
+ }
+ Status newStatus = newStatus(repo, data, pullRequestData);
Instant lastNotifiedAt = null;
if (oldStatus != null) {
@@ -82,22 +88,23 @@
nextNotifyAt = lastNotifiedAt.plus(1, DAYS);
}
- var actionOwners = findActionOwner(repo, issue, data, newStatus);
+ var actionOwners = findActionOwner(repo, issue, data, pullRequestData, newStatus);
return GithubIssueStatus.builder()
- .owner(issue.getOwner())
- .repo(issue.getRepo())
- .issueNumber(issue.getIssueNumber())
- .status(newStatus)
+ .owner(issue.getOwner())
+ .repo(issue.getRepo())
+ .issueNumber(issue.getIssueNumber())
+ .status(newStatus)
.actionOwners(actionOwners)
.updatedAt(data.getUpdatedAt())
- .expectedRespondAt(expectedRespondAt)
- .lastNotifiedAt(lastNotifiedAt)
- .nextNotifyAt(nextNotifyAt)
- .checkedAt(now)
- .build();
+ .expectedRespondAt(expectedRespondAt)
+ .lastNotifiedAt(lastNotifiedAt)
+ .nextNotifyAt(nextNotifyAt)
+ .checkedAt(now)
+ .build();
}
- static Status newStatus(GithubRepo repo, GithubIssue.Data data) {
+ static Status newStatus(
+ GithubRepo repo, GithubIssue.Data data, @Nullable GithubPullRequest.Data pullRequestData) {
if (data.getState().equals("closed")) {
return Status.CLOSED;
}
@@ -108,8 +115,16 @@
return Status.MORE_DATA_NEEDED;
}
+ if (pullRequestData != null && !pullRequestData.requestedReviewers().isEmpty()) {
+ return Status.TRIAGED;
+ }
+
+ if (!data.getAssignees().isEmpty()) {
+ return Status.TRIAGED;
+ }
+
if (!repo.isTeamLabelEnabled() || hasTeamLabel(labels)) {
- if (hasPriorityLabel(labels)) {
+ if (isTriaged(labels)) {
return Status.TRIAGED;
} else {
return Status.REVIEWED;
@@ -121,68 +136,78 @@
// TODO: More serious business days handling
static @Nullable Instant getExpectedRespondAt(GithubIssue.Data data, Status status) {
- Instant updatedAt = data.getUpdatedAt();
+ return switch (status) {
+ case TO_BE_REVIEWED, MORE_DATA_NEEDED, REVIEWED -> data.getUpdatedAt().plus(7, DAYS);
+ case TRIAGED -> getExpectedRespondAtForTriaged(data);
+ default -> null;
+ };
+ }
- switch (status) {
- case TO_BE_REVIEWED:
- case MORE_DATA_NEEDED:
- case REVIEWED:
- return updatedAt.plus(7, DAYS);
- case TRIAGED:
- {
- List<Label> labels = data.getLabels();
- if (hasLabelPrefix(labels, "type: bug")) {
- if (hasLabelPrefix(labels, "P0")) {
- return updatedAt.plus(1, DAYS);
- } else if (hasLabelPrefix(labels, "P1")) {
- return updatedAt.plus(7, DAYS);
- } else if (hasLabelPrefix(labels, "P2")) {
- return updatedAt.plus(120, DAYS);
- }
- }
-
- return null;
- }
-
- default:
+ static @Nullable Instant getExpectedRespondAtForTriaged(GithubIssue.Data data) {
+ List<Label> labels = data.getLabels();
+ if (hasLabelPrefix(labels, "type: bug")) {
+ if (hasLabelPrefix(labels, "P0")) {
+ return data.getUpdatedAt().plus(1, DAYS);
+ } else if (hasLabelPrefix(labels, "P1")) {
+ return data.getUpdatedAt().plus(7, DAYS);
+ } else if (hasLabelPrefix(labels, "P2")) {
+ return data.getUpdatedAt().plus(120, DAYS);
+ }
}
-
return null;
}
private ImmutableList<String> findActionOwner(
- GithubRepo repo, GithubIssue issue, GithubIssue.Data data, Status status) {
- switch (status) {
- case TO_BE_REVIEWED:
- return ImmutableList.of();
- case MORE_DATA_NEEDED:
- return ImmutableList.of(data.getUser().getLogin());
- case REVIEWED:
- case TRIAGED:
- User assignee = data.getAssignee();
- if (assignee != null) {
- return ImmutableList.of(assignee.getLogin());
- } else {
- List<Label> labels = data.getLabels();
- githubTeamService
- .findAll(issue.getOwner(), issue.getRepo())
- .stream()
- .filter(
- team ->
- labels.stream().anyMatch(label -> label.getName().equals(team.getLabel()))
- && !team.getTeamOwners().isEmpty())
- .findFirst()
- .map(GithubTeam::getTeamOwners)
- .orElseGet(() -> {
- if (repo.getActionOwner() != null) {
- return ImmutableList.of(repo.getActionOwner());
- }
- return ImmutableList.of();
- });
- }
+ GithubRepo repo,
+ GithubIssue issue,
+ GithubIssue.Data data,
+ @Nullable GithubPullRequest.Data pullRequestData,
+ Status status) {
+ return switch (status) {
+ case MORE_DATA_NEEDED -> ImmutableList.of(data.getUser().getLogin());
+ case REVIEWED, TRIAGED -> findActionOwnerForReviewedOrTriaged(
+ repo, issue, data, pullRequestData);
+ default -> ImmutableList.of();
+ };
+ }
+
+ private ImmutableList<String> findActionOwnerForReviewedOrTriaged(
+ GithubRepo repo,
+ GithubIssue issue,
+ GithubIssue.Data data,
+ @Nullable GithubPullRequest.Data pullRequestData) {
+
+ if (pullRequestData != null) {
+ if (!pullRequestData.requestedReviewers().isEmpty()) {
+ return pullRequestData.requestedReviewers().stream()
+ .map(User::getLogin)
+ .collect(ImmutableList.toImmutableList());
+ }
}
- return ImmutableList.of();
+ var assignees = data.getAssignees();
+ if (!assignees.isEmpty()) {
+ return assignees.stream().map(User::getLogin).collect(ImmutableList.toImmutableList());
+ } else {
+ List<Label> labels = data.getLabels();
+ var teams =
+ githubTeamService.findAll(issue.getOwner(), issue.getRepo()).stream()
+ .filter(
+ team ->
+ labels.stream()
+ .anyMatch(label -> label.getName().equalsIgnoreCase(team.getLabel())))
+ .toList();
+ if (teams.isEmpty()) {
+ if (repo.getActionOwner() != null) {
+ return ImmutableList.of(repo.getActionOwner());
+ } else {
+ return ImmutableList.of();
+ }
+ }
+ return teams.stream()
+ .flatMap(team -> team.getTeamOwners().stream())
+ .collect(ImmutableList.toImmutableList());
+ }
}
private static boolean hasTeamLabel(List<Label> labels) {
@@ -190,16 +215,33 @@
}
private static boolean hasMoreDataNeededLabel(List<Label> labels) {
- return hasLabelPrefix(labels, "more data needed");
+ return hasLabel(labels, "more data needed")
+ || hasLabel(labels, "awaiting-user-response");
}
- private static boolean hasPriorityLabel(List<Label> labels) {
- return hasLabelPrefix(labels, "P");
+ private static boolean isTriaged(List<Label> labels) {
+ return hasLabel(labels, "P0")
+ || hasLabel(labels, "P1")
+ || hasLabel(labels, "P2")
+ || hasLabel(labels, "P3")
+ || hasLabel(labels, "P4")
+ || hasLabel(labels, "awaiting-review")
+ || hasLabel(labels, "awaiting-PR-merge");
}
private static boolean hasLabelPrefix(List<Label> labels, String prefix) {
for (Label label : labels) {
- if (label.getName().startsWith(prefix)) {
+ if (label.getName().toLowerCase().startsWith(prefix.toLowerCase())) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ private static boolean hasLabel(List<Label> labels, String name) {
+ for (Label label : labels) {
+ if (label.getName().equalsIgnoreCase(name)) {
return true;
}
}
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/notification/NotificationTask.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/notification/NotificationTask.java
index fe4338e..cf487f4 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/notification/NotificationTask.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/notification/NotificationTask.java
@@ -326,6 +326,7 @@
private Single<String> buildUserNotificationBody(GithubUser user) {
return Flowable.concatArray(
buildNeedTriageMessage(user).toFlowable(),
+ buildReviewPrMessage(user).toFlowable(),
buildFixP0BugsMessage(user).toFlowable(),
buildFixP1BugsMessage(user).toFlowable(),
buildFixP2BugsMessage(user).toFlowable())
@@ -372,8 +373,31 @@
});
}
+ Single<String> buildReviewPrMessage(GithubUser user) {
+ ListParams params = new ListParams();
+ params.setIsPullRequest(true);
+ params.setLabels(ImmutableList.of("awaiting-review"));
+ params.setActionOwner(user.getUsername());
+
+ return githubIssueListService
+ .find(params)
+ .flatMap(
+ list -> {
+ if (list.getTotal() > 0) {
+ String reviewLink =
+ dashboardConfig.getHost()
+ + "/issues?q=%7B%22isPullRequest%22%3Atrue%2C%22actionOwner%22%3A%22"
+ + user.getUsername()
+ + "%22%2C%22page%22%3A1%2C%22extraLabels%22%3A%5B%22awaiting-review%22%5D%7D";
+ return buildNotificationBody(reviewLink, "PRs", "review", list);
+ }
+ return Single.just("");
+ });
+ }
+
Single<String> buildFixP0BugsMessage(GithubUser user) {
ListParams params = new ListParams();
+ params.setIsPullRequest(false);
params.setStatus(GithubIssueStatus.Status.TRIAGED);
params.setLabels(ImmutableList.of("P0", "type: bug"));
params.setActionOwner(user.getUsername());
@@ -396,6 +420,7 @@
Single<String> buildFixP1BugsMessage(GithubUser user) {
ListParams params = new ListParams();
+ params.setIsPullRequest(false);
params.setStatus(GithubIssueStatus.Status.TRIAGED);
params.setLabels(ImmutableList.of("P1", "type: bug"));
params.setActionOwner(user.getUsername());
@@ -418,6 +443,7 @@
Single<String> buildFixP2BugsMessage(GithubUser user) {
ListParams params = new ListParams();
+ params.setIsPullRequest(false);
params.setStatus(GithubIssueStatus.Status.TRIAGED);
params.setLabels(ImmutableList.of("P2", "type: bug"));
params.setActionOwner(user.getUsername());
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/repo/GithubRepoRepoPg.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/repo/GithubRepoRepoPg.java
index aa9af92..0a240c4 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/repo/GithubRepoRepoPg.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/repo/GithubRepoRepoPg.java
@@ -36,7 +36,7 @@
return databaseClient
.sql(
"SELECT owner, repo, created_at, updated_at, action_owner, is_team_label_enabled"
- + " FROM github_repo")
+ + " FROM github_repo ORDER BY owner, repo")
.map(this::toGithubRepo)
.all()
.collect(toImmutableList())
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/sync/issue/GithubSyncIssueTask.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/sync/issue/GithubSyncIssueTask.java
index 7fa73db..546377e 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/sync/issue/GithubSyncIssueTask.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/sync/issue/GithubSyncIssueTask.java
@@ -8,6 +8,8 @@
import build.bazel.dashboard.github.repo.GithubRepoService;
import build.bazel.dashboard.utils.JsonStateStore;
import build.bazel.dashboard.utils.JsonStateStore.JsonState;
+import com.fasterxml.jackson.databind.PropertyNamingStrategies;
+import com.fasterxml.jackson.databind.annotation.JsonNaming;
import io.reactivex.rxjava3.core.Completable;
import java.time.Instant;
import javax.annotation.Nullable;
@@ -33,12 +35,14 @@
@Builder
@Value
+ @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
static class SyncIssueState {
String owner;
String repo;
int start;
@With int current;
int end;
+ Instant createdAt;
}
@PutMapping("/internal/github/{owner}/{repo}/sync/issues")
@@ -67,6 +71,7 @@
.start(start)
.current(start)
.end(end)
+ .createdAt(Instant.now())
.build());
}
@@ -80,10 +85,9 @@
for (var repo : githubRepoService.findAll()) {
String stateKey = buildSyncStateKey(repo.getOwner(), repo.getRepo());
var jsonState = jsonStateStore.load(stateKey, SyncIssueState.class);
- if (jsonState.getData() != null) {
- return;
+ if (jsonState.getData() == null) {
+ saveAllSyncIssueState(repo.getOwner(), repo.getRepo(), jsonState.getTimestamp());
}
- saveAllSyncIssueState(repo.getOwner(), repo.getRepo(), jsonState.getTimestamp());
}
}
@@ -96,7 +100,6 @@
buildSyncStateKey(repo.getOwner(), repo.getRepo()), SyncIssueState.class);
if (jsonState.getData() != null) {
this.syncGithubIssue(jsonState);
- return;
}
}
}
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableRepo.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableRepo.java
index f37e94a..0abc040 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableRepo.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableRepo.java
@@ -18,7 +18,6 @@
Instant createdAt;
Instant updatedAt;
String name;
- String noneTeamOwner;
List<Header> headers;
@Builder
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableRepoPg.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableRepoPg.java
index f5218d3..e6b1154 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableRepoPg.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableRepoPg.java
@@ -20,7 +20,7 @@
Optional.ofNullable(
databaseClient
.sql(
- "SELECT owner, repo, id, created_at, updated_at, name, none_team_owner FROM"
+ "SELECT owner, repo, id, created_at, updated_at, name FROM"
+ " github_team_table WHERE owner = :owner AND repo = :repo AND id = :id")
.bind("owner", owner)
.bind("repo", repo)
@@ -33,8 +33,7 @@
.id(row.get("id", String.class))
.createdAt(row.get("created_at", Instant.class))
.updatedAt(row.get("updated_at", Instant.class))
- .name(row.get("name", String.class))
- .noneTeamOwner(row.get("none_team_owner", String.class)))
+ .name(row.get("name", String.class)))
.one()
.block());
if (builderOptional.isEmpty()) {
diff --git a/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableService.java b/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableService.java
index 8d4d2bb..4b1e7b0 100644
--- a/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableService.java
+++ b/dashboard/server/src/main/java/build/bazel/dashboard/github/teamtable/GithubTeamTableService.java
@@ -4,6 +4,7 @@
import build.bazel.dashboard.github.GithubUtils;
import build.bazel.dashboard.github.issuequery.GithubIssueQueryExecutor;
+import build.bazel.dashboard.github.repo.GithubRepoService;
import build.bazel.dashboard.github.team.GithubTeam;
import build.bazel.dashboard.github.team.GithubTeamService;
import build.bazel.dashboard.github.teamtable.GithubTeamTable.Row;
@@ -32,6 +33,7 @@
@RequiredArgsConstructor
public class GithubTeamTableService {
private final GithubTeamTableRepo githubTeamTableRepo;
+ private final GithubRepoService githubRepoService;
private final GithubTeamService githubTeamService;
private final GithubIssueQueryExecutor githubIssueQueryExecutor;
@@ -62,13 +64,21 @@
}
GithubTeamTable loadOne(String owner, String repo, String tableId) {
- return githubTeamTableRepo
- .findOne(owner, repo, tableId)
- .map(
- table -> {
- var teams = findAllTeams(owner, repo, table.getNoneTeamOwner());
- return fetchTable(teams, table);
- })
+ return githubRepoService
+ .findOne(owner, repo)
+ .flatMap(
+ githubRepo ->
+ githubTeamTableRepo
+ .findOne(githubRepo.getOwner(), githubRepo.getRepo(), tableId)
+ .map(
+ table -> {
+ var noneTeamOwner = "";
+ if (githubRepo.getActionOwner() != null) {
+ noneTeamOwner = githubRepo.getActionOwner();
+ }
+ var teams = findAllTeams(owner, repo, noneTeamOwner);
+ return fetchTable(teams, table);
+ }))
.orElseGet(() -> GithubTeamTable.buildNone(owner, repo, tableId, ""));
}
