aboutsummaryrefslogtreecommitdiffstats
path: root/sonar-core/src
diff options
context:
space:
mode:
authorSimon Brandhof <simon.brandhof@gmail.com>2013-07-30 16:38:00 +0200
committerSimon Brandhof <simon.brandhof@gmail.com>2013-07-30 16:51:12 +0200
commitfcae413fe38ec35d9f12664b6b644c34626f4d92 (patch)
treea49e0dbbc3cbb016aafaa31d20c542173cbfc89e /sonar-core/src
parent57e19a8e1d24d2ec197b9d75d003bc30a256318e (diff)
downloadsonarqube-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')
-rw-r--r--sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java48
-rw-r--r--sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java8
-rw-r--r--sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java1
-rw-r--r--sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java28
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();
}
}