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

Assign unassigned issues when resolving them #357

Closed
wants to merge 2 commits into from
Closed
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,6 +22,7 @@
*/
package org.openjdk.skara.bots.notify;

import org.openjdk.skara.email.EmailAddress;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.issuetracker.Issue;
import org.openjdk.skara.issuetracker.*;
@@ -216,6 +217,20 @@ private Issue findIssue(Issue primary, String fixVersion) {
return createBackportIssue(primary);
}

private Optional<String> findIssueUsername(Commit commit) {
var authorEmail = EmailAddress.from(commit.author().email());
if (authorEmail.domain().equals("openjdk.org")) {
return Optional.of(authorEmail.localPart());
}

var committerEmail = EmailAddress.from(commit.committer().email());
if (!committerEmail.domain().equals("openjdk.org")) {
log.severe("Cannot determine issue tracker user name from committer email: " + committerEmail);
return Optional.empty();
}
return Optional.of(committerEmail.localPart());
}
Copy link
Member

@edvbld edvbld Jan 16, 2020

Choose a reason for hiding this comment

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

Instead of repeating Optional.of(issueProject.issueTracker().user(...)), you might want to code this like:

        String username = null;

        var authorEmail = EmailAddress.from(commit.author().email());
        if (authorEmail.domain().equals("openjdk.org")) {
            username = authorEmail.localPart();
        } else {
            var committerEmail = EmailAddress.from(commit.committer().email());
            if (!committerEmail.domain().equals("openjdk.org")) {
                log.severe("Cannot determine issue tracker user name from committer email: " + committerEmail);
            }
            username = committerEmail.localPart();
        }

        return username == null ? Optional.empty() : Optional.of(issueProject.issueTracker().user(username));

Copy link
Member

@edvbld edvbld Jan 16, 2020

Choose a reason for hiding this comment

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

Oh, I should mention that I'm fine your version as well 👍 If you want to use your version with early return, then please remove the else { part after the return 😃

Copy link
Member Author

@rwestberg rwestberg Jan 16, 2020

Choose a reason for hiding this comment

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

Fair enough, how about this? :)


@Override
public void handleCommits(HostedRepository repository, Repository localRepository, List<Commit> commits, Branch branch) {
for (var commit : commits) {
@@ -267,7 +282,16 @@ public void handleCommits(HostedRepository repository, Repository localRepositor
if (!alreadyPostedComment) {
issue.addComment(commitNotification);
}
issue.setState(Issue.State.RESOLVED);
if (issue.state() == Issue.State.OPEN) {
issue.setState(Issue.State.RESOLVED);
if (issue.assignees().isEmpty()) {
var username = findIssueUsername(commit);
if (username.isPresent()) {
var assignee = issueProject.issueTracker().user(username.get());
issue.setAssignees(List.of(assignee));
}
}
}

if (commitLink) {
var linkBuilder = Link.create(repository.webUrl(commit.hash()), "Commit")
@@ -926,27 +926,34 @@ void testIssue(TestInfo testInfo) throws IOException {
TestBotRunner.runPeriodicItems(notifyBot);

// Create an issue and commit a fix
var authorEmailAddress = issueProject.issueTracker().currentUser().userName() + "@openjdk.org";
var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue");
var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue", "Duke", authorEmailAddress);
localRepo.push(editHash, repo.url(), "master");
TestBotRunner.runPeriodicItems(notifyBot);

// The changeset should be reflected in a comment
var comments = issue.comments();
var updatedIssue = issueProject.issue(issue.id()).orElseThrow();

var comments = updatedIssue.comments();
assertEquals(1, comments.size());
var comment = comments.get(0);
assertTrue(comment.body().contains(editHash.abbreviate()));

// And in a link
var links = issue.links();
var links = updatedIssue.links();
assertEquals(1, links.size());
var link = links.get(0);
assertEquals(commitIcon, link.iconUrl().orElseThrow());
assertEquals("Commit", link.title().orElseThrow());
assertEquals(repo.webUrl(editHash), link.uri().orElseThrow());

// As well as a fixVersion
assertEquals(Set.of("0.1"), fixVersions(issue));
assertEquals(Set.of("0.1"), fixVersions(updatedIssue));

// The issue should be assigned and resolved
assertEquals(RESOLVED, updatedIssue.state());
assertEquals(List.of(issueProject.issueTracker().currentUser()), updatedIssue.assignees());
}
}

@@ -1200,24 +1207,27 @@ void testIssueBackport(TestInfo testInfo) throws IOException {
issue.setProperty("fixVersions", JSON.array().add("13.0.1"));
issue.setProperty("priority", JSON.of("1"));

var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue");
var authorEmailAddress = issueProject.issueTracker().currentUser().userName() + "@openjdk.org";
var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue", "Duke", authorEmailAddress);
localRepo.push(editHash, repo.url(), "master");
TestBotRunner.runPeriodicItems(notifyBot);

// The fixVersion should not have been updated
var updatedIssue = issueProject.issue(issue.id()).orElseThrow();
assertEquals(Set.of("13.0.1"), fixVersions(updatedIssue));
assertEquals(OPEN, updatedIssue.state());
assertEquals(List.of(), updatedIssue.assignees());

// There should be a link
var links = updatedIssue.links();
assertEquals(1, links.size());
var link = links.get(0);
var backport = link.issue().orElseThrow();

// The backport issue should have a correct fixVersion
// The backport issue should have a correct fixVersion and assignee
assertEquals(Set.of("12.0.2"), fixVersions(backport));
assertEquals(RESOLVED, backport.state());
assertEquals(List.of(issueProject.issueTracker().currentUser()), backport.assignees());

// Custom properties should also propagate
assertEquals("1", backport.properties().get("priority").asString());