Add better (jinja-based) messaging to the notifications and add some fixes for the email templates

data/model/legacy.py

@@ -579,6 +579,13 @@ def get_user(username):
     return None
 
 
+def get_namespace_user(username):
+  try:
+    return User.get(User.username == username)
+  except User.DoesNotExist:
+    return None
+
+
 def get_user_or_org(username):
   try:
     return User.get(User.username == username, User.robot == False)

endpoints/notificationevent.py

@@ -1,7 +1,9 @@
 import logging
 
 from notificationhelper import build_event_data
+from util.jinjautil import get_template_env
 
+template_env = get_template_env("events")
 logger = logging.getLogger(__name__)
 
 class InvalidNotificationEventException(Exception):
@@ -14,7 +16,7 @@ class NotificationEvent(object):
   def get_level(self, event_data, notification_data):
     """
     Returns a 'level' representing the severity of the event.
-    Valid values are: 'info', 'warning', 'error', 'primary'
+    Valid values are: 'info', 'warning', 'error', 'primary', 'success'
     """
     raise NotImplementedError
 
@@ -28,7 +30,10 @@ class NotificationEvent(object):
     """
     Returns a human readable HTML message for the given notification data.
     """
-    raise NotImplementedError
+    return template_env.get_template(self.event_name() + '.html').render({
+      'event_data': event_data,
+      'notification_data': notification_data
+    })
 
   def get_sample_data(self, repository=None):
     """
@@ -59,28 +64,11 @@ class RepoPushEvent(NotificationEvent):
     return 'repo_push'
 
   def get_level(self, event_data, notification_data):
-    return 'info'
+    return 'primary'
 
   def get_summary(self, event_data, notification_data):
     return 'Repository %s updated' % (event_data['repository'])
 
-  def get_message(self, event_data, notification_data):
-    if not event_data.get('updated_tags', {}).keys():
-      html = """
-        Repository <a href="%s">%s</a> has been updated via a push.
-      """ % (event_data['homepage'],
-             event_data['repository'])
-    else:
-      html = """
-       Repository <a href="%s">%s</a> has been updated via a push.
-       <br><br>
-       Tags Updated: %s
-      """ % (event_data['homepage'],
-             event_data['repository'],
-             ', '.join(event_data['updated_tags'].keys()))
-
-    return html
-
   def get_sample_data(self, repository):
     return build_event_data(repository, {
       'updated_tags': {'latest': 'someimageid', 'foo': 'anotherimage'},
@@ -110,25 +98,6 @@ class BuildQueueEvent(NotificationEvent):
   def get_summary(self, event_data, notification_data):
     return 'Build queued for repository %s' % (event_data['repository'])      
 
-  def get_message(self, event_data, notification_data):
-    is_manual = event_data['is_manual']
-    if is_manual:
-      html = """
-      A <a href="%s">new build</a> has been manually queued to start on repository %s.
-      <br><br>
-      Build ID: %s
-      """ % (event_data['homepage'], event_data['repository'], event_data['build_id'])
-    else:
-      html = """
-      A <a href="%s">new build</a> has been queued via a %s trigger to start on repository %s.
-      <br><br>
-      Build ID: %s
-      """ % (event_data['homepage'], event_data['trigger_kind'],
-             event_data['repository'], event_data['build_id'])
-
-    return html
-      
-
 
 class BuildStartEvent(NotificationEvent):
   @classmethod
@@ -151,15 +120,6 @@ class BuildStartEvent(NotificationEvent):
   def get_summary(self, event_data, notification_data):
     return 'Build started for repository %s' % (event_data['repository'])
 
-  def get_message(self, event_data, notification_data):
-    html = """
-      A <a href="%s">new build</a> has started on repository %s.
-      <br><br>
-      Build ID: %s
-      """ % (event_data['homepage'], event_data['repository'], event_data['build_id'])
-
-    return html
-      
 
 class BuildSuccessEvent(NotificationEvent):
   @classmethod
@@ -167,7 +127,7 @@ class BuildSuccessEvent(NotificationEvent):
     return 'build_success'
 
   def get_level(self, event_data, notification_data):
-    return 'primary'
+    return 'success'
 
   def get_sample_data(self, repository):
     build_uuid = 'fake-build-id'
@@ -182,15 +142,6 @@ class BuildSuccessEvent(NotificationEvent):
   def get_summary(self, event_data, notification_data):
     return 'Build succeeded for repository %s' % (event_data['repository'])
 
-  def get_message(self, event_data, notification_data):
-    html = """
-      A <a href="%s">build</a> has finished on repository %s.
-      <br><br>
-      Build ID: %s
-      """ % (event_data['homepage'], event_data['repository'], event_data['build_id'])
-
-    return html
-
 
 class BuildFailureEvent(NotificationEvent):
   @classmethod
@@ -214,13 +165,3 @@ class BuildFailureEvent(NotificationEvent):
   def get_summary(self, event_data, notification_data):
     return 'Build failure for repository %s' % (event_data['repository'])
 
-  def get_message(self, event_data, notification_data):
-    html = """
-      A <a href="%s">build</a> has failed on repository %s.
-      <br><br>
-      Reason: %s<br>
-      Build ID: %s<br>
-      """ % (event_data['homepage'], event_data['repository'],
-             event_data['error_message'], event_data['build_id'])
-
-    return html

endpoints/notificationhelper.py

@@ -1,5 +1,6 @@
 from app import app, notification_queue
 from data import model
+from auth.auth_context import get_authenticated_user, get_validated_oauth_token
 
 import json
 
@@ -27,21 +28,37 @@ def build_event_data(repo, extra_data={}, subpage=None):
   event_data.update(extra_data)
   return event_data
 
-def build_notification_data(notification, event_data):
+def build_notification_data(notification, event_data, performer_data=None):
+  if not performer_data:
+    performer_data = {}
+
+    oauth_token = get_validated_oauth_token()
+    if oauth_token:
+      performer_data['oauth_token_id'] = oauth_token.id
+      performer_data['oauth_token_application_id'] = oauth_token.application.client_id
+      performer_data['oauth_token_application'] = oauth_token.application.name
+
+    performer_user = get_authenticated_user()
+    if performer_user:
+      performer_data['entity_id'] = performer_user.id
+      performer_data['entity_name'] = performer_user.username
+
   return {
     'notification_uuid': notification.uuid,
     'repository_namespace': notification.repository.namespace_user.username,
     'repository_name': notification.repository.name,
-    'event_data': event_data
+    'event_data': event_data,
+    'performer_data': performer_data
   }
 
 
-def spawn_notification(repo, event_name, extra_data={}, subpage=None, pathargs=[]):
+def spawn_notification(repo, event_name, extra_data={}, subpage=None, pathargs=[],
+                       performer_data=None):
   event_data = build_event_data(repo, extra_data=extra_data, subpage=subpage)
 
   notifications = model.list_repo_notifications(repo.namespace_user.username, repo.name,
                                                 event_name=event_name)
-  for notification in notifications:
+  for notification in list(notifications):
-    notification_data = build_notification_data(notification, event_data)
+    notification_data = build_notification_data(notification, event_data, performer_data)
     path = [repo.namespace_user.username, repo.name, event_name] + pathargs
     notification_queue.put(path, json.dumps(notification_data))

endpoints/notificationmethod.py

@@ -279,6 +279,7 @@ class HipchatMethod(NotificationMethod):
       'info': 'gray',
       'warning': 'yellow',
       'error': 'red',
+      'success': 'green',
       'primary': 'purple'
     }.get(level, 'gray')
 
@@ -303,6 +304,56 @@ class HipchatMethod(NotificationMethod):
       raise NotificationMethodPerformException(ex.message)
 
 
+from HTMLParser import HTMLParser
+
+class SlackAdjuster(HTMLParser):
+  def __init__(self):
+    self.reset()
+    self.result = []
+
+  def handle_data(self, d):
+    self.result.append(d)
+
+  def get_attr(self, attrs, name):
+    for attr in attrs:
+      if attr[0] == name:
+        return attr[1]
+
+    return ''
+
+  def handle_starttag(self, tag, attrs):
+    if tag == 'a':
+      self.result.append('<%s|' % (self.get_attr(attrs, 'href'), ))
+
+    if tag == 'i':
+      self.result.append('_')
+
+    if tag == 'b' or tag == 'strong':
+      self.result.append('*')
+
+    if tag == 'img':
+      self.result.append(self.get_attr(attrs, 'alt'))
+      self.result.append(' ')
+
+  def handle_endtag(self, tag):
+    if tag == 'a':
+      self.result.append('>')
+
+    if tag == 'b' or tag == 'strong':
+      self.result.append('*')
+
+    if tag == 'i':
+      self.result.append('_')
+
+  def get_data(self):
+    return ''.join(self.result)
+
+def adjust_tags(html):
+  s = SlackAdjuster()
+  s.feed(html)
+  return s.get_data()
+
+
 class SlackMethod(NotificationMethod):
   """ Method for sending notifications to Slack via the API:
       https://api.slack.com/docs/attachments
