SONAR-4525 Use names instead of keys for action plans and users in issue changes...

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplate.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplate.java
index 9c689f3c0e4b70c3fdcb0f0cdd85d7b473591c72..4f59bce24ad0cf50eaa8e37d478da60f8e7d54e9 100644 (file)
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplate.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplate.java
@@ -75,13 +75,13 @@ public class IssueChangesEmailTemplate extends EmailTemplate {
 
   private void appendChanges(Notification notif, StringBuilder sb) {
     appendField(sb, "Comment", null, notif.getFieldValue("comment"));
-    appendField(sb, "Assignee", notif.getFieldValue("old.assignee"), notif.getFieldValue("new.assignee"));
+    appendFieldWithoutHistory(sb, "Assignee", notif.getFieldValue("old.assignee"), notif.getFieldValue("new.assignee"));
     appendField(sb, "Severity", notif.getFieldValue("old.severity"), notif.getFieldValue("new.severity"));
     appendField(sb, "Resolution", notif.getFieldValue("old.resolution"), notif.getFieldValue("new.resolution"));
     appendField(sb, "Status", notif.getFieldValue("old.status"), notif.getFieldValue("new.status"));
     appendField(sb, "Message", notif.getFieldValue("old.message"), notif.getFieldValue("new.message"));
     appendField(sb, "Author", notif.getFieldValue("old.author"), notif.getFieldValue("new.author"));
-    appendField(sb, "Action Plan", notif.getFieldValue("old.actionPlanKey"), notif.getFieldValue("new.actionPlanKey"));
+    appendFieldWithoutHistory(sb, "Action Plan", notif.getFieldValue("old.actionPlan"), notif.getFieldValue("new.actionPlan")) ;
   }
 
   private void appendHeader(Notification notif, StringBuilder sb) {
@@ -114,6 +114,19 @@ public class IssueChangesEmailTemplate extends EmailTemplate {
     }
   }
 
+  private void appendFieldWithoutHistory(StringBuilder sb, String name, @Nullable String oldValue, @Nullable String newValue) {
+    if (oldValue != null || newValue != null) {
+      sb.append(name);
+      if (newValue != null) {
+        sb.append(" changed to ");
+        sb.append(newValue);
+      } else {
+        sb.append(" removed");
+      }
+      sb.append(NEW_LINE);
+    }
+  }
+
   private String getUserFullName(@Nullable String login) {
     if (login == null) {
       return null;

diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest.java
index 64384c40f39c2d6d8d1dbfef29802bfb10b2040a..bcf6225e5c50e91bc27914a60304cb842a891d0c 100644 (file)
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest.java
@@ -59,14 +59,42 @@ public class IssueChangesEmailTemplateTest {
   }
 
   @Test
-  public void test_email_with_changes() {
-    Notification notification = new Notification("issue-changes")
-      .setFieldValue("projectName", "Struts")
-      .setFieldValue("projectKey", "org.apache:struts")
-      .setFieldValue("componentName", "org.apache.struts.Action")
-      .setFieldValue("key", "ABCDE")
-      .setFieldValue("ruleName", "Avoid Cycles")
-      .setFieldValue("message", "Has 3 cycles")
+  public void email_should_display_assignee_change() throws Exception {
+    Notification notification = generateNotification()
+      .setFieldValue("old.assignee", "simon")
+      .setFieldValue("new.assignee", "louis");
+
+    EmailMessage email = template.format(notification);
+    assertThat(email.getMessageId()).isEqualTo("issue-changes/ABCDE");
+    assertThat(email.getSubject()).isEqualTo("Struts, change on issue #ABCDE");
+
+    String message = email.getMessage();
+    String expectedMessage = TestUtils.getResourceContent("/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_assignee_change.txt");
+    expectedMessage = StringUtils.remove(expectedMessage, '\r');
+    assertThat(message).isEqualTo(expectedMessage);
+    assertThat(email.getFrom()).isNull();
+  }
+
+  @Test
+  public void email_should_display_plan_change() throws Exception {
+    Notification notification = generateNotification()
+      .setFieldValue("old.actionPlan", null)
+      .setFieldValue("new.actionPlan", "ABC 1.0");
+
+    EmailMessage email = template.format(notification);
+    assertThat(email.getMessageId()).isEqualTo("issue-changes/ABCDE");
+    assertThat(email.getSubject()).isEqualTo("Struts, change on issue #ABCDE");
+
+    String message = email.getMessage();
+    String expectedMessage = TestUtils.getResourceContent("/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_action_plan_change.txt");
+    expectedMessage = StringUtils.remove(expectedMessage, '\r');
+    assertThat(message).isEqualTo(expectedMessage);
+    assertThat(email.getFrom()).isNull();
+  }
+
+  @Test
+  public void test_email_with_multiple_changes() {
+    Notification notification = generateNotification()
       .setFieldValue("comment", "How to fix it?")
       .setFieldValue("old.assignee", "simon")
       .setFieldValue("new.assignee", "louis")
@@ -78,7 +106,7 @@ public class IssueChangesEmailTemplateTest {
     assertThat(email.getSubject()).isEqualTo("Struts, change on issue #ABCDE");
 
     String message = email.getMessage();
-    String expectedMessage = TestUtils.getResourceContent("/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_changes.txt");
+    String expectedMessage = TestUtils.getResourceContent("/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_multiple_changes.txt");
     expectedMessage = StringUtils.remove(expectedMessage, '\r');
     assertThat(message).isEqualTo(expectedMessage);
     assertThat(email.getFrom()).isNull();
@@ -99,4 +127,14 @@ public class IssueChangesEmailTemplateTest {
     assertThat(message.getFrom()).isEqualTo("Simon");
   }
 
+  private Notification generateNotification() {
+    Notification notification = new Notification("issue-changes")
+      .setFieldValue("projectName", "Struts")
+      .setFieldValue("projectKey", "org.apache:struts")
+      .setFieldValue("componentName", "org.apache.struts.Action")
+      .setFieldValue("key", "ABCDE")
+      .setFieldValue("ruleName", "Avoid Cycles")
+      .setFieldValue("message", "Has 3 cycles");
+    return notification;
+  }
 }

diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_action_plan_change.txt b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_action_plan_change.txt
new file mode 100644 (file)
index 0000000..d1230a1
--- /dev/null
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_action_plan_change.txt
@@ -0,0 +1,7 @@
+org.apache.struts.Action
+Rule: Avoid Cycles
+Message: Has 3 cycles
+
+Action Plan changed to ABC 1.0
+
+See it in SonarQube: http://nemo.sonarsource.org/issue/show/ABCDE

diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_assignee_change.txt b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_assignee_change.txt
new file mode 100644 (file)
index 0000000..8cc9ab6
--- /dev/null
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_assignee_change.txt
@@ -0,0 +1,7 @@
+org.apache.struts.Action
+Rule: Avoid Cycles
+Message: Has 3 cycles
+
+Assignee changed to louis
+
+See it in SonarQube: http://nemo.sonarsource.org/issue/show/ABCDE

diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_changes.txt b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_changes.txt
deleted file mode 100644 (file)
index d6f50cc..0000000
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_changes.txt
+++ /dev/null
@@ -1,10 +0,0 @@
-org.apache.struts.Action
-Rule: Avoid Cycles
-Message: Has 3 cycles
-
-Comment: How to fix it?
-Assignee: louis (was simon)
-Resolution: FALSE-POSITIVE
-Status: RESOLVED
-
-See it in SonarQube: http://nemo.sonarsource.org/issue/show/ABCDE

diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_multiple_changes.txt b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_multiple_changes.txt
new file mode 100644 (file)
index 0000000..995de70
--- /dev/null
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest/email_with_multiple_changes.txt
@@ -0,0 +1,10 @@
+org.apache.struts.Action
+Rule: Avoid Cycles
+Message: Has 3 cycles
+
+Comment: How to fix it?
+Assignee changed to louis
+Resolution: FALSE-POSITIVE
+Status: RESOLVED
+
+See it in SonarQube: http://nemo.sonarsource.org/issue/show/ABCDE

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 f0b464fb987495b11a05bec5855f63f4da476e71..7f9e521118e6253fb1d686b01c5b75517bc3f229 100644 (file)
--- a/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java
@@ -129,4 +129,26 @@ public class IssueNotificationsTest {
     assertThat(notification).isNull();
     Mockito.verifyZeroInteractions(manager);
   }
+
+  @Test
+  public void should_store_changelog_in_notification() throws Exception {
+    IssueChangeContext context = IssueChangeContext.createScan(new Date());
+
+    DefaultIssue issue = new DefaultIssue()
+      .setMessage("the message")
+      .setKey("ABCDE")
+      .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));
+    queryResult.addProjects(Arrays.<Component>asList(new Project("struts")));
+
+    Notification notification = issueNotifications.sendChanges(issue, context, queryResult);
+
+    assertThat(notification.getFieldValue("changeLog")).contains("Resolution: FIXED");
+    assertThat(notification.getFieldValue("changeLog")).contains("Status: RESOLVED (was OPEN)");
+  }
 }

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java b/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java
index 735cfed12e778ad40f282000c1f06816ea7ced53..808d8690f75bece05bf0c40f5a76e63a2200a476 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java
@@ -41,6 +41,8 @@ public class AssignAction extends Action implements ServerComponent {
   private final UserFinder userFinder;
   private final IssueUpdater issueUpdater;
 
+  private User assignee;
+
   public AssignAction(UserFinder userFinder, IssueUpdater issueUpdater) {
     super(KEY);
     this.userFinder = userFinder;
@@ -50,8 +52,8 @@ public class AssignAction extends Action implements ServerComponent {
 
   @Override
   public boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession){
-    String assignee = assignee(properties);
-    if (!Strings.isNullOrEmpty(assignee) && userFinder.findByLogin(assignee) == null) {
+    String assignee = assigneeValue(properties);
+    if (!Strings.isNullOrEmpty(assignee) && getOrSelectUser(assignee) == null) {
       throw new IllegalArgumentException("Unknown user: " + assignee);
     }
     return true;
@@ -59,11 +61,18 @@ public class AssignAction extends Action implements ServerComponent {
 
   @Override
   public boolean execute(Map<String, Object> properties, Context context) {
-    User user = userFinder.findByLogin(assignee(properties));
-    return issueUpdater.assign((DefaultIssue) context.issue(), user, context.issueChangeContext());
+    String assignee = assigneeValue(properties);
+    return issueUpdater.assign((DefaultIssue) context.issue(), getOrSelectUser(assignee), context.issueChangeContext());
   }
 
-  private String assignee(Map<String, Object> properties){
+  private String assigneeValue(Map<String, Object> properties) {
     return (String) properties.get("assignee");
   }
+
+  private User getOrSelectUser(String assigneeKey) {
+    if(assignee == null) {
+      assignee = userFinder.findByLogin(assigneeKey);
+    }
+    return assignee;
+  }
 }
\ No newline at end of file

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java b/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java
index b357474775ef5e3f559afcf6ac6685baeba07730..2ff701d364eef482077de8900cf3d505752ade23 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java
@@ -40,6 +40,8 @@ public class PlanAction extends Action implements ServerComponent {
   private final ActionPlanService actionPlanService;
   private final IssueUpdater issueUpdater;
 
+  private ActionPlan actionPlan;
+
   public PlanAction(ActionPlanService actionPlanService, IssueUpdater issueUpdater) {
     super(KEY);
     this.actionPlanService = actionPlanService;
@@ -49,11 +51,11 @@ public class PlanAction extends Action implements ServerComponent {
 
   @Override
   public boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession) {
-    String actionPlanKey = planKey(properties);
-    if (!Strings.isNullOrEmpty(actionPlanKey)) {
-      ActionPlan actionPlan = actionPlanService.findByKey(actionPlanKey, userSession);
+    String actionPlanValue = planValue(properties);
+    if (!Strings.isNullOrEmpty(actionPlanValue)) {
+      ActionPlan actionPlan = getOrSelectActionPlan(actionPlanValue, userSession);
       if (actionPlan == null) {
-        throw new IllegalArgumentException("Unknown action plan: " + actionPlanKey);
+        throw new IllegalArgumentException("Unknown action plan: " + actionPlanValue);
       }
       verifyIssuesAreAllRelatedOnActionPlanProject(issues, actionPlan);
     }
@@ -62,11 +64,11 @@ public class PlanAction extends Action implements ServerComponent {
 
   @Override
   public boolean execute(Map<String, Object> properties, Context context) {
-    ActionPlan actionPlan = actionPlanService.findByKey(planKey(properties), UserSession.get());
-    return issueUpdater.plan((DefaultIssue) context.issue(), actionPlan, context.issueChangeContext());
+    String actionPlan = planValue(properties);
+    return issueUpdater.plan((DefaultIssue) context.issue(), getOrSelectActionPlan(actionPlan, UserSession.get()), context.issueChangeContext());
   }
 
-  private String planKey(Map<String, Object> properties) {
+  private String planValue(Map<String, Object> properties) {
     return (String) properties.get("plan");
   }
 
@@ -81,4 +83,10 @@ public class PlanAction extends Action implements ServerComponent {
     }
   }
 
+  private ActionPlan getOrSelectActionPlan(String planValue, UserSession userSession) {
+    if(actionPlan == null) {
+      actionPlan = actionPlanService.findByKey(planValue, userSession);
+    }
+    return actionPlan;
+  }
 }
\ No newline at end of file