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

549: Links to users under Reviewers lead to a 404 page #763

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 18 additions & 4 deletions bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java
Expand Up @@ -263,11 +263,25 @@ private String formatReviewer(HostUser reviewer) {
var namespace = censusInstance.namespace();
var contributor = namespace.get(reviewer.id());
if (contributor == null) {
return reviewer.userName() + " (no known " + namespace.name() + " user name / role)";
return "@" + reviewer.userName() + " (no known " + namespace.name() + " user name / role)";
} else {
var userNameLink = "[" + contributor.username() + "](@" + reviewer.userName() + ")";
return contributor.fullName().orElse(contributor.username()) + " (" + userNameLink + " - " +
getRole(contributor.username()) + ")";
var ret = new StringBuilder();
var censusLink = workItem.bot.censusLink(contributor);
if (censusLink.isPresent()) {
ret.append("[");
}
ret.append(contributor.fullName().orElse(contributor.username()));
if (censusLink.isPresent()) {
ret.append("](");
ret.append(censusLink.get().toString());
ret.append("]");
}
ret.append(" (@");
ret.append(reviewer.userName());
ret.append(" - ");
ret.append(getRole(contributor.username()));
ret.append(")");
return ret.toString();
}
}

Expand Down
Expand Up @@ -23,11 +23,13 @@
package org.openjdk.skara.bots.pr;

import org.openjdk.skara.bot.*;
import org.openjdk.skara.census.Contributor;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.issuetracker.IssueProject;
import org.openjdk.skara.json.JSONValue;
import org.openjdk.skara.vcs.Hash;

import java.net.URI;
import java.nio.file.Path;
import java.util.*;
import java.util.concurrent.*;
Expand All @@ -51,6 +53,7 @@ class PullRequestBot implements Bot {
private final HostedRepository confOverrideRepo;
private final String confOverrideName;
private final String confOverrideRef;
private final String censusLink;
private final ConcurrentMap<Hash, Boolean> currentLabels;
private final PullRequestUpdateCache updateCache;
private final Logger log = Logger.getLogger("org.openjdk.skara.bots.pr");
Expand All @@ -60,7 +63,8 @@ class PullRequestBot implements Bot {
Map<String, String> blockingCheckLabels, Set<String> readyLabels,
Map<String, Pattern> readyComments, IssueProject issueProject, boolean ignoreStaleReviews,
Set<String> allowedIssueTypes, Pattern allowedTargetBranches, Path seedStorage,
HostedRepository confOverrideRepo, String confOverrideName, String confOverrideRef) {
HostedRepository confOverrideRepo, String confOverrideName, String confOverrideRef,
String censusLink) {
remoteRepo = repo;
this.censusRepo = censusRepo;
this.censusRef = censusRef;
Expand All @@ -77,6 +81,7 @@ class PullRequestBot implements Bot {
this.confOverrideRepo = confOverrideRepo;
this.confOverrideName = confOverrideName;
this.confOverrideRef = confOverrideRef;
this.censusLink = censusLink;

this.currentLabels = new ConcurrentHashMap<>();
this.updateCache = new PullRequestUpdateCache();
Expand Down Expand Up @@ -215,4 +220,11 @@ String confOverrideName() {
String confOverrideRef() {
return confOverrideRef;
}

Optional<URI> censusLink(Contributor contributor) {
if (censusLink == null) {
return Optional.empty();
}
return Optional.of(URI.create(censusLink.replace("{{contributor}}", contributor.username())));
}
}
Expand Up @@ -46,6 +46,7 @@ public class PullRequestBotBuilder {
private HostedRepository confOverrideRepo = null;
private String confOverrideName = ".conf/jcheck";
private String confOverrideRef = "master";
private String censusLink = null;

PullRequestBotBuilder() {
}
Expand Down Expand Up @@ -130,10 +131,16 @@ public PullRequestBotBuilder confOverrideRef(String confOverrideRef) {
return this;
}

public PullRequestBotBuilder censusLink(String censusLink) {
this.censusLink = censusLink;
return this;
}

public PullRequestBot build() {
return new PullRequestBot(repo, censusRepo, censusRef, labelConfiguration, externalCommands,
blockingCheckLabels, readyLabels, readyComments, issueProject,
ignoreStaleReviews, allowedIssueTypes, allowedTargetBranches,
seedStorage, confOverrideRepo, confOverrideName, confOverrideRef);
seedStorage, confOverrideRepo, confOverrideName, confOverrideRef,
censusLink);
}
}
Expand Up @@ -72,7 +72,6 @@ public List<Bot> create(BotConfiguration configuration) {
for (var repo : specific.get("repositories").fields()) {
var censusRepo = configuration.repository(repo.value().get("census").asString());
var censusRef = configuration.repositoryRef(repo.value().get("census").asString());

var botBuilder = PullRequestBot.newBuilder()
.repo(configuration.repository(repo.name()))
.censusRepo(censusRepo)
Expand Down Expand Up @@ -112,6 +111,9 @@ public List<Bot> create(BotConfiguration configuration) {
botBuilder.confOverrideName(repo.value().get("jcheck").get("name").asString());
}
}
if (repo.value().contains("censuslink")) {
botBuilder.censusLink(repo.value().get("censuslink").asString());
}

ret.add(botBuilder.build());
}
Expand Down
11 changes: 10 additions & 1 deletion bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java
Expand Up @@ -48,7 +48,11 @@ void simpleCommit(TestInfo testInfo) throws IOException {
var censusBuilder = credentials.getCensusBuilder()
.addAuthor(author.forge().currentUser().id())
.addReviewer(reviewer.forge().currentUser().id());
var checkBot = PullRequestBot.newBuilder().repo(author).censusRepo(censusBuilder.build()).build();
var checkBot = PullRequestBot.newBuilder()
.repo(author)
.censusRepo(censusBuilder.build())
.censusLink("https://census.com/{{contributor}}-profile")
.build();

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType());
Expand Down Expand Up @@ -88,6 +92,7 @@ void simpleCommit(TestInfo testInfo) throws IOException {

// The PR should now be ready
assertTrue(pr.labels().contains("ready"));
assertTrue(pr.body().contains("https://census.com/integrationreviewer2-profile"));
}
}

Expand Down Expand Up @@ -244,6 +249,10 @@ void multipleReviews(TestInfo testInfo) throws IOException {
assertEquals(1, authorPr.body().split("Generated Reviewer", -1).length - 1);
assertTrue(authorPr.reviews().size() >= 2);
assertFalse(authorPr.body().contains("Note"));

// No census link is set
var reviewerString = "Generated Reviewer 2 (@" + reviewer.forge().currentUser().userName() + " - **Reviewer**)";
assertTrue(authorPr.body().contains(reviewerString));
}
}

Expand Down