@@ -318,12 +369,11 @@ class SlackMethod(NotificationMethod):
     if not config_data.get('subdomain', '').isalnum():
       raise CannotValidateNotificationMethodException('Missing Slack Subdomain Name')
 
-  def formatForSlack(self, message):
+  def format_for_slack(self, message):
     message = message.replace('\n', '')
     message = re.sub(r'\s+', ' ', message)
     message = message.replace('<br>', '\n')
-    message = re.sub(r'<a href="(.+)">(.+)</a>', '<\\1|\\2>', message)
+    return adjust_tags(message)
-    return message
 
   def perform(self, notification, event_handler, notification_data):
     config_data = json.loads(notification.config_json)
@@ -346,6 +396,7 @@ class SlackMethod(NotificationMethod):
       'info': '#ffffff',
       'warning': 'warning',
       'error': 'danger',
+      'success': 'good',
       'primary': 'good'
     }.get(level, '#ffffff')
 
@@ -359,8 +410,9 @@ class SlackMethod(NotificationMethod):
       'attachments': [
         {
           'fallback': summary,
-          'text': self.formatForSlack(message),
+          'text': self.format_for_slack(message),
-          'color': color
+          'color': color,
+          'mrkdwn_in': ["text"]
         }
       ]
     }

events/build_failure.html

