fix(appOwner): convert app owner emails to html (#169)

spinnaker · Sep 1, 2017 · 7865051 · 7865051
1 parent 9aedf00
commit 7865051
Show file tree

Hide file tree

Showing 6 changed files with 92 additions and 76 deletions.
diff --git a/echo-notifications/src/main/resources/templates/appOwnerMultipleUsers/body-email.ftl b/echo-notifications/src/main/resources/templates/appOwnerMultipleUsers/body-email.ftl
@@ -1,7 +1,8 @@
-The owner email for the following applications matches multiple users:
-
-<#foreach event in notification.additionalContext.events>${event.email} owns...
-<#foreach application in event.applications>- ${application.name}
+<p>The owner email for the following applications matches multiple users:</p>
+<#foreach event in notification.additionalContext.events>
+${event.email} owns...
+<ul>
+  <#foreach application in event.applications><li>${application.name}</li></#foreach>
+</ul>
+<br/>
 </#foreach>
-
-</#foreach>
diff --git a/echo-notifications/src/main/resources/templates/appOwnerOutdatedNoSuggestion/body-email.ftl b/echo-notifications/src/main/resources/templates/appOwnerOutdatedNoSuggestion/body-email.ftl
@@ -1,7 +1,8 @@
-The following users are ex-employees but still own applications. We could not identify managers or team members to reassign to.
-
-<#foreach event in notification.additionalContext.events>${event.user.name}<${event.user.email}> owns...
-<#foreach application in event.applications>- ${application.name}
+<p>The following users are ex-employees but still own applications. We could not identify managers or team members to reassign to.</p>
+<#foreach event in notification.additionalContext.events>
+${event.user.name} (${event.user.email}) owns...
+<ul>
+  <#foreach application in event.applications><li>${application.name}</li></#foreach>
+</ul>
+<br/>
 </#foreach>
-
-</#foreach>
diff --git a/echo-notifications/src/main/resources/templates/appOwnerOutdatedReassign/body-email.ftl b/echo-notifications/src/main/resources/templates/appOwnerOutdatedReassign/body-email.ftl
@@ -1,7 +1,9 @@
-The following users are ex-employees but still own applications. We have identified possible managers or team members to reassign to.
-
-<#foreach event in notification.additionalContext.events>${event.user.name}<${event.user.email}> owns...
-<#foreach application in event.applications>- ${application.name}
-</#foreach>Suggest reassigning to ${event.suggestedUser.name}<${event.suggestedUser.email}>
-
-</#foreach>
+<p>The following users are ex-employees but still own applications. We have identified possible managers or team members to reassign to.</p>
+<#foreach event in notification.additionalContext.events>
+${event.user.name} (${event.user.email}) owns...
+<ul>
+  <#foreach application in event.applications><li>${application.name}</li></#foreach>
+</ul>
+<p>Suggest reassigning to ${event.suggestedUser.name} (${event.suggestedUser.email})</p>
+<br/>
+</#foreach>
diff --git a/echo-notifications/src/main/resources/templates/appOwnerUnknown/body-email.ftl b/echo-notifications/src/main/resources/templates/appOwnerUnknown/body-email.ftl
@@ -1,7 +1,8 @@
-The following email addresses are not recognized employee or group addresses but own applications.
-
-<#foreach event in notification.additionalContext.events>${event.email} owns...
-<#foreach application in event.applications>- ${application.name}
+<p>The following email addresses are not recognized employee or group addresses but own applications.</p>
+<#foreach event in notification.additionalContext.events>
+${event.email} owns...
+<ul>
+  <#foreach application in event.applications><li>${application.name}</li></#foreach>
+</ul>
+<br/>
 </#foreach>
-
-</#foreach>
diff --git a/echo-notifications/src/main/resources/templates/appsReassigned/body-email.ftl b/echo-notifications/src/main/resources/templates/appsReassigned/body-email.ftl
@@ -1,7 +1,8 @@
-The following applications have been reassigned to you as their owner is no longer a Netflix employee and you are the current manager of their team.
-
-<#foreach event in notification.additionalContext.events>${event.user.name}<${event.user.email}> owned...
-<#foreach application in event.applications>- ${application.name}
+<p>The following applications have been reassigned to you as their owner is no longer a Netflix employee and you are the current manager of their team.</p>
+<#foreach event in notification.additionalContext.events>
+${event.user.name} (${event.user.email}) owned...
+<ul>
+  <#foreach application in event.applications><li>${application.name}</li></#foreach>
+</ul>
+<br/>
 </#foreach>
-
-</#foreach>
diff --git a/...tions/src/test/groovy/com/netflix/spinnaker/echo/notification/AppOwnerTemplateTest.groovy b/...tions/src/test/groovy/com/netflix/spinnaker/echo/notification/AppOwnerTemplateTest.groovy
@@ -45,7 +45,7 @@ class AppOwnerTemplateTest extends Specification {
         not.additionalContext.events = events
 
         when:
-        def rendered = notificationTemplateEngine.build(not, NotificationTemplateEngine.Type.BODY)
+        def rendered = notificationTemplateEngine.build(not, NotificationTemplateEngine.Type.BODY).stripIndent()
 
         then:
         rendered == expected
@@ -63,15 +63,17 @@ class AppOwnerTemplateTest extends Specification {
 
         ]
         expected = '''\
-        The owner email for the following applications matches multiple users:
-        
+        <p>The owner email for the following applications matches multiple users:</p>
         user1@host.net owns...
-        - oneapp
-        
+        <ul>
+          <li>oneapp</li>
+        </ul>
+        <br/>
         user2@host.net owns...
-        - oneapp
-        - twoapp
-        
+        <ul>
+          <li>oneapp</li><li>twoapp</li>
+        </ul>
+        <br/>
         '''.stripIndent()
     }
 
@@ -100,15 +102,17 @@ class AppOwnerTemplateTest extends Specification {
 
         ]
         expected = '''\
-        The following users are ex-employees but still own applications. We could not identify managers or team members to reassign to.
-        
-        User 1<user1@host.net> owns...
-        - oneapp
-        
-        User 2<user2@host.net> owns...
-        - oneapp
-        - twoapp
-        
+        <p>The following users are ex-employees but still own applications. We could not identify managers or team members to reassign to.</p>
+        User 1 (user1@host.net) owns...
+        <ul>
+          <li>oneapp</li>
+        </ul>
+        <br/>
+        User 2 (user2@host.net) owns...
+        <ul>
+          <li>oneapp</li><li>twoapp</li>
+        </ul>
+        <br/>
         '''.stripIndent()
     }
 
