From 7fc90932dcb356e1f4078f1e2c07bf7a4f27bc23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 5 Apr 2019 09:43:00 +0200 Subject: [PATCH] SONAR-11929 do not send issue notifications on PRs and short branches --- .../step/SendIssueNotificationsStep.java | 7 ++ .../step/SendIssueNotificationsStepTest.java | 64 ++++++++++++------- .../org/sonar/db/issue/IssueDbTester.java | 2 +- .../org/sonar/server/issue/IssueUpdater.java | 25 ++++++-- .../server/issue/ws/BulkChangeAction.java | 27 ++++++-- .../sonar/server/issue/IssueUpdaterTest.java | 53 ++++++++++++--- .../server/issue/ws/BulkChangeActionTest.java | 41 +++++++++++- .../issue/ws/DoTransitionActionTest.java | 2 +- .../issue/ws/SetSeverityActionTest.java | 2 +- .../server/issue/ws/SetTagsActionTest.java | 3 +- .../server/issue/ws/SetTypeActionTest.java | 2 +- 11 files changed, 177 insertions(+), 51 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java index 6bfe290e0dc7..0e98642046e4 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java @@ -53,6 +53,7 @@ import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.db.component.BranchType; import org.sonar.db.user.UserDto; import org.sonar.server.issue.notification.IssueChangeNotification; import org.sonar.server.issue.notification.MyNewIssuesNotification; @@ -67,6 +68,7 @@ import static java.util.stream.StreamSupport.stream; import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER; import static org.sonar.db.component.BranchType.PULL_REQUEST; +import static org.sonar.db.component.BranchType.SHORT; /** * Reads issues from disk cache and send related notifications. For performance reasons, @@ -103,6 +105,11 @@ public SendIssueNotificationsStep(IssueCache issueCache, RuleRepository rules, T @Override public void execute(ComputationStep.Context context) { + BranchType branchType = analysisMetadataHolder.getBranch().getType(); + if (branchType == PULL_REQUEST || branchType == SHORT) { + return; + } + Component project = treeRootHolder.getRoot(); NotificationStatistics notificationStatistics = new NotificationStatistics(); if (service.hasProjectSubscribersForTypes(analysisMetadataHolder.getProject().getUuid(), NOTIF_TYPES)) { diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java index 56161e1a6bab..22dc5ac1ede7 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java @@ -52,6 +52,7 @@ import org.sonar.ce.task.step.TestComputationStepContext; import org.sonar.core.issue.DefaultIssue; import org.sonar.db.DbTester; +import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.user.UserDto; @@ -81,11 +82,14 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.sonar.ce.task.projectanalysis.component.Component.Type; import static org.sonar.ce.task.projectanalysis.component.ReportComponent.builder; import static org.sonar.ce.task.projectanalysis.step.SendIssueNotificationsStep.NOTIF_TYPES; +import static org.sonar.db.component.BranchType.LONG; import static org.sonar.db.component.BranchType.PULL_REQUEST; +import static org.sonar.db.component.BranchType.SHORT; import static org.sonar.db.component.ComponentTesting.newBranchDto; import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.component.ComponentTesting.newPrivateProjectDto; @@ -232,14 +236,14 @@ public void do_not_send_global_new_issues_notification_if_issue_has_been_backdat } @Test - public void send_global_new_issues_notification_on_branch() { + public void send_global_new_issues_notification_on_long_branch() { ComponentDto project = newPrivateProjectDto(newOrganizationDto()); - ComponentDto branch = setUpBranch(project); + ComponentDto branch = setUpBranch(project, LONG); issueCache.newAppender().append( new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE))).close(); - when(notificationService.hasProjectSubscribersForTypes(project.uuid(), NOTIF_TYPES)).thenReturn(true); + when(notificationService.hasProjectSubscribersForTypes(branch.uuid(), NOTIF_TYPES)).thenReturn(true); analysisMetadataHolder.setProject(Project.from(project)); - analysisMetadataHolder.setBranch(newBranch()); + analysisMetadataHolder.setBranch(newBranch(BranchType.LONG)); TestComputationStepContext context = new TestComputationStepContext(); underTest.execute(context); @@ -253,9 +257,25 @@ public void send_global_new_issues_notification_on_branch() { } @Test - public void send_global_new_issues_notification_on_pull_request() { + public void do_not_send_global_new_issues_notification_on_short_branch() { + ComponentDto project = newPrivateProjectDto(newOrganizationDto()); + ComponentDto branch = setUpBranch(project, SHORT); + issueCache.newAppender().append( + new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE))).close(); + when(notificationService.hasProjectSubscribersForTypes(project.uuid(), NOTIF_TYPES)).thenReturn(true); + analysisMetadataHolder.setProject(Project.from(project)); + analysisMetadataHolder.setBranch(newBranch(SHORT)); + + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); + + verifyZeroInteractions(notificationService, newIssuesNotificationMock); + } + + @Test + public void do_not_send_global_new_issues_notification_on_pull_request() { ComponentDto project = newPrivateProjectDto(newOrganizationDto()); - ComponentDto branch = setUpBranch(project); + ComponentDto branch = setUpBranch(project, PULL_REQUEST); issueCache.newAppender().append( new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE))).close(); when(notificationService.hasProjectSubscribersForTypes(project.uuid(), NOTIF_TYPES)).thenReturn(true); @@ -266,23 +286,18 @@ public void send_global_new_issues_notification_on_pull_request() { TestComputationStepContext context = new TestComputationStepContext(); underTest.execute(context); - verify(notificationService).deliver(newIssuesNotificationMock); - verify(newIssuesNotificationMock).setProject(branch.getKey(), branch.longName(), null, PULL_REQUEST_ID); - verify(newIssuesNotificationMock).setAnalysisDate(new Date(ANALYSE_DATE)); - verify(newIssuesNotificationMock).setStatistics(eq(branch.longName()), any(NewIssuesStatistics.Stats.class)); - verify(newIssuesNotificationMock).setDebt(ISSUE_DURATION); - verifyStatistics(context, 1, 0, 0); + verifyZeroInteractions(notificationService, newIssuesNotificationMock); } @Test - public void do_not_send_global_new_issues_notification_on_branch_if_issue_has_been_backdated() { + public void do_not_send_global_new_issues_notification_on_long_branch_if_issue_has_been_backdated() { ComponentDto project = newPrivateProjectDto(newOrganizationDto()); - ComponentDto branch = setUpBranch(project); + ComponentDto branch = setUpBranch(project, LONG); issueCache.newAppender().append( new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS))).close(); when(notificationService.hasProjectSubscribersForTypes(branch.uuid(), NOTIF_TYPES)).thenReturn(true); analysisMetadataHolder.setProject(Project.from(project)); - analysisMetadataHolder.setBranch(newBranch()); + analysisMetadataHolder.setBranch(newBranch(BranchType.LONG)); TestComputationStepContext context = new TestComputationStepContext(); underTest.execute(context); @@ -535,16 +550,16 @@ private DefaultIssue prepareIssue(long issueCreatedAt, UserDto user, ComponentDt } @Test - public void send_issues_change_notification_on_branch() { - sendIssueChangeNotificationOnBranch(ANALYSE_DATE); + public void send_issues_change_notification_on_long_branch() { + sendIssueChangeNotificationOnLongBranch(ANALYSE_DATE); } @Test - public void send_issues_change_notification_on_branch_even_if_issue_is_backdated() { - sendIssueChangeNotificationOnBranch(ANALYSE_DATE - FIVE_MINUTES_IN_MS); + public void send_issues_change_notification_on_long_branch_even_if_issue_is_backdated() { + sendIssueChangeNotificationOnLongBranch(ANALYSE_DATE - FIVE_MINUTES_IN_MS); } - private void sendIssueChangeNotificationOnBranch(long issueCreatedAt) { + private void sendIssueChangeNotificationOnLongBranch(long issueCreatedAt) { ComponentDto project = newPrivateProjectDto(newOrganizationDto()); ComponentDto branch = newProjectBranch(project, newBranchDto(project).setKey(BRANCH_NAME)); ComponentDto file = newFileDto(branch); @@ -561,7 +576,7 @@ private void sendIssueChangeNotificationOnBranch(long issueCreatedAt) { ruleRepository.add(ruleDefinitionDto.getKey()).setName(ruleDefinitionDto.getName()); issueCache.newAppender().append(issue).close(); when(notificationService.hasProjectSubscribersForTypes(project.uuid(), NOTIF_TYPES)).thenReturn(true); - analysisMetadataHolder.setBranch(newBranch()); + analysisMetadataHolder.setBranch(newBranch(BranchType.LONG)); underTest.execute(new TestComputationStepContext()); @@ -623,10 +638,11 @@ private MyNewIssuesNotification createMyNewIssuesNotificationMock() { return notification; } - private static Branch newBranch() { + private static Branch newBranch(BranchType type) { Branch branch = mock(Branch.class); when(branch.isMain()).thenReturn(false); when(branch.getName()).thenReturn(BRANCH_NAME); + when(branch.getType()).thenReturn(type); return branch; } @@ -639,8 +655,8 @@ private static Branch newPullRequest() { return branch; } - private ComponentDto setUpBranch(ComponentDto project) { - ComponentDto branch = newProjectBranch(project, newBranchDto(project).setKey(BRANCH_NAME)); + private ComponentDto setUpBranch(ComponentDto project, BranchType branchType) { + ComponentDto branch = newProjectBranch(project, newBranchDto(project, branchType).setKey(BRANCH_NAME)); ComponentDto file = newFileDto(branch); treeRootHolder.setRoot(builder(Type.PROJECT, 2).setKey(branch.getDbKey()).setPublicKey(branch.getKey()).setName(branch.longName()).setUuid(branch.uuid()).addChildren( builder(Type.FILE, 11).setKey(file.getDbKey()).setPublicKey(file.getKey()).setName(file.longName()).build()).build()); diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDbTester.java b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDbTester.java index 31359985a76b..a7c5c0c73555 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDbTester.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDbTester.java @@ -63,7 +63,7 @@ public final IssueDto insertIssue(Consumer... populateIssueDto) { @SafeVarargs public final IssueDto insertIssue(OrganizationDto organizationDto, Consumer... populators) { RuleDefinitionDto rule = db.rules().insert(); - ComponentDto project = db.components().insertPrivateProject(organizationDto); + ComponentDto project = db.components().insertMainBranch(organizationDto); ComponentDto file = db.components().insertComponent(newFileDto(project)); IssueDto issue = newIssue(rule, project, file); stream(populators).forEach(p -> p.accept(issue)); diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java index a8019018120e..4146385e9c43 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java @@ -30,6 +30,7 @@ import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.db.component.BranchDto; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueDto; import org.sonar.db.rule.RuleDefinitionDto; @@ -41,6 +42,8 @@ import static com.google.common.base.Preconditions.checkState; import static java.util.Collections.singleton; import static java.util.Collections.singletonList; +import static org.sonar.db.component.BranchType.PULL_REQUEST; +import static org.sonar.db.component.BranchType.SHORT; public class IssueUpdater { @@ -66,8 +69,9 @@ public SearchResponseData saveIssueAndPreloadSearchResponseData(DbSession dbSess Optional rule = getRuleByKey(dbSession, issue.getRuleKey()); ComponentDto project = dbClient.componentDao().selectOrFailByUuid(dbSession, issue.projectUuid()); + BranchDto branch = getBranch(dbSession, issue, issue.projectUuid()); ComponentDto component = getComponent(dbSession, issue, issue.componentUuid()); - IssueDto issueDto = doSaveIssue(dbSession, issue, context, comment, rule, project, component); + IssueDto issueDto = doSaveIssue(dbSession, issue, context, comment, rule, project, branch, component); SearchResponseData result = new SearchResponseData(issueDto); rule.ifPresent(r -> result.addRules(singletonList(r))); @@ -85,14 +89,15 @@ public SearchResponseData saveIssueAndPreloadSearchResponseData(DbSession dbSess public IssueDto saveIssue(DbSession session, DefaultIssue issue, IssueChangeContext context, @Nullable String comment) { Optional rule = getRuleByKey(session, issue.getRuleKey()); ComponentDto project = getComponent(session, issue, issue.projectUuid()); + BranchDto branch = getBranch(session, issue, issue.projectUuid()); ComponentDto component = getComponent(session, issue, issue.componentUuid()); - return doSaveIssue(session, issue, context, comment, rule, project, component); + return doSaveIssue(session, issue, context, comment, rule, project, branch, component); } private IssueDto doSaveIssue(DbSession session, DefaultIssue issue, IssueChangeContext context, @Nullable String comment, - Optional rule, ComponentDto project, ComponentDto component) { + Optional rule, ComponentDto project, BranchDto branch, ComponentDto component) { IssueDto issueDto = issueStorage.save(session, singletonList(issue)).iterator().next(); - if (issue.type() != RuleType.SECURITY_HOTSPOT) { + if (issue.type() != RuleType.SECURITY_HOTSPOT && hasNotificationSupport(branch)) { String assigneeUuid = issue.assignee(); UserDto assignee = assigneeUuid == null ? null : dbClient.userDao().selectByUuid(session, assigneeUuid); String authorUuid = context.userUuid(); @@ -109,6 +114,10 @@ private IssueDto doSaveIssue(DbSession session, DefaultIssue issue, IssueChangeC return issueDto; } + private static boolean hasNotificationSupport(BranchDto branch) { + return branch.getBranchType() != PULL_REQUEST && branch.getBranchType() != SHORT; + } + private ComponentDto getComponent(DbSession dbSession, DefaultIssue issue, @Nullable String componentUuid) { String issueKey = issue.key(); checkState(componentUuid != null, "Issue '%s' has no component", issueKey); @@ -117,6 +126,14 @@ private ComponentDto getComponent(DbSession dbSession, DefaultIssue issue, @Null return component; } + private BranchDto getBranch(DbSession dbSession, DefaultIssue issue, @Nullable String projectUuid) { + String issueKey = issue.key(); + checkState(projectUuid != null, "Issue '%s' has no project", issueKey); + BranchDto component = dbClient.branchDao().selectByUuid(dbSession, projectUuid).orElse(null); + checkState(component != null, "Branch uuid '%s' for issue key '%s' cannot be found", projectUuid, issueKey); + return component; + } + private Optional getRuleByKey(DbSession session, RuleKey ruleKey) { Optional rule = dbClient.ruleDao().selectDefinitionByKey(session, ruleKey); return (rule.isPresent() && rule.get().getStatus() != RuleStatus.REMOVED) ? rule : Optional.empty(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java index d3fba0d5bc23..4f49fb5abe5f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java @@ -30,6 +30,7 @@ import java.util.Set; import java.util.function.Consumer; import java.util.function.Predicate; +import javax.annotation.Nullable; import org.sonar.api.issue.DefaultTransitions; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; @@ -47,6 +48,8 @@ import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.db.component.BranchDto; +import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueDto; import org.sonar.db.rule.RuleDefinitionDto; @@ -253,17 +256,24 @@ private static void addCommentIfNeeded(ActionContext actionContext, BulkChangeDa private Consumer sendNotification(BulkChangeData bulkChangeData, Map userDtoByUuid, UserDto author) { return issue -> { if (bulkChangeData.sendNotification && issue.type() != RuleType.SECURITY_HOTSPOT) { - notificationService.scheduleForSending(new IssueChangeNotification() - .setIssue(issue) - .setAssignee(userDtoByUuid.get(issue.assignee())) - .setChangeAuthor(author) - .setRuleName(bulkChangeData.rulesByKey.get(issue.ruleKey()).getName()) - .setProject(bulkChangeData.projectsByUuid.get(issue.projectUuid())) - .setComponent(bulkChangeData.componentsByUuid.get(issue.componentUuid()))); + BranchDto branch = bulkChangeData.branchesByProjectUuid.get(issue.projectUuid()); + if (hasNotificationSupport(branch)) { + notificationService.scheduleForSending(new IssueChangeNotification() + .setIssue(issue) + .setAssignee(userDtoByUuid.get(issue.assignee())) + .setChangeAuthor(author) + .setRuleName(bulkChangeData.rulesByKey.get(issue.ruleKey()).getName()) + .setProject(bulkChangeData.projectsByUuid.get(issue.projectUuid())) + .setComponent(bulkChangeData.componentsByUuid.get(issue.componentUuid()))); + } } }; } + private static boolean hasNotificationSupport(@Nullable BranchDto branch) { + return branch != null && branch.getBranchType() != BranchType.PULL_REQUEST && branch.getBranchType() != BranchType.SHORT; + } + private static Issues.BulkChangeWsResponse toWsResponse(BulkChangeResult result) { return Issues.BulkChangeWsResponse.newBuilder() .setTotal(result.countTotal()) @@ -305,6 +315,7 @@ private class BulkChangeData { private final boolean sendNotification; private final Collection issues; private final Map projectsByUuid; + private final Map branchesByProjectUuid; private final Map componentsByUuid; private final Map rulesByKey; private final List availableActions; @@ -319,6 +330,8 @@ private class BulkChangeData { List allProjects = getComponents(dbSession, allIssues.stream().map(IssueDto::getProjectUuid).collect(MoreCollectors.toSet())); this.projectsByUuid = getAuthorizedProjects(allProjects).stream().collect(uniqueIndex(ComponentDto::uuid, identity())); + this.branchesByProjectUuid = dbClient.branchDao().selectByUuids(dbSession, projectsByUuid.keySet()).stream() + .collect(uniqueIndex(BranchDto::getUuid, identity())); this.issues = getAuthorizedIssues(allIssues); this.componentsByUuid = getComponents(dbSession, issues.stream().map(DefaultIssue::componentUuid).collect(MoreCollectors.toSet())).stream() diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java index ba8d7cec6694..e7b02a808d2a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java @@ -31,6 +31,7 @@ import org.sonar.core.issue.IssueChangeContext; import org.sonar.db.DbClient; import org.sonar.db.DbTester; +import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueDto; import org.sonar.db.issue.IssueTesting; @@ -53,8 +54,10 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.sonar.api.rule.Severity.BLOCKER; import static org.sonar.api.rule.Severity.MAJOR; +import static org.sonar.db.component.BranchType.LONG; import static org.sonar.db.component.ComponentTesting.newFileDto; public class IssueUpdaterTest { @@ -98,7 +101,7 @@ public void update_issue() { public void verify_notification() { UserDto assignee = db.users().insertUser(); RuleDto rule = db.rules().insertRule(); - ComponentDto project = db.components().insertPrivateProject(); + ComponentDto project = db.components().insertMainBranch(); ComponentDto file = db.components().insertComponent(newFileDto(project)); RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)]; DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file) @@ -131,7 +134,7 @@ public void verify_notification() { public void verify_no_notification_on_hotspot() { UserDto assignee = db.users().insertUser(); RuleDto rule = db.rules().insertRule(); - ComponentDto project = db.components().insertPrivateProject(); + ComponentDto project = db.components().insertMainBranch(); ComponentDto file = db.components().insertComponent(newFileDto(project)); DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file) .setType(RuleType.SECURITY_HOTSPOT)) @@ -148,10 +151,10 @@ public void verify_no_notification_on_hotspot() { } @Test - public void verify_notification_on_branch() { + public void verify_notification_on_long_branch() { RuleDto rule = db.rules().insertRule(); ComponentDto project = db.components().insertMainBranch(); - ComponentDto branch = db.components().insertProjectBranch(project); + ComponentDto branch = db.components().insertProjectBranch(project, t -> t.setBranchType(LONG)); ComponentDto file = db.components().insertComponent(newFileDto(branch)); RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)]; DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), branch, file) @@ -169,10 +172,44 @@ public void verify_notification_on_branch() { assertThat(issueChangeNotification.getFieldValue("branch")).isEqualTo(branch.getBranch()); } + @Test + public void verify_no_notification_on_short_branch() { + RuleDto rule = db.rules().insertRule(); + ComponentDto project = db.components().insertMainBranch(); + ComponentDto branch = db.components().insertProjectBranch(project, t -> t.setBranchType(BranchType.SHORT)); + ComponentDto file = db.components().insertComponent(newFileDto(branch)); + RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)]; + DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), branch, file) + .setType(randomTypeExceptHotspot)).setSeverity(MAJOR).toDefaultIssue(); + IssueChangeContext context = IssueChangeContext.createUser(new Date(), "user_uuid"); + issueFieldsSetter.setSeverity(issue, BLOCKER, context); + + underTest.saveIssue(db.getSession(), issue, context, "increase severity"); + + verifyZeroInteractions(notificationManager); + } + + @Test + public void verify_no_notification_on_pr() { + RuleDto rule = db.rules().insertRule(); + ComponentDto project = db.components().insertMainBranch(); + ComponentDto branch = db.components().insertProjectBranch(project, t -> t.setBranchType(BranchType.PULL_REQUEST)); + ComponentDto file = db.components().insertComponent(newFileDto(branch)); + RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)]; + DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), branch, file) + .setType(randomTypeExceptHotspot)).setSeverity(MAJOR).toDefaultIssue(); + IssueChangeContext context = IssueChangeContext.createUser(new Date(), "user_uuid"); + issueFieldsSetter.setSeverity(issue, BLOCKER, context); + + underTest.saveIssue(db.getSession(), issue, context, "increase severity"); + + verifyZeroInteractions(notificationManager); + } + @Test public void verify_notification_when_issue_is_linked_on_removed_rule() { RuleDto rule = db.rules().insertRule(r -> r.setStatus(RuleStatus.REMOVED)); - ComponentDto project = db.components().insertPrivateProject(); + ComponentDto project = db.components().insertMainBranch(); ComponentDto file = db.components().insertComponent(newFileDto(project)); RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)]; DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file) @@ -190,7 +227,7 @@ public void verify_notification_when_issue_is_linked_on_removed_rule() { public void verify_notification_when_assignee_has_changed() { UserDto oldAssignee = db.users().insertUser(); RuleDto rule = db.rules().insertRule(); - ComponentDto project = db.components().insertPrivateProject(); + ComponentDto project = db.components().insertMainBranch(); ComponentDto file = db.components().insertComponent(newFileDto(project)); RuleType randomTypeExceptHotspot = RuleType.values()[nextInt(RuleType.values().length - 1)]; DefaultIssue issue = db.issues().insertIssue(IssueTesting.newIssue(rule.getDefinition(), project, file) @@ -215,7 +252,7 @@ public void verify_notification_when_assignee_has_changed() { @Test public void saveIssue_populates_specified_SearchResponseData_with_rule_project_and_component_retrieved_from_DB() { RuleDto rule = db.rules().insertRule(); - ComponentDto project = db.components().insertPrivateProject(); + ComponentDto project = db.components().insertMainBranch(); ComponentDto file = db.components().insertComponent(newFileDto(project)); IssueDto issueDto = IssueTesting.newIssue(rule.getDefinition(), project, file); DefaultIssue issue = db.issues().insertIssue(issueDto).setSeverity(MAJOR).toDefaultIssue(); @@ -240,7 +277,7 @@ public void saveIssue_populates_specified_SearchResponseData_with_rule_project_a @Test public void saveIssue_populates_specified_SearchResponseData_with_no_rule_but_with_project_and_component_if_rule_is_removed() { RuleDto rule = db.rules().insertRule(r -> r.setStatus(RuleStatus.REMOVED)); - ComponentDto project = db.components().insertPrivateProject(); + ComponentDto project = db.components().insertMainBranch(); ComponentDto file = db.components().insertComponent(newFileDto(project)); IssueDto issueDto = IssueTesting.newIssue(rule.getDefinition(), project, file); DefaultIssue issue = db.issues().insertIssue(issueDto).setSeverity(MAJOR).toDefaultIssue(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java index 4af1280a2040..2ac7faf5bac1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java @@ -36,6 +36,7 @@ import org.sonar.api.utils.internal.TestSystem2; import org.sonar.db.DbClient; import org.sonar.db.DbTester; +import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueChangeDto; import org.sonar.db.issue.IssueDto; @@ -73,6 +74,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.issue.Issue.STATUS_CLOSED; import static org.sonar.api.issue.Issue.STATUS_OPEN; @@ -284,7 +286,7 @@ public void bulk_change_many_issues() { public void send_notification() { UserDto user = db.users().insertUser(); userSession.logIn(user); - ComponentDto project = db.components().insertPrivateProject(); + ComponentDto project = db.components().insertMainBranch(); ComponentDto file = db.components().insertComponent(newFileDto(project)); addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); RuleDefinitionDto rule = db.rules().insert(); @@ -331,11 +333,11 @@ public void hotspots_are_ignored_and_no_notification_is_sent() { } @Test - public void send_notification_on_branch() { + public void send_notification_on_long_branch() { UserDto user = db.users().insertUser(); userSession.logIn(user); ComponentDto project = db.components().insertMainBranch(); - ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey("feature")); + ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey("feature").setBranchType(BranchType.LONG)); ComponentDto fileOnBranch = db.components().insertComponent(newFileDto(branch)); addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); RuleDefinitionDto rule = db.rules().insert(); @@ -361,6 +363,39 @@ public void send_notification_on_branch() { verifyPostProcessorCalled(fileOnBranch); } + @Test + public void send_notification_on_short_branch() { + BranchType branchType = BranchType.SHORT; + verifySendNoNotification(branchType); + } + + @Test + public void send_no_notification_on_PR() { + verifySendNoNotification(BranchType.PULL_REQUEST); + } + + private void verifySendNoNotification(BranchType branchType) { + UserDto user = db.users().insertUser(); + userSession.logIn(user); + ComponentDto project = db.components().insertMainBranch(); + ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey("feature").setBranchType(branchType)); + ComponentDto fileOnBranch = db.components().insertComponent(newFileDto(branch)); + addUserProjectPermissions(user, project, USER, ISSUE_ADMIN); + RuleDefinitionDto rule = db.rules().insert(); + IssueDto issue = db.issues().insert(rule, branch, fileOnBranch, i -> i.setType(BUG) + .setStatus(STATUS_OPEN).setResolution(null)); + + BulkChangeWsResponse response = call(builder() + .setIssues(singletonList(issue.getKey())) + .setDoTransition("confirm") + .setSendNotifications(true) + .build()); + + checkResponse(response, 1, 1, 0, 0); + verifyZeroInteractions(notificationManager); + verifyPostProcessorCalled(fileOnBranch); + } + @Test public void send_notification_only_on_changed_issues() { UserDto user = db.users().insertUser(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java index 1a7bc48687f2..2ed99bd9a8b8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java @@ -111,7 +111,7 @@ public void setUp() throws Exception { @Test public void do_transition() { - ComponentDto project = db.components().insertPrivateProject(); + ComponentDto project = db.components().insertMainBranch(); ComponentDto file = db.components().insertComponent(newFileDto(project)); RuleDefinitionDto rule = db.rules().insert(); IssueDto issue = db.issues().insert(rule, project, file, i -> i.setStatus(STATUS_OPEN).setResolution(null)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetSeverityActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetSeverityActionTest.java index a51524105b63..94a489526e60 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetSeverityActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetSeverityActionTest.java @@ -181,7 +181,7 @@ private TestResponse call(@Nullable String issueKey, @Nullable String severity) private IssueDto newIssue() { RuleDto rule = dbTester.rules().insertRule(newRuleDto()); - ComponentDto project = dbTester.components().insertPrivateProject(); + ComponentDto project = dbTester.components().insertMainBranch(); ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); return newDto(rule, file, project); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTagsActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTagsActionTest.java index de1ba1302181..bff4568b616f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTagsActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTagsActionTest.java @@ -68,6 +68,7 @@ import static org.sonar.api.web.UserRole.ISSUE_ADMIN; import static org.sonar.core.util.stream.MoreCollectors.join; import static org.sonar.db.component.ComponentTesting.newFileDto; +import static org.sonar.db.component.ComponentTesting.newPublicProjectDto; public class SetTagsActionTest { @@ -236,7 +237,7 @@ private TestResponse call(@Nullable String issueKey, String... tags) { private IssueDto newIssue() { RuleDefinitionDto rule = db.rules().insert(); - ComponentDto project = db.components().insertPublicProject(); + ComponentDto project = db.components().insertMainBranch(newPublicProjectDto(db.getDefaultOrganization())); ComponentDto file = db.components().insertComponent(newFileDto(project)); return IssueTesting.newIssue(rule, project, file); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java index 989df80eecc1..700f241ef0cd 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java @@ -200,7 +200,7 @@ private TestResponse call(@Nullable String issueKey, @Nullable String type) { private IssueDto newIssue() { RuleDto rule = dbTester.rules().insertRule(newRuleDto()); - ComponentDto project = dbTester.components().insertPrivateProject(); + ComponentDto project = dbTester.components().insertMainBranch(); ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); return newDto(rule, file, project); }