@@ -0,0 +1,2 @@
+<a href="{{ event_data.homepage }}">Build</a> failed for repository 
+{{ event_data.repository | repository_reference }} ({{ event_data.build_id }}): {{ event_data.error_message }}

events/build_queued.html

@@ -0,0 +1,9 @@
+{% if event_data.is_manual and notification_data.performer_data.entity_name %}
+{{ notification_data.performer_data.entity_name | user_reference }} queued a
+<a href="{{ event_data.homepage }}">build</a>
+{% elif event_data.trigger_kind %}
+<a href="{{ event_data.homepage }}">Build</a> queued via a {{ event_data.trigger_kind }} trigger
+{% else %}
+<a href="{{ event_data.homepage }}">Build</a> queued
+{% endif %}
+ for repository {{ event_data.repository | repository_reference }} ({{ event_data.build_id }})

events/build_start.html

@@ -0,0 +1,2 @@
+<a href="{{ event_data.homepage }}">Build</a> started for repository 
+{{ event_data.repository | repository_reference }} ({{ event_data.build_id }})

events/build_success.html

@@ -0,0 +1,2 @@
+<a href="{{ event_data.homepage }}">Build</a> completed for repository 
+{{ event_data.repository | repository_reference }} ({{ event_data.build_id }})

events/repo_push.html

@@ -0,0 +1,12 @@
+{% if notification_data.performer_data.entity_name %}
+{{ notification_data.performer_data.entity_name | user_reference }} pushed
+{% else %}
+Push of 
+{% endif %}
+
+{% if event_data.updated_tags %}
+ {{ 'tags' | icon_image }}
+ {% for tag in event_data.updated_tags %}{%if loop.index > 1 %}, {% endif %}{{ (event_data.repository, tag) | repository_tag_reference }}{% endfor %} in
+{% endif %}
+
+ repository {{ event_data.repository | repository_reference }}

util/jinjautil.py

