Merge pull request #62 from ckan/recip-details

The email that gets sent now has full details of why recipient got the email

ckanext/issues/logic/action/action.py

@@ -109,33 +109,44 @@ def _get_next_issue_number(session, dataset_id):
 
 def _get_recipients(context, dataset):
     organization = dataset.owner_org
-    if organization:
-        roles = authz.get_roles_with_permission('update_dataset')
-        recipients = []
-        for role in roles:
-            recipients += p.toolkit.get_action('member_list')(
-                context,
-                data_dict={
-                    'id': organization,
-                    'object_type': 'user',
-                    'capacity': role,
-                }
-            )
-        return [recipient[0] for recipient in recipients]
-    else:
+    if not organization:
+        return []
+    org_obj = model.Group.get(organization)
+    if not org_obj:
         return []
+    recipients = []
+    roles = authz.get_roles_with_permission('update_dataset')
+    for role in roles:
+        members = p.toolkit.get_action('member_list')(
+            context,
+            data_dict={
+                'id': organization,
+                'object_type': 'user',
+                'capacity': role,
+            }
+        )
+        for member in members:
+            recipients.append(dict(
+                user_id=member[0],
+                capacity=member[2],
+                organization_name=org_obj.name,
+                organization_title=org_obj.title,
+                ))
+    return recipients
+
 
-def _get_issue_vars(issue, issue_subject, user_obj):
+def _get_issue_vars(issue, issue_subject, user_obj, recipient):
     return {'issue': issue,
             'issue_subject': issue_subject,
             'dataset': model.Package.get(issue.dataset_id),
             'user': user_obj,
             'site_title': get_site_title(),
+            'recipient': recipient,
             'h': h}
 
 
-def _get_issue_email_body(issue, issue_subject, user_obj):
-    extra_vars = _get_issue_vars(issue, issue_subject, user_obj)
+def _get_issue_email_body(issue, issue_subject, user_obj, recipient):
+    extra_vars = _get_issue_vars(issue, issue_subject, user_obj, recipient)
     # Would use p.toolkit.render, but it mucks with response and other things,
     # which is unnecessary, and p.toolkit.render_text uses genshi...
     return render_jinja2('issues/email/new_issue.html', extra_vars=extra_vars)
@@ -191,10 +202,13 @@ def issue_create(context, data_dict):
     if notifications:
         recipients = _get_recipients(context, dataset)
         subject = get_issue_subject(issue.as_dict())
-        body = _get_issue_email_body(issue, subject, user_obj)
 
-        for recipient in recipients:
-            user_obj = model.User.get(recipient)
+        for i, recipient in enumerate(recipients):
+            body = _get_issue_email_body(issue, subject, user_obj, recipient)
+            user_obj = model.User.get(recipient['user_id'])
+            if i == 0:
+                log.debug('Mailing to %s (and %s others):\n%s',
+                          user_obj.email, len(recipients) - 1, body)
             try:
                 mailer.mail_user(user_obj, subject, body)
             except (mailer.MailerException, TypeError), e:

ckanext/issues/templates/issues/email/new_issue.html

@@ -1,11 +1,10 @@
-
-A user '{{ user.fullname }}' has raised an issue with one of your datasets on {{ site_title }}.
+On {{ site_title }} a user has raised an issue with one of the datasets from organization '{{ recipient.organization_title }}', for which you are an {{ recipient.capacity }}.' for which you are an {{ capacity }}.
 
 The dataset is: {{ dataset.title }}
 The issue is:
 
-    {{ issue_subject }}
-    "{{ issue.description }}"
+> {{ issue_subject|wordwrap(width=76)|replace('\n', '\n> '|safe) }}
+> {{ issue.description|wordwrap(width=76)|replace('\n', '\n> '|safe) }}
 
 Please correct the problem or add an initial comment about the issue at: {{ h.url_for('issues_show', dataset_id=dataset.name, issue_number=issue.number, qualified=True) }}. Once the issue has been resolved, click 'Close issue'.
 
@@ -17,4 +16,4 @@
 
 Issue: {{ h.url_for('issues_show', dataset_id=dataset.name, issue_number=issue.number, qualified=True) }}
 Dataset: {{ h.url_for('dataset_read', id=dataset.name, qualified=True) }}
-
+User: {{ user.fullname or user.name }}

ckanext/issues/tests/logic/action/test_issue.py

@@ -7,6 +7,7 @@
 from ckanext.issues.tests import factories as issue_factories
 from ckanext.issues.model import Issue, IssueComment, AbuseStatus
 from ckanext.issues.tests.helpers import ClearOnTearDownMixin
+from ckanext.issues.logic.action.action import _get_recipients
 
 from ckan import model
 
@@ -158,6 +159,20 @@ def test_issue_create_cannot_set_abuse(self):
         issue_object = Issue.get(issue_create_result['id'])
         assert_equals('visible', issue_object.visibility)
 
+    def test_get_recipients(self):
+        user = factories.User()
+        org = factories.Organization(user=user)
+        dataset = factories.Dataset(owner_org=org['id'])
+        dataset_obj = model.Package.get(dataset['id'])
+
+        recip = _get_recipients(context={}, dataset=dataset_obj)
+
+        assert_equals(len(recip), 1)
+        assert_equals(recip[0]['user_id'], user['id'])
+        assert_equals(recip[0]['capacity'], 'Admin')
+        assert_equals(recip[0]['organization_name'], org['name'])
+        assert_equals(recip[0]['organization_title'], org['title'])
+
 
 class TestIssueComment(ClearOnTearDownMixin):
     def test_create_comment_on_issue(self):