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.

Sign up for GitHub

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

1364: Jira Issues should be resolved as "Fixed" #1382

Closed
wants to merge 4 commits into from
+26 −22
Closed

1364: Jira Issues should be resolved as "Fixed" #1382

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4fa4aaa
Jira Issues should be resolved as Fixed
zhaosongzs Sep 23, 2022
8c82437
Check the resolution of JiraIssue before setting the state to Resolved
zhaosongzs Sep 23, 2022
dba67f8
remove some conditions
zhaosongzs Sep 23, 2022
84874a6
Adjusted the format
zhaosongzs Sep 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 25 additions & 21 deletions bots/notify/src/main/java/org/openjdk/skara/bots/notify/issue/IssueNotifier.java
View file
Original file line number Diff line number Diff line change
@@ -192,15 +192,17 @@ public void onIntegratedPullRequest(PullRequest pr, Path scratchPath, Hash hash)

// If prOnly is false, this is instead done when processing commits
if (prOnly && resolve) {
if (issue.state() == Issue.State.OPEN) {
log.info("Resolving issue " + issue.id());
log.info("Resolving issue " + issue.id() + " from state " + issue.state());
Copy link
Member

Choose a reason for hiding this comment

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

We should keep the log message and the issue.setState call together. I think moving lines 203-207 up here would be best to keep the flow as similar to before as we can. Same applies in the other method.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure

if (!issue.isFixed()) {
issue.setState(Issue.State.RESOLVED);
if (issue.assignees().isEmpty()) {
var username = findIssueUsername(commit, scratchPath);
if (username.isPresent()) {
var assignee = issueProject.issueTracker().user(username.get());
assignee.ifPresent(hostUser -> issue.setAssignees(List.of(hostUser)));
}
} else {
log.info("The issue was already resolved");
}
if (issue.assignees().isEmpty()) {
var username = findIssueUsername(commit, scratchPath);
if (username.isPresent()) {
var assignee = issueProject.issueTracker().user(username.get());
assignee.ifPresent(hostUser -> issue.setAssignees(List.of(hostUser)));
}
}
}
@@ -311,20 +313,22 @@ public void onNewCommits(HostedRepository repository, Repository localRepository
if (!alreadyPostedComment) {
issue.addComment(commitNotification);
}
if (issue.state() == Issue.State.OPEN) {
log.info("Resolving issue " + issue.id());
log.info("Resolving issue " + issue.id() + " from state " + issue.state());
if (!issue.isFixed()) {
issue.setState(Issue.State.RESOLVED);
var assignees = issue.assignees();
// Due to a bug in the backport plugin, certain users can't be assigned directly.
// Work around this by overwriting the assignee afterwards if the current assignee
// is the bot user.
if (assignees.isEmpty() || (assignees.size() == 1 && assignees.get(0).equals(issueProject.issueTracker().currentUser()))) {
if (username.isPresent()) {
var assignee = issueProject.issueTracker().user(username.get());
if (assignee.isPresent()) {
log.info("Setting assignee for issue " + issue.id() + " to " + assignee.get());
issue.setAssignees(List.of(assignee.get()));
}
} else {
log.info("The issue was already resolved");
}
var assignees = issue.assignees();
// Due to a bug in the backport plugin, certain users can't be assigned directly.
// Work around this by overwriting the assignee afterwards if the current assignee
// is the bot user.
if (assignees.isEmpty() || (assignees.size() == 1 && assignees.get(0).equals(issueProject.issueTracker().currentUser()))) {
if (username.isPresent()) {
var assignee = issueProject.issueTracker().user(username.get());
if (assignee.isPresent()) {
log.info("Setting assignee for issue " + issue.id() + " to " + assignee.get());
issue.setAssignees(List.of(assignee.get()));
}
}
}
2 changes: 1 addition & 1 deletion bots/notify/src/test/java/org/openjdk/skara/bots/notify/issue/IssueNotifierTests.java
View file
Original file line number Diff line number Diff line change
@@ -1778,7 +1778,7 @@ void testAltFixVersionsMatch(TestInfo testInfo) throws IOException {
var updatedIssue = issueProject.issue(issue.id()).orElseThrow();
assertEquals(Set.of("18"), fixVersions(updatedIssue));
assertEquals(RESOLVED, updatedIssue.state());
assertEquals(List.of(), updatedIssue.assignees());
assertEquals(List.of(issueProject.issueTracker().currentUser()), updatedIssue.assignees());
// A commit comment should have been added
List<Comment> comments = updatedIssue.comments();
assertEquals(1, comments.size());