@@ -0,0 +1,91 @@
+from app import get_app_url
+from data import model
+from util.gravatar import compute_hash
+from util.names import parse_robot_username
+from jinja2 import Template, Environment, FileSystemLoader, contextfilter
+
+def icon_path(icon_name):
+  return '%s/static/img/icons/%s.png' % (get_app_url(), icon_name)
+
+def icon_image(icon_name):
+  return  '<img src="%s" alt="%s">' % (icon_path(icon_name), icon_name)
+
+def user_reference(username):
+  user = model.get_namespace_user(username)
+  if not user:
+    return username
+
+  is_robot = False
+  if user.robot:
+    parts = parse_robot_username(username)
+    user = model.get_namespace_user(parts[0])
+
+    return """<span><img src="%s" alt="Robot"> <b>%s</b></span>""" % (icon_path('wrench'), username)
+
+  alt = 'Organization' if user.organization else 'User'
+  return """
+  <span>
+  <img src="http://www.gravatar.com/avatar/%s?s=16&amp;d=identicon"
+      style="vertical-align: middle; margin-left: 6px; margin-right: 4px;" alt="%s">
+  <b>%s</b>
+  </span>""" % (compute_hash(user.email), alt, username)
+
+
+def repository_tag_reference(repository_path_and_tag):
+  (repository_path, tag) = repository_path_and_tag
+  (namespace, repository) = repository_path.split('/')
+  owner = model.get_namespace_user(namespace)
+  if not owner:
+    return tag
+
+  return """<a href="%s/repository/%s/%s?tag=%s">%s</a>""" % (get_app_url(), namespace, repository,
+                                                              tag, tag)
+
+def repository_reference(pair):
+  if isinstance(pair, tuple):
+    (namespace, repository) = pair
+  else:
+    pair = pair.split('/')
+    namespace = pair[0]
+    repository = pair[1]
+
+  owner = model.get_namespace_user(namespace)
+  if not owner:
+    return "%s/%s" % (namespace, repository)
+
+  return """
+  <span style="white-space: nowrap;">
+  <img src="http://www.gravatar.com/avatar/%s?s=16&amp;d=identicon" style="vertical-align: middle; margin-left: 6px; margin-right: 4px;">
+  <a href="%s/repository/%s/%s">%s/%s</a>
+  </span>
+  """ % (compute_hash(owner.email), get_app_url(), namespace, repository, namespace, repository)
+
+
+def admin_reference(username):
+  user = model.get_user_or_org(username)
+  if not user:
+    return 'account settings'
+
+  if user.organization:
+    return """
+    <a href="%s/organization/%s/admin">organization's admin setting</a>
+    """ % (get_app_url(), username)
+  else:
+    return """
+    <a href="%s/user/">account settings</a>
+    """ % (get_app_url())
+
+
+def get_template_env(searchpath):
+  template_loader = FileSystemLoader(searchpath=searchpath)
+  template_env = Environment(loader=template_loader)
+  add_filters(template_env)
+  return template_env
+
+
+def add_filters(template_env):
+  template_env.filters['icon_image'] = icon_image
+  template_env.filters['user_reference'] = user_reference
+  template_env.filters['admin_reference'] = admin_reference
+  template_env.filters['repository_reference'] = repository_reference
+  template_env.filters['repository_tag_reference'] = repository_tag_reference

util/useremails.py

@@ -1,58 +1,11 @@
 from flask.ext.mail import Message
 
 from app import mail, app, get_app_url
-from jinja2 import Template, Environment, FileSystemLoader, contextfilter
 from data import model
 from util.gravatar import compute_hash
+from util.jinjautil import get_template_env
 
-def user_reference(username):
+template_env = get_template_env("emails")
-  user = model.get_user_or_org(username)
-  if not user:
-    return username
-
-  return """
-  <span>
-  <img src="http://www.gravatar.com/avatar/%s?s=16&amp;d=identicon" style="vertical-align: middle; margin-left: 6px; margin-right: 4px;">
-  <b>%s</b>
-  </span>""" % (compute_hash(user.email), username)
-
-
-def repository_reference(pair):
-  (namespace, repository) = pair
-
-  owner = model.get_user(namespace)
-  if not owner:
-    return "%s/%s" % (namespace, repository)
-
-  return """
-  <span style="white-space: nowrap;">
-  <img src="http://www.gravatar.com/avatar/%s?s=16&amp;d=identicon" style="vertical-align: middle; margin-left: 6px; margin-right: 4px;">
-  <a href="%s/repository/%s/%s">%s/%s</a>
-  </span>
-  """ % (compute_hash(owner.email), get_app_url(), namespace, repository, namespace, repository)
-
-
-def admin_reference(username):
-  user = model.get_user(username)
-  if not user:
-    return 'account settings'
-
-  if user.organization:
-    return """
-    <a href="%s/organization/%s/admin">organization's admin setting</a>
-    """ % (get_app_url(), username)
-  else:
-    return """
-    <a href="%s/user/">account settings</a>
-    """ % (get_app_url())
-
-
-template_loader = FileSystemLoader(searchpath="emails")
-template_env = Environment(loader=template_loader)
-template_env.filters['user_reference'] = user_reference
-template_env.filters['admin_reference'] = admin_reference
-template_env.filters['repository_reference'] = repository_reference
-
 
 def send_email(recipient, subject, template_file, parameters):
   app_title = app.config['REGISTRY_TITLE_SHORT']