diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java index 3033a4495..3598a1921 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java @@ -26,6 +26,7 @@ import org.openjdk.skara.forge.*; import org.openjdk.skara.host.HostUser; import org.openjdk.skara.issuetracker.Comment; +import org.openjdk.skara.issuetracker.IssueProject; import org.openjdk.skara.vcs.*; import org.openjdk.skara.vcs.openjdk.Issue; @@ -89,6 +90,41 @@ private boolean isTargetBranchAllowed() { return matcher.matches(); } + private Set allowedIssueTypes() { + return workItem.bot.allowedIssueTypes(); + } + + private List issues() { + var issue = Issue.fromStringRelaxed(pr.title()); + if (issue.isPresent()) { + var issues = new ArrayList(); + issues.add(issue.get()); + issues.addAll(SolvesTracker.currentSolved(pr.repository().forge().currentUser(), comments)); + return issues; + } + return List.of(); + } + + private IssueProject issueProject() { + return workItem.bot.issueProject(); + } + + private List issuesOfDisallowedType() { + var issueProject = issueProject(); + var allowed = allowedIssueTypes(); + if (issueProject != null && allowed != null) { + return issues().stream() + .filter(i -> i.project().equals(Optional.of(issueProject.name()))) + .map(i -> issueProject.issue(i.shortId())) + .filter(Optional::isPresent) + .map(Optional::get) + .filter(i -> i.properties().containsKey("issuetype")) + .filter(i -> !allowed.contains(i.properties().get("issuetype").asString())) + .collect(Collectors.toList()); + } + return List.of(); + } + private List allowedTargetBranches() { return pr.repository() .branches() @@ -135,6 +171,20 @@ private List botSpecificChecks(Hash finalHash) throws IOException { ret.add(error); } + var disallowedIssues = issuesOfDisallowedType(); + if (!disallowedIssues.isEmpty()) { + var s = disallowedIssues.size() > 1 ? "s " : " "; + var are = disallowedIssues.size() > 1 ? "are" : "is"; + var links = disallowedIssues.stream() + .map(i -> "[" + i.id() + "](" + i.webUrl() + ")") + .collect(Collectors.toList()); + var error = "The issue" + s + String.join(",", links) + " " + are + " not of the expected type. The allowed issue types are:\n" + + allowedIssueTypes().stream() + .map(name -> " - " + name) + .collect(Collectors.joining("\n")); + ret.add(error); + } + var headHash = pr.headHash(); var originalCommits = localRepo.commitMetadata(baseHash, headHash); @@ -313,18 +363,15 @@ private String getStatusMessage(List comments, List reviews, Pu progressBody.append(getAdditionalErrorsList(allAdditionalErrors)); } - var issue = Issue.fromStringRelaxed(pr.title()); - var issueProject = workItem.bot.issueProject(); - if (issueProject != null && issue.isPresent()) { - var allIssues = new ArrayList(); - allIssues.add(issue.get()); - allIssues.addAll(SolvesTracker.currentSolved(pr.repository().forge().currentUser(), comments)); + var issues = issues(); + var issueProject = issueProject(); + if (issueProject != null && !issues.isEmpty()) { progressBody.append("\n\n### Issue"); - if (allIssues.size() > 1) { + if (issues.size() > 1) { progressBody.append("s"); } progressBody.append("\n"); - for (var currentIssue : allIssues) { + for (var currentIssue : issues) { progressBody.append(" * "); if (currentIssue.project().isPresent() && !currentIssue.project().get().equals(issueProject.name())) { progressBody.append("⚠️ Issue `"); diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBot.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBot.java index b767a294a..bebca84c2 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBot.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBot.java @@ -45,6 +45,7 @@ class PullRequestBot implements Bot { private final Map readyComments; private final IssueProject issueProject; private final boolean ignoreStaleReviews; + private final Set allowedIssueTypes; private final Pattern allowedTargetBranches; private final Path seedStorage; private final ConcurrentMap currentLabels; @@ -55,7 +56,7 @@ class PullRequestBot implements Bot { LabelConfiguration labelConfiguration, Map externalCommands, Map blockingCheckLabels, Set readyLabels, Map readyComments, IssueProject issueProject, boolean ignoreStaleReviews, - Pattern allowedTargetBranches, Path seedStorage) { + Set allowedIssueTypes, Pattern allowedTargetBranches, Path seedStorage) { remoteRepo = repo; this.censusRepo = censusRepo; this.censusRef = censusRef; @@ -66,6 +67,7 @@ class PullRequestBot implements Bot { this.issueProject = issueProject; this.readyComments = readyComments; this.ignoreStaleReviews = ignoreStaleReviews; + this.allowedIssueTypes = allowedIssueTypes; this.allowedTargetBranches = allowedTargetBranches; this.seedStorage = seedStorage; @@ -185,6 +187,10 @@ boolean ignoreStaleReviews() { return ignoreStaleReviews; } + Set allowedIssueTypes() { + return allowedIssueTypes; + } + Pattern allowedTargetBranches() { return allowedTargetBranches; } diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java index f55057e1a..d019c643e 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotBuilder.java @@ -40,6 +40,7 @@ public class PullRequestBotBuilder { private Map readyComments = Map.of(); private IssueProject issueProject = null; private boolean ignoreStaleReviews = false; + private Set allowedIssueTypes = null; private Pattern allowedTargetBranches = Pattern.compile(".*"); private Path seedStorage = null; @@ -96,6 +97,11 @@ public PullRequestBotBuilder ignoreStaleReviews(boolean ignoreStaleReviews) { return this; } + public PullRequestBotBuilder allowedIssueTypes(Set allowedIssueTypes) { + this.allowedIssueTypes = allowedIssueTypes; + return this; + } + public PullRequestBotBuilder allowedTargetBranches(String allowedTargetBranches) { this.allowedTargetBranches = Pattern.compile(allowedTargetBranches); return this; @@ -107,8 +113,9 @@ public PullRequestBotBuilder seedStorage(Path seedStorage) { } public PullRequestBot build() { - return new PullRequestBot(repo, censusRepo, censusRef, labelConfiguration, externalCommands, blockingCheckLabels, - readyLabels, readyComments, issueProject, ignoreStaleReviews, allowedTargetBranches, + return new PullRequestBot(repo, censusRepo, censusRef, labelConfiguration, externalCommands, + blockingCheckLabels, readyLabels, readyComments, issueProject, + ignoreStaleReviews, allowedIssueTypes, allowedTargetBranches, seedStorage); } -} \ No newline at end of file +} diff --git a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotFactory.java b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotFactory.java index 36d862891..37a3ea43d 100644 --- a/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotFactory.java +++ b/bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBotFactory.java @@ -118,6 +118,12 @@ public List create(BotConfiguration configuration) { if (repo.value().contains("ignorestale")) { botBuilder.ignoreStaleReviews(repo.value().get("ignorestale").asBoolean()); } + if (repo.value().contains("issuetypes")) { + var types = repo.value().get("issuetypes").stream() + .map(JSONValue::asString) + .collect(Collectors.toSet()); + botBuilder.allowedIssueTypes(types); + } if (repo.value().contains("targetbranches")) { botBuilder.allowedTargetBranches(repo.value().get("targetbranches").asString()); } diff --git a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java index 2047592de..ba7e841ab 100644 --- a/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java +++ b/bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckTests.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.*; import org.openjdk.skara.forge.*; +import org.openjdk.skara.json.JSON; import org.openjdk.skara.test.*; import java.io.IOException; @@ -1384,4 +1385,67 @@ void targetBranchPattern(TestInfo testInfo) throws IOException { } } + @Test + void allowedIssueTypes(TestInfo testInfo) throws IOException { + try (var credentials = new HostCredentials(testInfo); + var tempFolder = new TemporaryDirectory()) { + var author = credentials.getHostedRepository(); + var reviewer = credentials.getHostedRepository(); + var issues = credentials.getIssueProject(); + + var censusBuilder = credentials.getCensusBuilder() + .addAuthor(author.forge().currentUser().id()) + .addReviewer(reviewer.forge().currentUser().id()); + var checkBot = PullRequestBot.newBuilder().repo(author).censusRepo(censusBuilder.build()) + .allowedIssueTypes(Set.of("Bug")) + .issueProject(issues) + .build(); + + var bug = issues.createIssue("My first bug", List.of("A bug"), + Map.of("issuetype", JSON.of("Bug"))); + var feature = issues.createIssue("My first feature", List.of("A feature"), + Map.of("issuetype", JSON.of("Enhancement"))); + + // Populate the projects repository + var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType()); + var masterHash = localRepo.resolve("master").orElseThrow(); + localRepo.push(masterHash, author.url(), "master", true); + + // Make a change with a corresponding PR + var bugHash = CheckableRepository.appendAndCommit(localRepo); + localRepo.push(bugHash, author.url(), "bug", true); + var bugPR = credentials.createPullRequest(author, "master", "bug", + bug.id() + ": My first bug", true); + + // Check the status + TestBotRunner.runPeriodicItems(checkBot); + + // Verify that the check passed + var bugChecks = bugPR.checks(bugHash); + assertEquals(1, bugChecks.size()); + var bugCheck = bugChecks.get("jcheck"); + assertEquals(CheckStatus.SUCCESS, bugCheck.status()); + + // Make a change with a corresponding PR + var featureHash = CheckableRepository.appendAndCommit(localRepo); + localRepo.push(featureHash, author.url(), "feature", true); + var featurePR = credentials.createPullRequest(author, "master", "feature", + feature.id() + ": My first feature", true); + + // Check the status + TestBotRunner.runPeriodicItems(checkBot); + + // Verify that the check failed for the feature PR + var featureChecks = featurePR.checks(featureHash); + assertEquals(1, featureChecks.size()); + var featureCheck = featureChecks.get("jcheck"); + assertEquals(CheckStatus.FAILURE, featureCheck.status()); + var link = "[" + feature.id() + "](" + feature.webUrl() + ")"; + assertTrue(featureCheck.summary() + .orElseThrow() + .contains("The issue " + link + " is not of the expected type. " + + "The allowed issue types are:\n" + + " - Bug\n")); + } + } }