diff options
author | Simon Brandhof <simon.brandhof@gmail.com> | 2013-07-30 16:38:00 +0200 |
---|---|---|
committer | Simon Brandhof <simon.brandhof@gmail.com> | 2013-07-30 16:51:12 +0200 |
commit | fcae413fe38ec35d9f12664b6b644c34626f4d92 (patch) | |
tree | a49e0dbbc3cbb016aafaa31d20c542173cbfc89e /sonar-core/src | |
parent | 57e19a8e1d24d2ec197b9d75d003bc30a256318e (diff) | |
download | sonarqube-fcae413fe38ec35d9f12664b6b644c34626f4d92.tar.gz sonarqube-fcae413fe38ec35d9f12664b6b644c34626f4d92.zip |
SONAR-4540 Notifications should not be sent on changes of issue line, author or effort to fix
Diffstat (limited to 'sonar-core/src')
4 files changed, 60 insertions, 25 deletions
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java index 93d67e5c62d..1a96d48446d 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java @@ -88,31 +88,31 @@ public class IssueNotifications implements BatchComponent, ServerComponent { @CheckForNull private Notification createChangeNotification(DefaultIssue issue, IssueChangeContext context, Rule rule, Component project, @Nullable Component component, @Nullable String comment) { - FieldDiffs currentChange = issue.currentChange(); - if (comment == null && (currentChange == null || currentChange.diffs().isEmpty())) { - return null; - } - Notification notification = newNotification(project, "issue-changes"); - notification.setFieldValue("key", issue.key()); - notification.setFieldValue("changeAuthor", context.login()); - notification.setFieldValue("reporter", issue.reporter()); - notification.setFieldValue("assignee", issue.assignee()); - notification.setFieldValue("message", issue.message()); - notification.setFieldValue("ruleName", ruleName(rule)); - notification.setFieldValue("componentKey", issue.componentKey()); - if (component != null) { - notification.setFieldValue("componentName", component.longName()); - } - if (comment != null) { - notification.setFieldValue("comment", comment); - } + Notification notification = null; + if (comment != null || issue.mustSendNotifications()) { + FieldDiffs currentChange = issue.currentChange(); + notification = newNotification(project, "issue-changes"); + notification.setFieldValue("key", issue.key()); + notification.setFieldValue("changeAuthor", context.login()); + notification.setFieldValue("reporter", issue.reporter()); + notification.setFieldValue("assignee", issue.assignee()); + notification.setFieldValue("message", issue.message()); + notification.setFieldValue("ruleName", ruleName(rule)); + notification.setFieldValue("componentKey", issue.componentKey()); + if (component != null) { + notification.setFieldValue("componentName", component.longName()); + } + if (comment != null) { + notification.setFieldValue("comment", comment); + } - if (currentChange != null) { - for (Map.Entry<String, FieldDiffs.Diff> entry : currentChange.diffs().entrySet()) { - String type = entry.getKey(); - FieldDiffs.Diff diff = entry.getValue(); - notification.setFieldValue("old." + type, diff.oldValue() != null ? diff.oldValue().toString() : null); - notification.setFieldValue("new." + type, diff.newValue() != null ? diff.newValue().toString() : null); + if (currentChange != null) { + for (Map.Entry<String, FieldDiffs.Diff> entry : currentChange.diffs().entrySet()) { + String type = entry.getKey(); + FieldDiffs.Diff diff = entry.getValue(); + notification.setFieldValue("old." + type, diff.oldValue() != null ? diff.oldValue().toString() : null); + notification.setFieldValue("new." + type, diff.newValue() != null ? diff.newValue().toString() : null); + } } } return notification; diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java index a91d401e0df..3c5a80c53a0 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java @@ -63,6 +63,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { issue.setManualSeverity(true); issue.setUpdateDate(context.date()); issue.setChanged(true); + issue.setSendNotifications(true); return true; } return false; @@ -75,6 +76,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { issue.setAssignee(sanitizedAssignee); issue.setUpdateDate(context.date()); issue.setChanged(true); + issue.setSendNotifications(true); return true; } return false; @@ -101,6 +103,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { issue.setResolution(resolution); issue.setUpdateDate(context.date()); issue.setChanged(true); + issue.setSendNotifications(true); return true; } return false; @@ -112,6 +115,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { issue.setStatus(status); issue.setUpdateDate(context.date()); issue.setChanged(true); + issue.setSendNotifications(true); return true; } return false; @@ -123,6 +127,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { issue.setAuthorLogin(authorLogin); issue.setUpdateDate(context.date()); issue.setChanged(true); + // do not send notifications to prevent spam when installing the developer cockpit plugin return true; } return false; @@ -163,6 +168,8 @@ public class IssueUpdater implements BatchComponent, ServerComponent { issue.setEffortToFix(d); issue.setUpdateDate(context.date()); issue.setChanged(true); + // Do not send notifications to prevent spam when installing the SQALE plugin, + // and do not complete the changelog (for the moment) return true; } return false; @@ -193,6 +200,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { issue.setActionPlanKey(sanitizedActionPlanKey); issue.setUpdateDate(context.date()); issue.setChanged(true); + issue.setSendNotifications(true); return true; } return false; diff --git a/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java index 9f85faa3be9..f0b464fb987 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java @@ -76,6 +76,7 @@ public class IssueNotificationsTest { .setAssignee("freddy") .setFieldChange(context, "resolution", null, "FIXED") .setFieldChange(context, "status", "OPEN", "RESOLVED") + .setSendNotifications(true) .setComponentKey("struts:Action") .setProjectKey("struts"); DefaultIssueQueryResult queryResult = new DefaultIssueQueryResult(Arrays.<Issue>asList(issue)); diff --git a/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java index ade4e76cbd2..63f7ac1483e 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java @@ -39,6 +39,7 @@ public class IssueUpdaterTest { boolean updated = updater.assign(issue, "emmerik", context); assertThat(updated).isTrue(); assertThat(issue.assignee()).isEqualTo("emmerik"); + assertThat(issue.mustSendNotifications()).isTrue(); FieldDiffs.Diff diff = issue.currentChange().get("assignee"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("emmerik"); @@ -50,6 +51,7 @@ public class IssueUpdaterTest { boolean updated = updater.assign(issue, null, context); assertThat(updated).isTrue(); assertThat(issue.assignee()).isNull(); + assertThat(issue.mustSendNotifications()).isTrue(); FieldDiffs.Diff diff = issue.currentChange().get("assignee"); assertThat(diff.oldValue()).isEqualTo("morgan"); assertThat(diff.newValue()).isNull(); @@ -61,6 +63,7 @@ public class IssueUpdaterTest { boolean updated = updater.assign(issue, "emmerik", context); assertThat(updated).isTrue(); assertThat(issue.assignee()).isEqualTo("emmerik"); + assertThat(issue.mustSendNotifications()).isTrue(); FieldDiffs.Diff diff = issue.currentChange().get("assignee"); assertThat(diff.oldValue()).isEqualTo("morgan"); assertThat(diff.newValue()).isEqualTo("emmerik"); @@ -72,6 +75,7 @@ public class IssueUpdaterTest { boolean updated = updater.assign(issue, "morgan", context); assertThat(updated).isFalse(); assertThat(issue.currentChange()).isNull(); + assertThat(issue.mustSendNotifications()).isFalse(); } @@ -81,6 +85,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.severity()).isEqualTo("BLOCKER"); assertThat(issue.manualSeverity()).isFalse(); + assertThat(issue.mustSendNotifications()).isFalse(); FieldDiffs.Diff diff = issue.currentChange().get("severity"); assertThat(diff.oldValue()).isNull(); @@ -93,6 +98,7 @@ public class IssueUpdaterTest { boolean updated = updater.setPastSeverity(issue, "INFO", context); assertThat(updated).isTrue(); assertThat(issue.severity()).isEqualTo("BLOCKER"); + assertThat(issue.mustSendNotifications()).isFalse(); FieldDiffs.Diff diff = issue.currentChange().get("severity"); assertThat(diff.oldValue()).isEqualTo("INFO"); @@ -106,6 +112,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.severity()).isEqualTo("MINOR"); + assertThat(issue.mustSendNotifications()).isFalse(); FieldDiffs.Diff diff = issue.currentChange().get("severity"); assertThat(diff.oldValue()).isEqualTo("BLOCKER"); assertThat(diff.newValue()).isEqualTo("MINOR"); @@ -116,6 +123,7 @@ public class IssueUpdaterTest { issue.setSeverity("MINOR"); boolean updated = updater.setSeverity(issue, "MINOR", context); assertThat(updated).isFalse(); + assertThat(issue.mustSendNotifications()).isFalse(); assertThat(issue.currentChange()).isNull(); } @@ -137,6 +145,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.severity()).isEqualTo("MINOR"); assertThat(issue.manualSeverity()).isTrue(); + assertThat(issue.mustSendNotifications()).isTrue(); FieldDiffs.Diff diff = issue.currentChange().get("severity"); assertThat(diff.oldValue()).isEqualTo("BLOCKER"); assertThat(diff.newValue()).isEqualTo("MINOR"); @@ -148,6 +157,7 @@ public class IssueUpdaterTest { boolean updated = updater.setManualSeverity(issue, "MINOR", context); assertThat(updated).isFalse(); assertThat(issue.currentChange()).isNull(); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -155,7 +165,7 @@ public class IssueUpdaterTest { boolean updated = updater.setLine(issue, 123); assertThat(updated).isTrue(); assertThat(issue.line()).isEqualTo(123); - + assertThat(issue.mustSendNotifications()).isFalse(); // do not save change assertThat(issue.currentChange()).isNull(); } @@ -166,6 +176,7 @@ public class IssueUpdaterTest { boolean updated = updater.setPastLine(issue, 123); assertThat(updated).isTrue(); assertThat(issue.line()).isEqualTo(42); + assertThat(issue.mustSendNotifications()).isFalse(); // do not save change assertThat(issue.currentChange()).isNull(); @@ -178,6 +189,7 @@ public class IssueUpdaterTest { assertThat(updated).isFalse(); assertThat(issue.line()).isEqualTo(123); assertThat(issue.currentChange()).isNull(); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -189,6 +201,7 @@ public class IssueUpdaterTest { FieldDiffs.Diff diff = issue.currentChange().get("resolution"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("OPEN"); + assertThat(issue.mustSendNotifications()).isTrue(); } @Test @@ -198,6 +211,7 @@ public class IssueUpdaterTest { assertThat(updated).isFalse(); assertThat(issue.resolution()).isEqualTo("FIXED"); assertThat(issue.currentChange()).isNull(); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -209,6 +223,7 @@ public class IssueUpdaterTest { FieldDiffs.Diff diff = issue.currentChange().get("status"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("OPEN"); + assertThat(issue.mustSendNotifications()).isTrue(); } @Test @@ -218,6 +233,7 @@ public class IssueUpdaterTest { assertThat(updated).isFalse(); assertThat(issue.status()).isEqualTo("CLOSED"); assertThat(issue.currentChange()).isNull(); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -228,6 +244,7 @@ public class IssueUpdaterTest { assertThat(issue.currentChange().diffs()).hasSize(1); assertThat(issue.currentChange().get("JIRA").oldValue()).isNull(); assertThat(issue.currentChange().get("JIRA").newValue()).isEqualTo("FOO-123"); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -239,6 +256,7 @@ public class IssueUpdaterTest { assertThat(issue.currentChange().diffs()).hasSize(1); assertThat(issue.currentChange().get("JIRA").oldValue()).isEqualTo("FOO-123"); assertThat(issue.currentChange().get("JIRA").newValue()).isNull(); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -246,6 +264,7 @@ public class IssueUpdaterTest { issue.setAttribute("JIRA", "FOO-123"); boolean updated = updater.setAttribute(issue, "JIRA", "FOO-123", context); assertThat(updated).isFalse(); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -257,6 +276,7 @@ public class IssueUpdaterTest { FieldDiffs.Diff diff = issue.currentChange().get("actionPlanKey"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("ABCD"); + assertThat(issue.mustSendNotifications()).isTrue(); } @Test @@ -265,6 +285,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.isChanged()).isTrue(); assertThat(issue.effortToFix()).isEqualTo(3.14); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -274,6 +295,7 @@ public class IssueUpdaterTest { assertThat(updated).isFalse(); assertThat(issue.isChanged()).isFalse(); assertThat(issue.effortToFix()).isEqualTo(3.14); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -285,6 +307,7 @@ public class IssueUpdaterTest { // do not save change assertThat(issue.currentChange()).isNull(); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -293,6 +316,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.isChanged()).isTrue(); assertThat(issue.message()).isEqualTo("the message"); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -304,6 +328,7 @@ public class IssueUpdaterTest { // do not save change assertThat(issue.currentChange()).isNull(); + assertThat(issue.mustSendNotifications()).isFalse(); } @Test @@ -315,5 +340,6 @@ public class IssueUpdaterTest { FieldDiffs.Diff diff = issue.currentChange().get("author"); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("eric"); + assertThat(issue.mustSendNotifications()).isFalse(); } } |