@@ -139,17 +143,19 @@ class AppOwnerTemplateTest extends Specification {
 
         ]
         expected = '''\
-        The following users are ex-employees but still own applications. We have identified possible managers or team members to reassign to.
-        
-        User 1<user1@host.net> owns...
-        - oneapp
-        Suggest reassigning to Mgr 1<manager1@host.net>
-        
-        User 2<user2@host.net> owns...
-        - oneapp
-        - twoapp
-        Suggest reassigning to Mgr 2<manager2@host.net>
-        
+        <p>The following users are ex-employees but still own applications. We have identified possible managers or team members to reassign to.</p>
+        User 1 (user1@host.net) owns...
+        <ul>
+          <li>oneapp</li>
+        </ul>
+        <p>Suggest reassigning to Mgr 1 (manager1@host.net)</p>
+        <br/>
+        User 2 (user2@host.net) owns...
+        <ul>
+          <li>oneapp</li><li>twoapp</li>
+        </ul>
+        <p>Suggest reassigning to Mgr 2 (manager2@host.net)</p>
+        <br/>
         '''.stripIndent()
     }
 
@@ -178,15 +184,17 @@ class AppOwnerTemplateTest extends Specification {
 
         ]
         expected = '''\
-        The following email addresses are not recognized employee or group addresses but own applications.
-        
+        <p>The following email addresses are not recognized employee or group addresses but own applications.</p>
         user1@host.net owns...
-        - oneapp
-        
+        <ul>
+          <li>oneapp</li>
+        </ul>
+        <br/>
         user2@host.net owns...
-        - oneapp
-        - twoapp
-        
+        <ul>
+          <li>oneapp</li><li>twoapp</li>
+        </ul>
+        <br/>
         '''.stripIndent()
     }
 
@@ -215,15 +223,17 @@ class AppOwnerTemplateTest extends Specification {
 
         ]
         expected = '''\
-        The following applications have been reassigned to you as their owner is no longer a Netflix employee and you are the current manager of their team.
-        
-        User 1<user1@host.net> owned...
-        - oneapp
-        
-        User 2<user2@host.net> owned...
-        - oneapp
-        - twoapp
-        
+        <p>The following applications have been reassigned to you as their owner is no longer a Netflix employee and you are the current manager of their team.</p>
+        User 1 (user1@host.net) owned...
+        <ul>
+          <li>oneapp</li>
+        </ul>
+        <br/>
+        User 2 (user2@host.net) owned...
+        <ul>
+          <li>oneapp</li><li>twoapp</li>
+        </ul>
+        <br/>
         '''.stripIndent()
     }
     Notification buildFullNotification() {
@@ -240,4 +250,4 @@ class AppOwnerTemplateTest extends Specification {
         return notification
     }
 
-}
+}