From: Jacek Date: Fri, 13 Dec 2019 10:20:52 +0000 (+0100) Subject: SONAR-12745 Adjust `changes on my issue` email notification for Security Hotspots X-Git-Tag: 8.2.0.32929~196 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=ec1f6fbe740e121bbf14d4138663e353144d54a2;p=sonarqube.git SONAR-12745 Adjust `changes on my issue` email notification for Security Hotspots --- diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/NotificationFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/NotificationFactory.java index a8af2f653ab..c66877890cb 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/NotificationFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/NotificationFactory.java @@ -111,7 +111,7 @@ public class NotificationFactory { private IssuesChangesNotificationBuilder.Rule getRuleByRuleKey(RuleKey ruleKey) { return ruleRepository.findByKey(ruleKey) - .map(t -> new IssuesChangesNotificationBuilder.Rule(ruleKey, t.getName())) + .map(t -> new IssuesChangesNotificationBuilder.Rule(ruleKey, t.getType(), t.getName())) .orElseThrow(() -> new IllegalStateException("Can not find rule " + ruleKey + " in RuleRepository")); } 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 ab65a1138f1..2c34348cc4f 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 @@ -92,6 +92,7 @@ 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.api.rules.RuleType.SECURITY_HOTSPOT; 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; @@ -133,7 +134,7 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { public DbTester db = DbTester.create(System2.INSTANCE); private final Random random = new Random(); - private final RuleType[] RULE_TYPES_EXCEPT_HOTSPOTS = Stream.of(RuleType.values()).filter(r -> r != RuleType.SECURITY_HOTSPOT).toArray(RuleType[]::new); + private final RuleType[] RULE_TYPES_EXCEPT_HOTSPOTS = Stream.of(RuleType.values()).filter(r -> r != SECURITY_HOTSPOT).toArray(RuleType[]::new); private final RuleType randomRuleType = RULE_TYPES_EXCEPT_HOTSPOTS[random.nextInt(RULE_TYPES_EXCEPT_HOTSPOTS.length)]; @SuppressWarnings("unchecked") private Class> assigneeCacheType = (Class>) (Object) Map.class; @@ -460,7 +461,7 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { .isEmpty(); Tuple[] expected = stream(users).map(user -> tuple(user.getUuid(), user.getUuid(), user.getId(), user.getLogin())).toArray(Tuple[]::new); assertThat(cache.entrySet()) - .extracting(t -> t.getKey(), t -> t.getValue().getUuid(), t -> t.getValue().getId(), t -> t.getValue().getLogin()) + .extracting(Map.Entry::getKey, t -> t.getValue().getUuid(), t -> t.getValue().getId(), t -> t.getValue().getLogin()) .containsOnly(expected); } @@ -488,7 +489,7 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { } @Test - public void dont_send_issues_change_notification_for_hotspot() { + public void do_not_send_new_issues_notifications_for_hotspot() { UserDto user = db.users().insertUser(); ComponentDto project = newPrivateProjectDto(newOrganizationDto()).setDbKey(PROJECT.getDbKey()).setLongName(PROJECT.getName()); ComponentDto file = newFileDto(project).setDbKey(FILE.getDbKey()).setLongName(FILE.getName()); diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplate.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplate.java index a1424caa0b8..812551a41cc 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplate.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplate.java @@ -21,7 +21,6 @@ package org.sonar.server.issue.notification; import com.google.common.collect.ListMultimap; import com.google.common.collect.SetMultimap; -import java.util.Collection; import java.util.List; import javax.annotation.CheckForNull; import org.sonar.api.config.EmailSettings; @@ -34,9 +33,13 @@ import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.User import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.UserChange; import static com.google.common.base.Preconditions.checkState; +import static java.util.function.Function.identity; import static org.sonar.api.issue.Issue.STATUS_CLOSED; import static org.sonar.api.issue.Issue.STATUS_OPEN; import static org.sonar.core.util.stream.MoreCollectors.index; +import static org.sonar.server.issue.notification.RuleGroup.ISSUES; +import static org.sonar.server.issue.notification.RuleGroup.SECURITY_HOTSPOTS; +import static org.sonar.server.issue.notification.RuleGroup.resolveGroup; /** * Creates email message for notification "Changes on my issues". @@ -109,33 +112,28 @@ public class ChangesOnMyIssuesEmailTemplate extends IssueChangesEmailTemplate { return new EmailMessage() .setFrom(user.getName().orElse(user.getLogin())) .setMessageId("changes-on-my-issues") - .setSubject("A manual update has changed some of your issues") + .setSubject("A manual update has changed some of your issues/hotspots") .setHtmlMessage(buildMultiProjectMessage(notification)); } private String buildMultiProjectMessage(ChangesOnMyIssuesNotification notification) { + ListMultimap issuesAndHotspots = notification.getChangedIssues().values().stream() + .collect(index(changedIssue -> resolveGroup(changedIssue.getRule().getRuleType()), identity())); + + List issues = issuesAndHotspots.get(ISSUES); + List hotspots = issuesAndHotspots.get(SECURITY_HOTSPOTS); + StringBuilder sb = new StringBuilder(); paragraph(sb, s -> s.append("Hi,")); - paragraph(sb, s -> { - SetMultimap changedIssues = notification.getChangedIssues(); - s.append("A manual change has updated ").append(issuesOrAnIssue(changedIssues)) - .append(" assigned to you:"); - }); + paragraph(sb, s -> s.append("A manual change has updated ").append(RuleGroup.formatIssuesOrHotspots(issues, hotspots)).append(" assigned to you:")); - addIssuesByProjectThenRule(sb, notification.getChangedIssues()); + addIssuesAndHotspotsByProjectThenRule(sb, notification.getChangedIssues()); addFooter(sb, NOTIFICATION_NAME_I18N_KEY); return sb.toString(); } - private static String issueOrIssues(Collection collection) { - if (collection.size() > 1) { - return "issues"; - } - return "issue"; - } - private static String issuesOrAnIssue(SetMultimap changedIssues) { if (changedIssues.size() > 1) { return "issues"; diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssueChangesEmailTemplate.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssueChangesEmailTemplate.java index 4774ba5663f..53d4f8650e5 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssueChangesEmailTemplate.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssueChangesEmailTemplate.java @@ -31,10 +31,13 @@ import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Optional; +import java.util.Set; import java.util.SortedSet; import java.util.function.BiConsumer; import java.util.function.Consumer; +import javax.annotation.Nullable; import org.sonar.api.config.EmailSettings; +import org.sonar.api.rules.RuleType; import org.sonar.core.i18n.I18n; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.ChangedIssue; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.Project; @@ -42,7 +45,13 @@ import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.Rule import static java.net.URLEncoder.encode; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.function.Function.identity; import static org.sonar.core.util.stream.MoreCollectors.index; +import static org.sonar.server.issue.notification.RuleGroup.ISSUES; +import static org.sonar.server.issue.notification.RuleGroup.SECURITY_HOTSPOTS; +import static org.sonar.server.issue.notification.RuleGroup.formatIssueOrHotspot; +import static org.sonar.server.issue.notification.RuleGroup.formatIssuesOrHotspots; +import static org.sonar.server.issue.notification.RuleGroup.resolveGroup; public abstract class IssueChangesEmailTemplate implements EmailTemplate { @@ -50,6 +59,7 @@ public abstract class IssueChangesEmailTemplate implements EmailTemplate { private static final Comparator PROJECT_COMPARATOR = Comparator.comparing(Project::getProjectName) .thenComparing(t -> t.getBranchName().orElse("")); private static final Comparator CHANGED_ISSUE_KEY_COMPARATOR = Comparator.comparing(ChangedIssue::getKey, Comparator.naturalOrder()); + /** * Assuming: *
    @@ -97,6 +107,38 @@ public abstract class IssueChangesEmailTemplate implements EmailTemplate { }); } + void addIssuesAndHotspotsByProjectThenRule(StringBuilder sb, SetMultimap issuesByProject) { + issuesByProject.keySet().stream() + .sorted(PROJECT_COMPARATOR) + .forEach(project -> { + String encodedProjectParams = toUrlParams(project); + paragraph(sb, s -> toString(s, project)); + + Set changedIssues = issuesByProject.get(project); + ListMultimap issuesAndHotspots = changedIssues.stream() + .collect(index(changedIssue -> resolveGroup(changedIssue.getRule().getRuleType()), identity())); + + List issues = issuesAndHotspots.get(ISSUES); + List hotspots = issuesAndHotspots.get(SECURITY_HOTSPOTS); + + boolean hasSecurityHotspots = !hotspots.isEmpty(); + boolean hasOtherIssues = !issues.isEmpty(); + + if (hasOtherIssues) { + addIssuesByRule(sb, issues, projectIssuePageHref(encodedProjectParams)); + } + + if (hasSecurityHotspots && hasOtherIssues) { + paragraph(sb, stringBuilder -> { + }); + } + + if (hasSecurityHotspots) { + addIssuesByRule(sb, hotspots, securityHotspotPageHref(encodedProjectParams)); + } + }); + } + void addIssuesByRule(StringBuilder sb, Collection changedIssues, BiConsumer> issuePageHref) { ListMultimap issuesByRule = changedIssues.stream() .collect(index(ChangedIssue::getRule, t -> t)); @@ -114,21 +156,22 @@ public abstract class IssueChangesEmailTemplate implements EmailTemplate { Collection issues = issuesByRule.get(rule); sb.append("
  • ").append("Rule ").append(" ").append(rule.getName()).append(" - "); - appendIssueLinks(sb, issuePageHref, issues); + appendIssueLinks(sb, issuePageHref, issues, rule.getRuleType()); sb.append("
    • "); } sb.append("
"); } - private static void appendIssueLinks(StringBuilder sb, BiConsumer> issuePageHref, Collection issues) { + private static void appendIssueLinks(StringBuilder sb, BiConsumer> issuePageHref, Collection issues, + @Nullable RuleType ruleType) { SortedSet sortedIssues = ImmutableSortedSet.copyOf(CHANGED_ISSUE_KEY_COMPARATOR, issues); int issueCount = issues.size(); if (issueCount == 1) { - link(sb, s -> issuePageHref.accept(s, sortedIssues), s -> s.append("See the single issue")); + link(sb, s -> issuePageHref.accept(s, sortedIssues), s -> s.append("See the single ").append(formatIssueOrHotspot(ruleType))); } else if (issueCount <= MAX_ISSUES_BY_LINK) { - link(sb, s -> issuePageHref.accept(s, sortedIssues), s -> s.append("See all ").append(issueCount).append(" issues")); + link(sb, s -> issuePageHref.accept(s, sortedIssues), s -> s.append("See all ").append(issueCount).append(" ").append(formatIssuesOrHotspots(ruleType))); } else { - sb.append("See issues"); + sb.append("See ").append(formatIssuesOrHotspots(ruleType)); List> issueGroups = Lists.partition(ImmutableList.copyOf(sortedIssues), MAX_ISSUES_BY_LINK); Iterator> issueGroupsIterator = issueGroups.iterator(); int[] groupIndex = new int[] {0}; @@ -160,6 +203,21 @@ public abstract class IssueChangesEmailTemplate implements EmailTemplate { }; } + BiConsumer> securityHotspotPageHref(String projectParams) { + return (s, issues) -> { + s.append(settings.getServerBaseURL()).append("/security_hotspots?").append(projectParams) + .append("&hotspots="); + + Iterator issueIterator = issues.iterator(); + while (issueIterator.hasNext()) { + s.append(urlEncode(issueIterator.next().getKey())); + if (issueIterator.hasNext()) { + s.append(URL_ENCODED_COMMA); + } + } + }; + } + private static Consumer issueGroupLabel(StringBuilder sb, int[] groupIndex, List issueGroup) { return s -> { int firstIssueNumber = (groupIndex[0] * MAX_ISSUES_BY_LINK) + 1; @@ -207,4 +265,11 @@ public abstract class IssueChangesEmailTemplate implements EmailTemplate { } } + protected static String issueOrIssues(Collection collection) { + if (collection.size() > 1) { + return "issues"; + } + return "issue"; + } + } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssuesChangesNotificationBuilder.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssuesChangesNotificationBuilder.java index c1508db355c..5779d4989b3 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssuesChangesNotificationBuilder.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssuesChangesNotificationBuilder.java @@ -27,6 +27,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.RuleType; import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; @@ -233,10 +234,16 @@ public class IssuesChangesNotificationBuilder { @Immutable public static final class Rule { private final RuleKey key; + private final RuleType ruleType; private final String name; - public Rule(RuleKey key, String name) { + public Rule(RuleKey key, @Nullable String ruleType, String name) { + this(key, ruleType != null ? RuleType.valueOf(ruleType) : null, name); + } + + public Rule(RuleKey key, @Nullable RuleType ruleType, String name) { this.key = requireNonNull(key, KEY_CANT_BE_NULL_MESSAGE); + this.ruleType = ruleType; this.name = requireNonNull(name, "name can't be null"); } @@ -244,6 +251,11 @@ public class IssuesChangesNotificationBuilder { return key; } + @CheckForNull + public RuleType getRuleType() { + return ruleType; + } + public String getName() { return name; } @@ -257,18 +269,19 @@ public class IssuesChangesNotificationBuilder { return false; } Rule that = (Rule) o; - return key.equals(that.key) && name.equals(that.name); + return key.equals(that.key) && ruleType.equals(that.ruleType) && name.equals(that.name); } @Override public int hashCode() { - return Objects.hash(key, name); + return Objects.hash(key, ruleType, name); } @Override public String toString() { return "Rule{" + "key=" + key + + ", type=" + ruleType + ", name='" + name + '\'' + '}'; } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssuesChangesNotificationSerializer.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssuesChangesNotificationSerializer.java index 1c5c652e8e0..4b172445ab1 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssuesChangesNotificationSerializer.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/IssuesChangesNotificationSerializer.java @@ -35,6 +35,7 @@ import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.User import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; +import static java.util.Optional.ofNullable; import static org.sonar.core.util.stream.MoreCollectors.toSet; import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; @@ -44,6 +45,7 @@ public class IssuesChangesNotificationSerializer { private static final String FIELD_CHANGE_AUTHOR_UUID = "change.author.uuid"; private static final String FIELD_CHANGE_AUTHOR_LOGIN = "change.author.login"; private static final String FIELD_CHANGE_AUTHOR_NAME = "change.author.name"; + private static final String FIELD_PREFIX_RULES = "rules."; public IssuesChangesNotification serialize(IssuesChangesNotificationBuilder builder) { IssuesChangesNotification res = new IssuesChangesNotification(); @@ -160,7 +162,10 @@ public class IssuesChangesNotificationSerializer { issues.stream() .map(ChangedIssue::getRule) .collect(Collectors.toSet()) - .forEach(rule -> res.setFieldValue("rules." + rule.getKey(), rule.getName())); + .forEach(rule -> { + res.setFieldValue(FIELD_PREFIX_RULES + rule.getKey(), rule.getName()); + ofNullable(rule.getRuleType()).ifPresent(ruleType -> res.setFieldValue(FIELD_PREFIX_RULES + rule.getKey() + ".type", rule.getRuleType().name())); + }); } private static Map readRules(IssuesChangesNotification notification, List issues) { @@ -173,10 +178,11 @@ public class IssuesChangesNotificationSerializer { } private static Rule readRule(IssuesChangesNotification notification, RuleKey ruleKey) { - String fieldName = "rules." + ruleKey; + String fieldName = FIELD_PREFIX_RULES + ruleKey; String ruleName = notification.getFieldValue(fieldName); + String ruleType = notification.getFieldValue(fieldName + ".type"); checkState(ruleName != null, "can not find field %s", ruleKey); - return new Rule(ruleKey, ruleName); + return new Rule(ruleKey, ruleType, ruleName); } private static void serializeProjects(IssuesChangesNotification res, Set issues) { diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/RuleGroup.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/RuleGroup.java new file mode 100644 index 00000000000..2bce7b3f996 --- /dev/null +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/RuleGroup.java @@ -0,0 +1,65 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.issue.notification; + +import java.util.Collection; +import javax.annotation.Nullable; +import org.sonar.api.rules.RuleType; + +import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; + +enum RuleGroup { + SECURITY_HOTSPOTS, + ISSUES; + + static RuleGroup resolveGroup(@Nullable RuleType ruleType) { + return SECURITY_HOTSPOT.equals(ruleType) ? SECURITY_HOTSPOTS : ISSUES; + } + + static String formatIssuesOrHotspots(Collection issues, Collection hotspots) { + if (!issues.isEmpty() && !hotspots.isEmpty()) { + return "issues/hotspots"; + } + if (issues.size() == 1) { + return "an issue"; + } + if (issues.size() > 1) { + return "issues"; + } + if (hotspots.size() == 1) { + return "a hotspot"; + } + return "hotspots"; + } + + static String formatIssueOrHotspot(@Nullable RuleType ruleType) { + if (SECURITY_HOTSPOT.equals(ruleType)) { + return "hotspot"; + } + return "issue"; + } + + static String formatIssuesOrHotspots(@Nullable RuleType ruleType) { + if (SECURITY_HOTSPOT.equals(ruleType)) { + return "hotspots"; + } + return "issues"; + } +} diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandlerTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandlerTest.java index da9a10978fd..e18f8b3be9e 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandlerTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationHandlerTest.java @@ -36,7 +36,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; -import org.sonar.api.rule.RuleKey; import org.sonar.core.util.stream.MoreCollectors; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.AnalysisChange; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.Change; @@ -61,6 +60,7 @@ import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.sonar.core.util.stream.MoreCollectors.index; import static org.sonar.core.util.stream.MoreCollectors.unorderedIndex; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newRandomNotAHotspotRule; import static org.sonar.server.notification.NotificationDispatcherMetadata.GLOBAL_NOTIFICATION; import static org.sonar.server.notification.NotificationDispatcherMetadata.PER_PROJECT_NOTIFICATION; import static org.sonar.server.notification.NotificationManager.SubscriberPermissionsOnProject.ALL_MUST_HAVE_ROLE_USER; @@ -354,10 +354,10 @@ public class ChangesOnMyIssueNotificationHandlerTest { .collect(toSet()); when(notificationManager.findSubscribedEmailRecipients( CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, project1.getKey(), ImmutableSet.of(assignee1.getLogin()), ALL_MUST_HAVE_ROLE_USER)) - .thenReturn(ImmutableSet.of(emailRecipientOf(assignee1.getLogin()))); + .thenReturn(ImmutableSet.of(emailRecipientOf(assignee1.getLogin()))); when(notificationManager.findSubscribedEmailRecipients( CHANGE_ON_MY_ISSUES_DISPATCHER_KEY, project2.getKey(), ImmutableSet.of(assignee2.getLogin()), ALL_MUST_HAVE_ROLE_USER)) - .thenReturn(ImmutableSet.of(emailRecipientOf(assignee2.getLogin()))); + .thenReturn(ImmutableSet.of(emailRecipientOf(assignee2.getLogin()))); int deliveredCount = new Random().nextInt(100); when(emailNotificationChannel.deliverAll(anySet())).thenReturn(deliveredCount); @@ -437,7 +437,7 @@ public class ChangesOnMyIssueNotificationHandlerTest { } private static Rule newRule() { - return new Rule(RuleKey.of(randomAlphabetic(3), randomAlphabetic(4)), randomAlphabetic(5)); + return newRandomNotAHotspotRule(randomAlphabetic(5)); } private static Set randomSetOfNotifications(@Nullable String projectKey, @Nullable String assignee, @Nullable String changeAuthor) { diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplateTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplateTest.java index 6017143a11c..ad7343ccced 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplateTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesEmailTemplateTest.java @@ -32,11 +32,13 @@ import java.util.Set; import java.util.function.Function; import java.util.stream.IntStream; import java.util.stream.Stream; +import org.elasticsearch.common.util.set.Sets; import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.sonar.api.config.EmailSettings; import org.sonar.api.notifications.Notification; +import org.sonar.api.rules.RuleType; import org.sonar.core.i18n.I18n; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.AnalysisChange; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.Change; @@ -60,14 +62,24 @@ import static org.sonar.api.issue.Issue.STATUS_CONFIRMED; import static org.sonar.api.issue.Issue.STATUS_OPEN; import static org.sonar.api.issue.Issue.STATUS_REOPENED; import static org.sonar.api.issue.Issue.STATUS_RESOLVED; +import static org.sonar.api.issue.Issue.STATUS_REVIEWED; +import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; +import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newAnalysisChange; import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newBranch; import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newChangedIssue; import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newProject; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newRandomNotAHotspotRule; import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newRule; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newSecurityHotspotRule; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newUserChange; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.randomRuleTypeHotspotExcluded; @RunWith(DataProviderRunner.class) public class ChangesOnMyIssuesEmailTemplateTest { private static final String[] ISSUE_STATUSES = {STATUS_OPEN, STATUS_RESOLVED, STATUS_CONFIRMED, STATUS_REOPENED, STATUS_CLOSED}; + private static final String[] SECURITY_HOTSPOTS_STATUSES = {STATUS_TO_REVIEW, STATUS_REVIEWED}; + @org.junit.Rule public ExpectedException expectedException = ExpectedException.none(); @@ -84,7 +96,7 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void formats_fails_with_ISE_if_change_from_Analysis_and_no_issue() { - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); expectedException.expect(IllegalStateException.class); expectedException.expectMessage("changedIssues can't be empty"); @@ -95,9 +107,9 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void format_sets_message_id_with_project_key_of_first_issue_in_set_when_change_from_Analysis() { Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) - .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRule("rule_" + i))) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRandomNotAHotspotRule("rule_" + i))) .collect(toSet()); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, changedIssues)); @@ -107,7 +119,7 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void format_sets_subject_with_project_name_of_first_issue_in_set_when_change_from_Analysis() { Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) - .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRule("rule_" + i))) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRandomNotAHotspotRule("rule_" + i))) .collect(toSet()); AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); @@ -120,9 +132,9 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void format_sets_subject_with_project_name_and_branch_name_of_first_issue_in_set_when_change_from_Analysis() { Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) - .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newBranch("prj_" + i, "br_" + i), newRule("rule_" + i))) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newBranch("prj_" + i, "br_" + i), newRandomNotAHotspotRule("rule_" + i))) .collect(toSet()); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, changedIssues)); @@ -133,9 +145,9 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void format_set_html_message_with_header_dealing_with_plural_when_change_from_Analysis() { Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) - .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRule("rule_" + i))) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRandomNotAHotspotRule("rule_" + i))) .collect(toSet()); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); EmailMessage singleIssueMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, changedIssues.stream().limit(1).collect(toSet()))); EmailMessage multiIssueMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, changedIssues)); @@ -151,9 +163,9 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void format_sets_static_message_id_when_change_from_User() { Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) - .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRule("rule_" + i))) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRandomNotAHotspotRule("rule_" + i))) .collect(toSet()); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + UserChange userChange = newUserChange(); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, changedIssues)); @@ -163,21 +175,21 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void format_sets_static_subject_when_change_from_User() { Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) - .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRule("rule_" + i))) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRandomNotAHotspotRule("rule_" + i))) .collect(toSet()); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + UserChange userChange = newUserChange(); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, changedIssues)); - assertThat(emailMessage.getSubject()).isEqualTo("A manual update has changed some of your issues"); + assertThat(emailMessage.getSubject()).isEqualTo("A manual update has changed some of your issues/hotspots"); } @Test - public void format_set_html_message_with_header_dealing_with_plural_when_change_from_User() { + public void format_set_html_message_with_header_dealing_with_plural_issues_when_change_from_User() { Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) - .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRule("rule_" + i))) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRandomNotAHotspotRule("rule_" + i))) .collect(toSet()); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + UserChange userChange = newUserChange(); EmailMessage singleIssueMessage = underTest.format(new ChangesOnMyIssuesNotification( userChange, changedIssues.stream().limit(1).collect(toSet()))); @@ -195,26 +207,91 @@ public class ChangesOnMyIssuesEmailTemplateTest { .withoutLink(); } + @Test + public void format_set_html_message_with_header_dealing_with_plural_security_hotspots_when_change_from_User() { + Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newSecurityHotspotRule("rule_" + i))) + .collect(toSet()); + UserChange userChange = newUserChange(); + + EmailMessage singleIssueMessage = underTest.format(new ChangesOnMyIssuesNotification( + userChange, changedIssues.stream().limit(1).collect(toSet()))); + EmailMessage multiIssueMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, changedIssues)); + + HtmlFragmentAssert.assertThat(singleIssueMessage.getMessage()) + .hasParagraph("Hi,") + .withoutLink() + .hasParagraph("A manual change has updated a hotspot assigned to you:") + .withoutLink(); + HtmlFragmentAssert.assertThat(multiIssueMessage.getMessage()) + .hasParagraph("Hi,") + .withoutLink() + .hasParagraph("A manual change has updated hotspots assigned to you:") + .withoutLink(); + } + + @Test + public void format_set_html_message_with_header_dealing_with_plural_security_hotspots_and_issues_when_change_from_User() { + Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newRandomNotAHotspotRule("rule_" + i))) + .collect(toSet()); + + Set changedHotspots = IntStream.range(0, 2 + new Random().nextInt(4)) + .mapToObj(i -> newChangedIssue(i + "", randomValidStatus(), newProject("prj_" + i), newSecurityHotspotRule("rule_" + i))) + .collect(toSet()); + + Set issuesAndHotspots = Sets.union(changedIssues, changedHotspots); + + UserChange userChange = newUserChange(); + + EmailMessage multiIssueMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, issuesAndHotspots)); + + HtmlFragmentAssert.assertThat(multiIssueMessage.getMessage()) + .hasParagraph("Hi,") + .withoutLink() + .hasParagraph("A manual change has updated issues/hotspots assigned to you:") + .withoutLink(); + } + @Test @UseDataProvider("issueStatuses") - public void format_set_html_message_with_footer_when_change_from_user(String issueStatus) { - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + public void format_set_html_message_with_footer_when_issue_change_from_user(String issueStatus) { + UserChange userChange = newUserChange(); format_set_html_message_with_footer(userChange, issueStatus, c -> c // skip content - .hasParagraph() // open/closed issue - .hasList() // rule list - ); + .hasParagraph() // skip project header + .hasList(), // rule list, + randomRuleTypeHotspotExcluded()); } @Test @UseDataProvider("issueStatuses") - public void format_set_html_message_with_footer_when_change_from_analysis(String issueStatus) { - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + public void format_set_html_message_with_footer_when_issue_change_from_analysis(String issueStatus) { + AnalysisChange analysisChange = newAnalysisChange(); format_set_html_message_with_footer(analysisChange, issueStatus, c -> c - // skip content .hasParagraph() // status - .hasList() // rule list - ); + .hasList(), // rule list, + randomRuleTypeHotspotExcluded()); + } + + @Test + @UseDataProvider("securityHotspotsStatuses") + public void format_set_html_message_with_footer_when_security_hotspot_change_from_analysis(String securityHotspotStatus) { + AnalysisChange analysisChange = newAnalysisChange(); + format_set_html_message_with_footer(analysisChange, securityHotspotStatus, c -> c + .hasParagraph() + .hasList(), // rule list + SECURITY_HOTSPOT); + } + + @Test + @UseDataProvider("securityHotspotsStatuses") + public void format_set_html_message_with_footer_when_security_hotspot_change_from_user(String securityHotspotStatus) { + UserChange userChange = newUserChange(); + format_set_html_message_with_footer(userChange, securityHotspotStatus, c -> c + .hasParagraph() + .hasList(), // rule list + SECURITY_HOTSPOT); } @DataProvider @@ -224,14 +301,21 @@ public class ChangesOnMyIssuesEmailTemplateTest { .toArray(Object[][]::new); } - private void format_set_html_message_with_footer(Change change, String issueStatus, Function skipContent) { + @DataProvider + public static Object[][] securityHotspotsStatuses() { + return Arrays.stream(SECURITY_HOTSPOTS_STATUSES) + .map(t -> new Object[] {t}) + .toArray(Object[][]::new); + } + + private void format_set_html_message_with_footer(Change change, String issueStatus, Function skipContent, RuleType ruleType) { String wordingNotification = randomAlphabetic(20); String host = randomAlphabetic(15); when(i18n.message(Locale.ENGLISH, "notification.dispatcher.ChangesOnMyIssue", "notification.dispatcher.ChangesOnMyIssue")) .thenReturn(wordingNotification); when(emailSettings.getServerBaseURL()).thenReturn(host); Project project = newProject("foo"); - Rule rule = newRule("bar"); + Rule rule = newRule("bar", ruleType); Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(4)) .mapToObj(i -> newChangedIssue(i + "", issueStatus, project, rule)) .collect(toSet()); @@ -260,11 +344,11 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void format_set_html_message_with_issues_grouped_by_status_closed_or_any_other_when_change_from_analysis() { Project project = newProject("foo"); - Rule rule = newRule("bar"); + Rule rule = newRandomNotAHotspotRule("bar"); Set changedIssues = Arrays.stream(ISSUE_STATUSES) .map(status -> newChangedIssue(status + "", status, project, rule)) .collect(toSet()); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, changedIssues)); @@ -282,16 +366,16 @@ public class ChangesOnMyIssuesEmailTemplateTest { } @Test - public void format_set_html_message_with_status_title_handles_plural_when_change_from_analysis() { + public void format_set_html_message_with_issue_status_title_handles_plural_when_change_from_analysis() { Project project = newProject("foo"); - Rule rule = newRule("bar"); + Rule rule = newRandomNotAHotspotRule("bar"); Set closedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(status -> newChangedIssue(status + "", STATUS_CLOSED, project, rule)) .collect(toSet()); Set openIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(status -> newChangedIssue(status + "", STATUS_OPEN, project, rule)) .collect(toSet()); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); EmailMessage closedIssuesMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, closedIssues)); EmailMessage openIssuesMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, openIssues)); @@ -313,8 +397,8 @@ public class ChangesOnMyIssuesEmailTemplateTest { Project project = newProject("1"); String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); - ChangedIssue changedIssue = newChangedIssue("key", randomValidStatus(), project, ruleName); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + ChangedIssue changedIssue = newChangedIssue("key", randomValidStatus(), project, ruleName, randomRuleTypeHotspotExcluded()); + AnalysisChange analysisChange = newAnalysisChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, ImmutableSet.of(changedIssue))); @@ -333,8 +417,8 @@ public class ChangesOnMyIssuesEmailTemplateTest { Project project = newProject("1"); String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); - ChangedIssue changedIssue = newChangedIssue("key", randomValidStatus(), project, ruleName); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + ChangedIssue changedIssue = newChangedIssue("key", randomValidStatus(), project, ruleName, randomRuleTypeHotspotExcluded()); + UserChange userChange = newUserChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, ImmutableSet.of(changedIssue))); @@ -355,8 +439,8 @@ public class ChangesOnMyIssuesEmailTemplateTest { String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); String key = "key"; - ChangedIssue changedIssue = newChangedIssue(key, randomValidStatus(), project, ruleName); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + ChangedIssue changedIssue = newChangedIssue(key, randomValidStatus(), project, ruleName, randomRuleTypeHotspotExcluded()); + AnalysisChange analysisChange = newAnalysisChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, ImmutableSet.of(changedIssue))); @@ -378,8 +462,8 @@ public class ChangesOnMyIssuesEmailTemplateTest { String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); String key = "key"; - ChangedIssue changedIssue = newChangedIssue(key, randomValidStatus(), project, ruleName); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + ChangedIssue changedIssue = newChangedIssue(key, randomValidStatus(), project, ruleName, randomRuleTypeHotspotExcluded()); + UserChange userChange = newUserChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, ImmutableSet.of(changedIssue))); @@ -399,12 +483,12 @@ public class ChangesOnMyIssuesEmailTemplateTest { Project project = newProject("1"); String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); - Rule rule = newRule(ruleName); + Rule rule = newRule(ruleName, randomRuleTypeHotspotExcluded()); String issueStatus = randomValidStatus(); List changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(i -> newChangedIssue("issue_" + i, issueStatus, project, rule)) .collect(toList()); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, ImmutableSet.copyOf(changedIssues))); @@ -426,11 +510,11 @@ public class ChangesOnMyIssuesEmailTemplateTest { Project project = newProject("1"); String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); - Rule rule = newRule(ruleName); + Rule rule = newRule(ruleName, randomRuleTypeHotspotExcluded()); List changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(i -> newChangedIssue("issue_" + i, randomValidStatus(), project, rule)) .collect(toList()); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + UserChange userChange = newUserChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, ImmutableSet.copyOf(changedIssues))); @@ -453,12 +537,12 @@ public class ChangesOnMyIssuesEmailTemplateTest { Project project = newBranch("1", branchName); String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); - Rule rule = newRule(ruleName); + Rule rule = newRule(ruleName, randomRuleTypeHotspotExcluded()); String status = randomValidStatus(); List changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(i -> newChangedIssue("issue_" + i, status, project, rule)) .collect(toList()); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, ImmutableSet.copyOf(changedIssues))); @@ -481,11 +565,11 @@ public class ChangesOnMyIssuesEmailTemplateTest { Project project = newBranch("1", branchName); String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); - Rule rule = newRule(ruleName); + Rule rule = newRandomNotAHotspotRule(ruleName); List changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(i -> newChangedIssue("issue_" + i, randomValidStatus(), project, rule)) .collect(toList()); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + UserChange userChange = newUserChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, ImmutableSet.copyOf(changedIssues))); @@ -512,10 +596,10 @@ public class ChangesOnMyIssuesEmailTemplateTest { Project project3 = newProject("C"); String host = randomAlphabetic(15); List changedIssues = Stream.of(project1, project1Branch1, project1Branch2, project2, project2Branch1, project3) - .map(project -> newChangedIssue("issue_" + project.getUuid(), randomValidStatus(), project, newRule(randomAlphabetic(2)))) + .map(project -> newChangedIssue("issue_" + project.getUuid(), randomValidStatus(), project, newRule(randomAlphabetic(2), randomRuleTypeHotspotExcluded()))) .collect(toList()); Collections.shuffle(changedIssues); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + UserChange userChange = newUserChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, ImmutableSet.copyOf(changedIssues))); @@ -541,17 +625,18 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void formats_returns_html_message_with_rules_ordered_by_name_when_analysis_change() { Project project = newProject("1"); - Rule rule1 = newRule("1"); - Rule rule2 = newRule("a"); - Rule rule3 = newRule("b"); - Rule rule4 = newRule("X"); + Rule rule1 = newRandomNotAHotspotRule("1"); + Rule rule2 = newRandomNotAHotspotRule("a"); + Rule rule3 = newRandomNotAHotspotRule("b"); + Rule rule4 = newRandomNotAHotspotRule("X"); + String host = randomAlphabetic(15); String issueStatus = randomValidStatus(); List changedIssues = Stream.of(rule1, rule2, rule3, rule4) .map(rule -> newChangedIssue("issue_" + rule.getName(), issueStatus, project, rule)) .collect(toList()); Collections.shuffle(changedIssues); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, ImmutableSet.copyOf(changedIssues))); @@ -569,30 +654,43 @@ public class ChangesOnMyIssuesEmailTemplateTest { } @Test - public void formats_returns_html_message_with_rules_ordered_by_name_when_analysis_change_when_user_analysis() { + public void formats_returns_html_message_with_rules_ordered_by_name_user_change() { Project project = newProject("1"); - Rule rule1 = newRule("1"); - Rule rule2 = newRule("a"); - Rule rule3 = newRule("b"); - Rule rule4 = newRule("X"); + Rule rule1 = newRandomNotAHotspotRule("1"); + Rule rule2 = newRandomNotAHotspotRule("a"); + Rule rule3 = newRandomNotAHotspotRule("b"); + Rule rule4 = newRandomNotAHotspotRule("X"); + + Rule hotspot1 = newSecurityHotspotRule("S"); + Rule hotspot2 = newSecurityHotspotRule("Z"); + Rule hotspot3 = newSecurityHotspotRule("N"); + Rule hotspot4 = newSecurityHotspotRule("M"); + String host = randomAlphabetic(15); - List changedIssues = Stream.of(rule1, rule2, rule3, rule4) + List changedIssues = Stream.of(rule1, rule2, rule3, rule4, hotspot1, hotspot2, hotspot3, hotspot4) .map(rule -> newChangedIssue("issue_" + rule.getName(), randomValidStatus(), project, rule)) .collect(toList()); Collections.shuffle(changedIssues); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + UserChange userChange = newUserChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, ImmutableSet.copyOf(changedIssues))); HtmlFragmentAssert.assertThat(emailMessage.getMessage()) - .hasParagraph().hasParagraph() // skip header - .hasParagraph(project.getProjectName()) + .hasParagraph() + .hasParagraph() + .hasParagraph() // skip project name .hasList( "Rule " + rule1.getName() + " - See the single issue", "Rule " + rule2.getName() + " - See the single issue", "Rule " + rule3.getName() + " - See the single issue", "Rule " + rule4.getName() + " - See the single issue") + .hasEmptyParagraph() + .hasList( + "Rule " + hotspot1.getName() + " - See the single hotspot", + "Rule " + hotspot2.getName() + " - See the single hotspot", + "Rule " + hotspot3.getName() + " - See the single hotspot", + "Rule " + hotspot4.getName() + " - See the single hotspot") .hasParagraph().hasParagraph() // skip footer .noMoreBlock(); } @@ -600,11 +698,13 @@ public class ChangesOnMyIssuesEmailTemplateTest { @Test public void formats_returns_html_message_with_multiple_links_by_rule_of_groups_of_up_to_40_issues_when_analysis_change() { Project project1 = newProject("1"); - Rule rule1 = newRule("1"); - Rule rule2 = newRule("a"); + Rule rule1 = newRandomNotAHotspotRule("1"); + Rule rule2 = newRandomNotAHotspotRule("a"); + String host = randomAlphabetic(15); String issueStatusClosed = STATUS_CLOSED; String otherIssueStatus = STATUS_RESOLVED; + List changedIssues = Stream.of( IntStream.range(0, 39).mapToObj(i -> newChangedIssue("39_" + i, issueStatusClosed, project1, rule1)), IntStream.range(0, 40).mapToObj(i -> newChangedIssue("40_" + i, issueStatusClosed, project1, rule2)), @@ -612,8 +712,9 @@ public class ChangesOnMyIssuesEmailTemplateTest { IntStream.range(0, 6).mapToObj(i -> newChangedIssue("6_" + i, otherIssueStatus, project1, rule1))) .flatMap(t -> t) .collect(toList()); + Collections.shuffle(changedIssues); - AnalysisChange analysisChange = IssuesChangesNotificationBuilderTesting.newAnalysisChange(); + AnalysisChange analysisChange = newAnalysisChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(analysisChange, ImmutableSet.copyOf(changedIssues))); @@ -651,23 +752,32 @@ public class ChangesOnMyIssuesEmailTemplateTest { } @Test - public void formats_returns_html_message_with_multiple_links_by_rule_of_groups_of_up_to_40_issues_when_user_change() { + public void formats_returns_html_message_with_multiple_links_by_rule_of_groups_of_up_to_40_issues_and_hotspots_when_user_change() { Project project1 = newProject("1"); Project project2 = newProject("V"); Project project2Branch = newBranch("V", "AB"); - Rule rule1 = newRule("1"); - Rule rule2 = newRule("a"); + Rule rule1 = newRule("1", randomRuleTypeHotspotExcluded()); + Rule rule2 = newRule("a", randomRuleTypeHotspotExcluded()); + + Rule hotspot1 = newSecurityHotspotRule("h1"); + Rule hotspot2 = newSecurityHotspotRule("h2"); + String status = randomValidStatus(); String host = randomAlphabetic(15); List changedIssues = Stream.of( IntStream.range(0, 39).mapToObj(i -> newChangedIssue("39_" + i, status, project1, rule1)), IntStream.range(0, 40).mapToObj(i -> newChangedIssue("40_" + i, status, project1, rule2)), IntStream.range(0, 81).mapToObj(i -> newChangedIssue("1-40_41-80_1_" + i, status, project2, rule2)), - IntStream.range(0, 6).mapToObj(i -> newChangedIssue("6_" + i, status, project2Branch, rule1))) + IntStream.range(0, 6).mapToObj(i -> newChangedIssue("6_" + i, status, project2Branch, rule1)), + + IntStream.range(0, 39).mapToObj(i -> newChangedIssue("39_" + i, STATUS_REVIEWED, project1, hotspot1)), + IntStream.range(0, 40).mapToObj(i -> newChangedIssue("40_" + i, STATUS_REVIEWED, project1, hotspot2)), + IntStream.range(0, 81).mapToObj(i -> newChangedIssue("1-40_41-80_1_" + i, STATUS_TO_REVIEW, project2, hotspot2)), + IntStream.range(0, 6).mapToObj(i -> newChangedIssue("6_" + i, STATUS_TO_REVIEW, project2Branch, hotspot1))) .flatMap(t -> t) .collect(toList()); Collections.shuffle(changedIssues); - UserChange userChange = IssuesChangesNotificationBuilderTesting.newUserChange(); + UserChange userChange = newUserChange(); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new ChangesOnMyIssuesNotification(userChange, ImmutableSet.copyOf(changedIssues))); @@ -685,8 +795,20 @@ public class ChangesOnMyIssuesEmailTemplateTest { .withLink("See all 40 issues", host + "/project/issues?id=" + project1.getKey() + "&issues=" + IntStream.range(0, 40).mapToObj(i -> "40_" + i).sorted().collect(joining("%2C"))) + .hasEmptyParagraph() + .hasList() + .withItemTexts( + "Rule " + hotspot1.getName() + " - See all 39 hotspots", + "Rule " + hotspot2.getName() + " - See all 40 hotspots") + .withLink("See all 39 hotspots", + host + "/security_hotspots?id=" + project1.getKey() + + "&hotspots=" + IntStream.range(0, 39).mapToObj(i -> "39_" + i).sorted().collect(joining("%2C"))) + .withLink("See all 40 hotspots", + host + "/security_hotspots?id=" + project1.getKey() + + "&hotspots=" + IntStream.range(0, 40).mapToObj(i -> "40_" + i).sorted().collect(joining("%2C"))) .hasParagraph(project2.getProjectName()) - .hasList("Rule " + rule2.getName() + " - See issues 1-40 41-80 81") + .hasList( + "Rule " + rule2.getName() + " - See issues 1-40 41-80 81") .withLink("1-40", host + "/project/issues?id=" + project2.getKey() + "&issues=" + IntStream.range(0, 81).mapToObj(i -> "1-40_41-80_1_" + i).sorted().limit(40).collect(joining("%2C"))) @@ -696,11 +818,28 @@ public class ChangesOnMyIssuesEmailTemplateTest { .withLink("81", host + "/project/issues?id=" + project2.getKey() + "&issues=" + "1-40_41-80_1_9" + "&open=" + "1-40_41-80_1_9") + .hasEmptyParagraph() + .hasList("Rule " + hotspot2.getName() + " - See hotspots 1-40 41-80 81") + .withLink("1-40", + host + "/security_hotspots?id=" + project2.getKey() + + "&hotspots=" + IntStream.range(0, 81).mapToObj(i -> "1-40_41-80_1_" + i).sorted().limit(40).collect(joining("%2C"))) + .withLink("41-80", + host + "/security_hotspots?id=" + project2.getKey() + + "&hotspots=" + IntStream.range(0, 81).mapToObj(i -> "1-40_41-80_1_" + i).sorted().skip(40).limit(40).collect(joining("%2C"))) + .withLink("81", + host + "/security_hotspots?id=" + project2.getKey() + + "&hotspots=" + "1-40_41-80_1_9") .hasParagraph(project2Branch.getProjectName() + ", " + project2Branch.getBranchName().get()) - .hasList("Rule " + rule1.getName() + " - See all 6 issues") + .hasList( + "Rule " + rule1.getName() + " - See all 6 issues") .withLink("See all 6 issues", host + "/project/issues?id=" + project2Branch.getKey() + "&branch=" + project2Branch.getBranchName().get() + "&issues=" + IntStream.range(0, 6).mapToObj(i -> "6_" + i).sorted().collect(joining("%2C"))) + .hasEmptyParagraph() + .hasList("Rule " + hotspot1.getName() + " - See all 6 hotspots") + .withLink("See all 6 hotspots", + host + "/security_hotspots?id=" + project2Branch.getKey() + "&branch=" + project2Branch.getBranchName().get() + + "&hotspots=" + IntStream.range(0, 6).mapToObj(i -> "6_" + i).sorted().collect(joining("%2C"))) .hasParagraph().hasParagraph() // skip footer .noMoreBlock(); } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesNotificationTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesNotificationTest.java index 35dbe097585..5bce5adc5a8 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesNotificationTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/ChangesOnMyIssuesNotificationTest.java @@ -31,6 +31,7 @@ import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.User import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newRandomNotAHotspotRule; public class ChangesOnMyIssuesNotificationTest { @Test @@ -45,7 +46,8 @@ public class ChangesOnMyIssuesNotificationTest { @Test public void equals_is_based_on_change_and_issues() { AnalysisChange analysisChange = new AnalysisChange(new Random().nextLong()); - ChangedIssue changedIssue = IssuesChangesNotificationBuilderTesting.newChangedIssue("doo", IssuesChangesNotificationBuilderTesting.newProject("prj"), IssuesChangesNotificationBuilderTesting.newRule("rul")); + ChangedIssue changedIssue = IssuesChangesNotificationBuilderTesting.newChangedIssue("doo", IssuesChangesNotificationBuilderTesting.newProject("prj"), + newRandomNotAHotspotRule("rul")); ChangesOnMyIssuesNotification underTest = new ChangesOnMyIssuesNotification(analysisChange, ImmutableSet.of(changedIssue)); assertThat(underTest) @@ -59,7 +61,8 @@ public class ChangesOnMyIssuesNotificationTest { @Test public void hashcode_is_based_on_change_and_issues() { AnalysisChange analysisChange = new AnalysisChange(new Random().nextLong()); - ChangedIssue changedIssue = IssuesChangesNotificationBuilderTesting.newChangedIssue("doo", IssuesChangesNotificationBuilderTesting.newProject("prj"), IssuesChangesNotificationBuilderTesting.newRule("rul")); + ChangedIssue changedIssue = IssuesChangesNotificationBuilderTesting.newChangedIssue("doo", IssuesChangesNotificationBuilderTesting.newProject("prj"), + newRandomNotAHotspotRule("rul")); ChangesOnMyIssuesNotification underTest = new ChangesOnMyIssuesNotification(analysisChange, ImmutableSet.of(changedIssue)); assertThat(underTest.hashCode()) diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FPOrWontFixNotificationHandlerTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FPOrWontFixNotificationHandlerTest.java index 67ec465b4ef..b0d74a7fdaa 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FPOrWontFixNotificationHandlerTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FPOrWontFixNotificationHandlerTest.java @@ -34,12 +34,10 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import org.sonar.api.issue.Issue; -import org.sonar.api.rule.RuleKey; import org.sonar.server.issue.notification.FPOrWontFixNotification.FpOrWontFix; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.Change; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.ChangedIssue; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.Project; -import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.Rule; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.User; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.UserChange; import org.sonar.server.notification.NotificationDispatcherMetadata; @@ -66,6 +64,7 @@ import static org.sonar.api.issue.Issue.RESOLUTION_FALSE_POSITIVE; import static org.sonar.api.issue.Issue.RESOLUTION_WONT_FIX; import static org.sonar.core.util.stream.MoreCollectors.index; import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newProject; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newRandomNotAHotspotRule; import static org.sonar.server.notification.NotificationDispatcherMetadata.GLOBAL_NOTIFICATION; import static org.sonar.server.notification.NotificationDispatcherMetadata.PER_PROJECT_NOTIFICATION; import static org.sonar.server.notification.NotificationManager.SubscriberPermissionsOnProject.ALL_MUST_HAVE_ROLE_USER; @@ -477,7 +476,7 @@ public class FPOrWontFixNotificationHandlerTest { .setAssignee(new User(randomAlphabetic(3), randomAlphabetic(4), randomAlphabetic(5))) .setNewStatus(randomAlphabetic(12)) .setNewResolution(randomAlphabetic(13)) - .setRule(new Rule(RuleKey.of(randomAlphabetic(6), randomAlphabetic(7)), randomAlphabetic(8))) + .setRule(newRandomNotAHotspotRule(randomAlphabetic(8))) .setProject(new Project.Builder(randomAlphabetic(9)) .setKey(randomAlphabetic(10)) .setProjectName(randomAlphabetic(11)) diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FPOrWontFixNotificationTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FPOrWontFixNotificationTest.java index ed1e5e59e5c..83174524808 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FPOrWontFixNotificationTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FPOrWontFixNotificationTest.java @@ -26,7 +26,6 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.IntStream; import org.junit.Test; -import org.sonar.api.rule.RuleKey; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.AnalysisChange; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.ChangedIssue; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.Project; @@ -36,18 +35,19 @@ import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.User import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.server.issue.notification.FPOrWontFixNotification.FpOrWontFix.FP; import static org.sonar.server.issue.notification.FPOrWontFixNotification.FpOrWontFix.WONT_FIX; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newRandomNotAHotspotRule; public class FPOrWontFixNotificationTest { @Test public void equals_is_based_on_issues_change_and_resolution() { - Rule rule = new Rule(RuleKey.of("repo", "rule_key"), "rule_name"); + Rule rule = newRandomNotAHotspotRule("rule_name"); Project project = new Project.Builder("prj_uuid").setKey("prj_key").setProjectName("prj_name").build(); Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(i -> new ChangedIssue.Builder("key_" + i) .setNewStatus("status") .setRule(rule) .setProject(project) - .build()) + .build()) .collect(Collectors.toSet()); AnalysisChange change = new AnalysisChange(12); User user = new User("uuid", "login", null); @@ -64,16 +64,17 @@ public class FPOrWontFixNotificationTest { .isNotEqualTo(new FPOrWontFixNotification(new IssuesChangesNotificationBuilder.UserChange(12, user), changedIssues, WONT_FIX)) .isNotEqualTo(new FPOrWontFixNotification(change, changedIssues, FP)); } + @Test public void hashcode_is_based_on_issues_change_and_resolution() { - Rule rule = new Rule(RuleKey.of("repo", "rule_key"), "rule_name"); + Rule rule = newRandomNotAHotspotRule("rule_name"); Project project = new Project.Builder("prj_uuid").setKey("prj_key").setProjectName("prj_name").build(); Set changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(i -> new ChangedIssue.Builder("key_" + i) .setNewStatus("status") .setRule(rule) .setProject(project) - .build()) + .build()) .collect(Collectors.toSet()); AnalysisChange change = new AnalysisChange(12); User user = new User("uuid", "login", null); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FpOrWontFixEmailTemplateTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FpOrWontFixEmailTemplateTest.java index c0479b9f387..38e3ad3f6ee 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FpOrWontFixEmailTemplateTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/FpOrWontFixEmailTemplateTest.java @@ -34,6 +34,7 @@ import org.junit.runner.RunWith; import org.sonar.api.config.EmailSettings; import org.sonar.api.notifications.Notification; import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.RuleType; import org.sonar.core.i18n.I18n; import org.sonar.server.issue.notification.FPOrWontFixNotification.FpOrWontFix; import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.AnalysisChange; @@ -51,8 +52,12 @@ import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; import static org.sonar.server.issue.notification.FPOrWontFixNotification.FpOrWontFix.FP; import static org.sonar.server.issue.notification.FPOrWontFixNotification.FpOrWontFix.WONT_FIX; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newRandomNotAHotspotRule; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.newSecurityHotspotRule; +import static org.sonar.server.issue.notification.IssuesChangesNotificationBuilderTesting.randomRuleTypeHotspotExcluded; @RunWith(DataProviderRunner.class) public class FpOrWontFixEmailTemplateTest { @@ -160,7 +165,7 @@ public class FpOrWontFixEmailTemplateTest { Project project = newProject("1"); String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); - ChangedIssue changedIssue = newChangedIssue("key", project, ruleName); + ChangedIssue changedIssue = newChangedIssue("key", project, ruleName, randomRuleTypeHotspotExcluded()); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new FPOrWontFixNotification(change, ImmutableSet.of(changedIssue), fpOrWontFix)); @@ -174,6 +179,26 @@ public class FpOrWontFixEmailTemplateTest { .noMoreBlock(); } + @Test + @UseDataProvider("fpOrWontFixValuesByUserOrAnalysisChange") + public void formats_returns_html_message_for_single_hotspot_on_master(Change change, FpOrWontFix fpOrWontFix) { + Project project = newProject("1"); + String ruleName = randomAlphabetic(8); + String host = randomAlphabetic(15); + ChangedIssue changedIssue = newChangedIssue("key", project, ruleName, SECURITY_HOTSPOT); + when(emailSettings.getServerBaseURL()).thenReturn(host); + + EmailMessage emailMessage = underTest.format(new FPOrWontFixNotification(change, ImmutableSet.of(changedIssue), fpOrWontFix)); + + HtmlFragmentAssert.assertThat(emailMessage.getMessage()) + .hasParagraph().hasParagraph() // skip header + .hasParagraph(project.getProjectName()) + .hasList("Rule " + ruleName + " - See the single hotspot") + .withLink("See the single hotspot", host + "/project/issues?id=" + project.getKey() + "&issues=" + changedIssue.getKey() + "&open=" + changedIssue.getKey()) + .hasParagraph().hasParagraph() // skip footer + .noMoreBlock(); + } + @Test @UseDataProvider("fpOrWontFixValuesByUserOrAnalysisChange") public void formats_returns_html_message_for_single_issue_on_branch(Change change, FpOrWontFix fpOrWontFix) { @@ -182,7 +207,7 @@ public class FpOrWontFixEmailTemplateTest { String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); String key = "key"; - ChangedIssue changedIssue = newChangedIssue(key, project, ruleName); + ChangedIssue changedIssue = newChangedIssue(key, project, ruleName, randomRuleTypeHotspotExcluded()); when(emailSettings.getServerBaseURL()).thenReturn(host); EmailMessage emailMessage = underTest.format(new FPOrWontFixNotification(change, ImmutableSet.of(changedIssue), fpOrWontFix)); @@ -197,13 +222,36 @@ public class FpOrWontFixEmailTemplateTest { .noMoreBlock(); } + @Test + @UseDataProvider("fpOrWontFixValuesByUserOrAnalysisChange") + public void formats_returns_html_message_for_single_hotspot_on_branch(Change change, FpOrWontFix fpOrWontFix) { + String branchName = randomAlphabetic(6); + Project project = newBranch("1", branchName); + String ruleName = randomAlphabetic(8); + String host = randomAlphabetic(15); + String key = "key"; + ChangedIssue changedIssue = newChangedIssue(key, project, ruleName, SECURITY_HOTSPOT); + when(emailSettings.getServerBaseURL()).thenReturn(host); + + EmailMessage emailMessage = underTest.format(new FPOrWontFixNotification(change, ImmutableSet.of(changedIssue), fpOrWontFix)); + + HtmlFragmentAssert.assertThat(emailMessage.getMessage()) + .hasParagraph().hasParagraph() // skip header + .hasParagraph(project.getProjectName() + ", " + branchName) + .hasList("Rule " + ruleName + " - See the single hotspot") + .withLink("See the single hotspot", + host + "/project/issues?id=" + project.getKey() + "&branch=" + branchName + "&issues=" + changedIssue.getKey() + "&open=" + changedIssue.getKey()) + .hasParagraph().hasParagraph() // skip footer + .noMoreBlock(); + } + @Test @UseDataProvider("fpOrWontFixValuesByUserOrAnalysisChange") public void formats_returns_html_message_for_multiple_issues_of_same_rule_on_same_project_on_master(Change change, FpOrWontFix fpOrWontFix) { Project project = newProject("1"); String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); - Rule rule = newRule(ruleName); + Rule rule = newRandomNotAHotspotRule(ruleName); List changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(i -> newChangedIssue("issue_" + i, project, rule)) .collect(toList()); @@ -223,6 +271,32 @@ public class FpOrWontFixEmailTemplateTest { .noMoreBlock(); } + @Test + @UseDataProvider("fpOrWontFixValuesByUserOrAnalysisChange") + public void formats_returns_html_message_for_multiple_hotspots_of_same_rule_on_same_project_on_master(Change change, FpOrWontFix fpOrWontFix) { + Project project = newProject("1"); + String ruleName = randomAlphabetic(8); + String host = randomAlphabetic(15); + Rule rule = newSecurityHotspotRule(ruleName); + List changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) + .mapToObj(i -> newChangedIssue("issue_" + i, project, rule)) + .collect(toList()); + when(emailSettings.getServerBaseURL()).thenReturn(host); + + EmailMessage emailMessage = underTest.format(new FPOrWontFixNotification(change, ImmutableSet.copyOf(changedIssues), fpOrWontFix)); + + String expectedHref = host + "/project/issues?id=" + project.getKey() + + "&issues=" + changedIssues.stream().map(ChangedIssue::getKey).collect(joining("%2C")); + String expectedLinkText = "See all " + changedIssues.size() + " hotspots"; + HtmlFragmentAssert.assertThat(emailMessage.getMessage()) + .hasParagraph().hasParagraph() // skip header + .hasParagraph(project.getProjectName()) + .hasList("Rule " + ruleName + " - " + expectedLinkText) + .withLink(expectedLinkText, expectedHref) + .hasParagraph().hasParagraph() // skip footer + .noMoreBlock(); + } + @Test @UseDataProvider("fpOrWontFixValuesByUserOrAnalysisChange") public void formats_returns_html_message_for_multiple_issues_of_same_rule_on_same_project_on_branch(Change change, FpOrWontFix fpOrWontFix) { @@ -230,7 +304,7 @@ public class FpOrWontFixEmailTemplateTest { Project project = newBranch("1", branchName); String ruleName = randomAlphabetic(8); String host = randomAlphabetic(15); - Rule rule = newRule(ruleName); + Rule rule = newRandomNotAHotspotRule(ruleName); List changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) .mapToObj(i -> newChangedIssue("issue_" + i, project, rule)) .collect(toList()); @@ -250,6 +324,33 @@ public class FpOrWontFixEmailTemplateTest { .noMoreBlock(); } + @Test + @UseDataProvider("fpOrWontFixValuesByUserOrAnalysisChange") + public void formats_returns_html_message_for_multiple_hotspots_of_same_rule_on_same_project_on_branch(Change change, FpOrWontFix fpOrWontFix) { + String branchName = randomAlphabetic(19); + Project project = newBranch("1", branchName); + String ruleName = randomAlphabetic(8); + String host = randomAlphabetic(15); + Rule rule = newSecurityHotspotRule(ruleName); + List changedIssues = IntStream.range(0, 2 + new Random().nextInt(5)) + .mapToObj(i -> newChangedIssue("issue_" + i, project, rule)) + .collect(toList()); + when(emailSettings.getServerBaseURL()).thenReturn(host); + + EmailMessage emailMessage = underTest.format(new FPOrWontFixNotification(change, ImmutableSet.copyOf(changedIssues), fpOrWontFix)); + + String expectedHref = host + "/project/issues?id=" + project.getKey() + "&branch=" + branchName + + "&issues=" + changedIssues.stream().map(ChangedIssue::getKey).collect(joining("%2C")); + String expectedLinkText = "See all " + changedIssues.size() + " hotspots"; + HtmlFragmentAssert.assertThat(emailMessage.getMessage()) + .hasParagraph().hasParagraph() // skip header + .hasParagraph(project.getProjectName() + ", " + branchName) + .hasList("Rule " + ruleName + " - " + expectedLinkText) + .withLink(expectedLinkText, expectedHref) + .hasParagraph().hasParagraph() // skip footer + .noMoreBlock(); + } + @Test @UseDataProvider("fpOrWontFixValuesByUserOrAnalysisChange") public void formats_returns_html_message_with_projects_ordered_by_name(Change change, FpOrWontFix fpOrWontFix) { @@ -261,7 +362,7 @@ public class FpOrWontFixEmailTemplateTest { Project project3 = newProject("C"); String host = randomAlphabetic(15); List changedIssues = Stream.of(project1, project1Branch1, project1Branch2, project2, project2Branch1, project3) - .map(project -> newChangedIssue("issue_" + project.getUuid(), project, newRule(randomAlphabetic(2)))) + .map(project -> newChangedIssue("issue_" + project.getUuid(), project, newRandomNotAHotspotRule(randomAlphabetic(2)))) .collect(toList()); Collections.shuffle(changedIssues); when(emailSettings.getServerBaseURL()).thenReturn(host); @@ -290,10 +391,10 @@ public class FpOrWontFixEmailTemplateTest { @UseDataProvider("fpOrWontFixValuesByUserOrAnalysisChange") public void formats_returns_html_message_with_rules_ordered_by_name(Change change, FpOrWontFix fpOrWontFix) { Project project = newProject("1"); - Rule rule1 = newRule("1"); - Rule rule2 = newRule("a"); - Rule rule3 = newRule("b"); - Rule rule4 = newRule("X"); + Rule rule1 = newRandomNotAHotspotRule("1"); + Rule rule2 = newRandomNotAHotspotRule("a"); + Rule rule3 = newRandomNotAHotspotRule("b"); + Rule rule4 = newRandomNotAHotspotRule("X"); String host = randomAlphabetic(15); List changedIssues = Stream.of(rule1, rule2, rule3, rule4) .map(rule -> newChangedIssue("issue_" + rule.getName(), project, rule)) @@ -321,8 +422,8 @@ public class FpOrWontFixEmailTemplateTest { Project project1 = newProject("1"); Project project2 = newProject("V"); Project project2Branch = newBranch("V", "AB"); - Rule rule1 = newRule("1"); - Rule rule2 = newRule("a"); + Rule rule1 = newRandomNotAHotspotRule("1"); + Rule rule2 = newRandomNotAHotspotRule("a"); String host = randomAlphabetic(15); List changedIssues = Stream.of( IntStream.range(0, 39).mapToObj(i -> newChangedIssue("39_" + i, project1, rule1)), @@ -393,8 +494,8 @@ public class FpOrWontFixEmailTemplateTest { }; } - private static ChangedIssue newChangedIssue(String key, Project project, String ruleName) { - return newChangedIssue(key, project, newRule(ruleName)); + private static ChangedIssue newChangedIssue(String key, Project project, String ruleName, RuleType ruleType) { + return newChangedIssue(key, project, newRule(ruleName, ruleType)); } private static ChangedIssue newChangedIssue(String key, Project project, Rule rule) { @@ -405,8 +506,8 @@ public class FpOrWontFixEmailTemplateTest { .build(); } - private static Rule newRule(String ruleName) { - return new Rule(RuleKey.of(randomAlphabetic(6), randomAlphabetic(7)), ruleName); + private static Rule newRule(String ruleName, RuleType ruleType) { + return new Rule(RuleKey.of(randomAlphabetic(6), randomAlphabetic(7)), ruleType, ruleName); } private static Project newProject(String uuid) { diff --git a/server/sonar-server-common/src/testFixtures/java/org/sonar/server/issue/notification/IssuesChangesNotificationBuilderTesting.java b/server/sonar-server-common/src/testFixtures/java/org/sonar/server/issue/notification/IssuesChangesNotificationBuilderTesting.java index ee1727a0407..2d2c39e1f06 100644 --- a/server/sonar-server-common/src/testFixtures/java/org/sonar/server/issue/notification/IssuesChangesNotificationBuilderTesting.java +++ b/server/sonar-server-common/src/testFixtures/java/org/sonar/server/issue/notification/IssuesChangesNotificationBuilderTesting.java @@ -21,6 +21,7 @@ package org.sonar.server.issue.notification; import java.util.Random; import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.RuleType; import org.sonar.db.DbTester; import org.sonar.db.component.BranchDto; import org.sonar.db.component.ComponentDto; @@ -36,15 +37,24 @@ import org.sonar.server.issue.notification.IssuesChangesNotificationBuilder.User import static com.google.common.base.Preconditions.checkArgument; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; +import static org.sonar.api.rules.RuleType.BUG; +import static org.sonar.api.rules.RuleType.CODE_SMELL; +import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; +import static org.sonar.api.rules.RuleType.VULNERABILITY; public class IssuesChangesNotificationBuilderTesting { + private static final RuleType[] RULE_TYPES = {CODE_SMELL, BUG, VULNERABILITY, SECURITY_HOTSPOT}; + + private IssuesChangesNotificationBuilderTesting() { + } + public static Rule ruleOf(RuleDto rule) { - return new Rule(rule.getKey(), rule.getName()); + return new Rule(rule.getKey(), RuleType.valueOfNullable(rule.getType()), rule.getName()); } public static Rule ruleOf(RuleDefinitionDto rule) { - return new Rule(rule.getKey(), rule.getName()); + return new Rule(rule.getKey(), RuleType.valueOfNullable(rule.getType()), rule.getName()); } public static User userOf(UserDto changeAuthor) { @@ -76,8 +86,8 @@ public class IssuesChangesNotificationBuilderTesting { .build(); } - static ChangedIssue newChangedIssue(String key, String status, Project project, String ruleName) { - return newChangedIssue(key, status, project, newRule(ruleName)); + static ChangedIssue newChangedIssue(String key, String status, Project project, String ruleName, RuleType ruleType) { + return newChangedIssue(key, status, project, newRule(ruleName, ruleType)); } static ChangedIssue newChangedIssue(String key, String status, Project project, Rule rule) { @@ -88,8 +98,16 @@ public class IssuesChangesNotificationBuilderTesting { .build(); } - static Rule newRule(String ruleName) { - return new Rule(RuleKey.of(randomAlphabetic(6), randomAlphabetic(7)), ruleName); + static Rule newRule(String ruleName, RuleType ruleType) { + return new Rule(RuleKey.of(randomAlphabetic(6), randomAlphabetic(7)), ruleType, ruleName); + } + + static Rule newRandomNotAHotspotRule(String ruleName) { + return newRule(ruleName, randomRuleTypeHotspotExcluded()); + } + + static Rule newSecurityHotspotRule(String ruleName) { + return newRule(ruleName, SECURITY_HOTSPOT); } static Project newProject(String uuid) { @@ -107,4 +125,8 @@ public class IssuesChangesNotificationBuilderTesting { static AnalysisChange newAnalysisChange() { return new AnalysisChange(new Random().nextLong()); } + + static RuleType randomRuleTypeHotspotExcluded() { + return RULE_TYPES[new Random().nextInt(RULE_TYPES.length - 1)]; + } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java index d67f119cbd7..26ffc7aacb4 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java @@ -306,7 +306,7 @@ public class BulkChangeAction implements IssuesWsAction { .setNewStatus(issue.status()) .setNewResolution(issue.resolution()) .setAssignee(assignee.map(u -> new User(u.getUuid(), u.getLogin(), u.getName())).orElse(null)) - .setRule(new IssuesChangesNotificationBuilder.Rule(ruleDefinitionDto.getKey(), ruleDefinitionDto.getName())) + .setRule(new IssuesChangesNotificationBuilder.Rule(ruleDefinitionDto.getKey(), RuleType.valueOfNullable(ruleDefinitionDto.getType()), ruleDefinitionDto.getName())) .setProject(new Project.Builder(projectDto.uuid()) .setKey(projectDto.getKey()) .setProjectName(projectDto.name()) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/IssueUpdater.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/IssueUpdater.java index c393ce34697..0e01c13fa7e 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/IssueUpdater.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/IssueUpdater.java @@ -24,6 +24,7 @@ import java.util.Optional; import javax.annotation.Nullable; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; +import org.sonar.api.rules.RuleType; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.util.stream.MoreCollectors; @@ -112,7 +113,7 @@ public class IssueUpdater { .setNewResolution(issue.resolution()) .setNewStatus(issue.status()) .setAssignee(assignee.map(assigneeDto -> new User(assigneeDto.getUuid(), assigneeDto.getLogin(), assigneeDto.getName())).orElse(null)) - .setRule(rule.map(r -> new Rule(r.getKey(), r.getName())).get()) + .setRule(rule.map(r -> new Rule(r.getKey(), RuleType.valueOfNullable(r.getType()), r.getName())).get()) .setProject(new Project.Builder(project.uuid()) .setKey(project.getKey()) .setProjectName(project.name()) diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties index 4d815040df9..34fb7b43a10 100644 --- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties +++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties @@ -1608,7 +1608,7 @@ email_configuration.test.email_was_sent_to_x=Email was sent to {0} #------------------------------------------------------------------------------ notification.channel.EmailNotificationChannel=Email notification.dispatcher.information=A notification is never sent to the author of the event. -notification.dispatcher.ChangesOnMyIssue=Changes in issues assigned to me +notification.dispatcher.ChangesOnMyIssue=Changes in issues/hotspots assigned to me notification.dispatcher.NewIssues=New issues notification.dispatcher.NewAlerts=New quality gate status notification.dispatcher.NewFalsePositiveIssue=Issues resolved as false positive or won't fix diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/rules/RuleType.java b/sonar-plugin-api/src/main/java/org/sonar/api/rules/RuleType.java index a8012e26cf6..69dca215e83 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/rules/RuleType.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/rules/RuleType.java @@ -61,7 +61,7 @@ public enum RuleType { } throw new IllegalArgumentException(format("Unsupported type value : %d", dbConstant)); } - + @CheckForNull public static RuleType valueOfNullable(int dbConstant) { // iterating the array is fast-enough as size is small. No need for a map. @@ -75,5 +75,4 @@ public enum RuleType { } throw new IllegalArgumentException(format("Unsupported type value : %d", dbConstant)); } - }