Skip to content

Commit a8c80fd

Browse files
committed
parse reviewer/developer emails and save notes (bug 879412)
1 parent 35dee1c commit a8c80fd

File tree

8 files changed

+218
-59
lines changed

8 files changed

+218
-59
lines changed

apps/addons/models.py

+3
Original file line numberDiff line numberDiff line change
@@ -1394,6 +1394,9 @@ def get_localepicker(self):
13941394
pass
13951395
return ''
13961396

1397+
def get_mozilla_contacts(self):
1398+
return [x.strip() for x in self.mozilla_contact.split(',')]
1399+
13971400
@amo.cached_property
13981401
def upsell(self):
13991402
"""Return the upsell or add-on, or None if there isn't one."""

apps/comm/tasks.py

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import logging
2+
from celeryutils import task
3+
4+
from comm.utils import save_from_email_reply
5+
6+
7+
log = logging.getLogger('z.task')
8+
9+
10+
@task
11+
def consume_email(email_text):
12+
"""Parse emails and save notes."""
13+
res = save_from_email_reply(email_text)
14+
if not res:
15+
log.error('Failed to save email.')

apps/comm/tests/email.txt

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
2+
3+
This is the body
4+
5+
> this is the reply text

apps/comm/tests/test_utils_.py

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import os.path
2+
3+
from django.conf import settings
4+
5+
from nose.tools import eq_
6+
7+
import amo
8+
from amo.tests import app_factory, TestCase
9+
from comm.utils import CommEmailParser, save_from_email_reply
10+
from comm.models import CommunicationThread, CommunicationThreadToken
11+
from mkt.site.fixtures import fixture
12+
from users.models import UserProfile
13+
14+
15+
sample_email = os.path.join(settings.ROOT, 'apps', 'comm', 'tests',
16+
'email.txt')
17+
18+
19+
class TestEmailReplySaving(TestCase):
20+
fixtures = fixture('user_999')
21+
22+
def setUp(self):
23+
app = app_factory(name='Antelope', status=amo.STATUS_PENDING)
24+
self.profile = UserProfile.objects.get(pk=999)
25+
t = CommunicationThread.objects.create(addon=app,
26+
version=app.current_version, read_permission_reviewer=True)
27+
28+
self.token = CommunicationThreadToken.objects.create(thread=t,
29+
user=self.profile)
30+
self.email_template = open(sample_email).read()
31+
32+
def test_successful_save(self):
33+
self.grant_permission(self.profile, 'Apps:Review')
34+
email_text = self.email_template % self.token.uuid
35+
note = save_from_email_reply(email_text)
36+
assert note
37+
eq_(note.body, 'This is the body')
38+
39+
def test_invalid_token_deletion(self):
40+
"""Test when the token's user does not have a permission on thread."""
41+
email_text = self.email_template % self.token.uuid
42+
assert not save_from_email_reply(email_text)
43+
44+
def test_non_existent_token(self):
45+
email_text = self.email_template % (self.token.uuid + 'junk')
46+
assert not save_from_email_reply(email_text)
47+
48+
def test_with_junk_body(self):
49+
email_text = 'this is junk'
50+
assert not save_from_email_reply(email_text)
51+
52+
53+
class TestEmailParser(TestCase):
54+
55+
def setUp(self):
56+
email_text = open(sample_email).read() % 'someuuid'
57+
self.parser = CommEmailParser(email_text)
58+
59+
def test_uuid(self):
60+
eq_(self.parser.get_uuid(), 'someuuid')
61+
62+
def test_body(self):
63+
eq_(self.parser.get_body(), 'This is the body')

apps/comm/utils.py

+124
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
from email import message_from_string
2+
from email.utils import parseaddr
3+
4+
import commonware.log
5+
from email_reply_parser import EmailReplyParser
6+
7+
from access import acl
8+
from comm.models import (CommunicationNote, CommunicationThreadCC,
9+
CommunicationThreadToken)
10+
from mkt.constants import comm
11+
12+
13+
log = commonware.log.getLogger('comm')
14+
15+
16+
class ThreadObjectPermission(object):
17+
"""
18+
Class for determining user permissions on a thread.
19+
"""
20+
21+
def check_acls(self, acl_type):
22+
"""Check ACLs."""
23+
user = self.user_profile
24+
obj = self.thread_obj
25+
if acl_type == 'moz_contact':
26+
return user.email in obj.addon.get_mozilla_contacts()
27+
elif acl_type == 'admin':
28+
return acl.action_allowed_user(user, 'Admin', '%')
29+
elif acl_type == 'reviewer':
30+
return acl.action_allowed_user(user, 'Apps', 'Review')
31+
elif acl_type == 'senior_reviewer':
32+
return acl.action_allowed_user(user, 'Apps', 'ReviewEscalated')
33+
else:
34+
raise 'Invalid ACL lookup.'
35+
36+
return False
37+
38+
def user_has_permission(self, thread, profile):
39+
"""
40+
Check if the user has read/write permissions on the given thread.
41+
42+
Developers of the add-on used in the thread, users in the CC list,
43+
and users who post to the thread are allowed to access the object.
44+
45+
Moreover, other object permissions are also checked agaisnt the ACLs
46+
of the user.
47+
"""
48+
self.thread_obj = thread
49+
self.user_profile = profile
50+
user_post = CommunicationNote.objects.filter(author=profile,
51+
thread=thread)
52+
user_cc = CommunicationThreadCC.objects.filter(user=profile,
53+
thread=thread)
54+
55+
if user_post.exists() or user_cc.exists():
56+
return True
57+
58+
# User is a developer of the add-on and has the permission to read.
59+
user_is_author = profile.addons.filter(pk=thread.addon_id)
60+
if thread.read_permission_developer and user_is_author.exists():
61+
return True
62+
63+
if thread.read_permission_reviewer and self.check_acls('reviewer'):
64+
return True
65+
66+
if (thread.read_permission_senior_reviewer and
67+
self.check_acls('senior_reviewer')):
68+
return True
69+
70+
if (thread.read_permission_mozilla_contact and
71+
self.check_acls('moz_contact')):
72+
return True
73+
74+
if thread.read_permission_staff and self.check_acls('admin'):
75+
return True
76+
77+
return False
78+
79+
80+
class CommEmailParser(object):
81+
"""Utility to parse email replies."""
82+
83+
address_prefix = 'reply+'
84+
85+
def __init__(self, email_text):
86+
self.email = message_from_string(email_text)
87+
self.reply_text = EmailReplyParser.read(self.email.get_payload()).reply
88+
89+
def _get_address_line(self):
90+
return parseaddr(self.email['to'])
91+
92+
def get_uuid(self):
93+
name, addr = self._get_address_line()
94+
if addr.startswith(self.address_prefix):
95+
# Strip everything between "reply+" and the "@" sign.
96+
uuid = addr.lstrip(self.address_prefix).split('@')[0]
97+
else:
98+
return False
99+
100+
return uuid
101+
102+
def get_body(self):
103+
return self.reply_text
104+
105+
106+
def save_from_email_reply(reply_text):
107+
parser = CommEmailParser(reply_text)
108+
uuid = parser.get_uuid()
109+
110+
if not uuid:
111+
return False
112+
try:
113+
tok = CommunicationThreadToken.objects.get(uuid=uuid)
114+
except CommunicationThreadToken.DoesNotExist:
115+
log.error('An email was skipped with non-existing uuid %s' % uuid)
116+
return False
117+
118+
if ThreadObjectPermission().user_has_permission(tok.thread, tok.user):
119+
n = CommunicationNote.objects.create(note_type=comm.NO_ACTION,
120+
thread=tok.thread, author=tok.user, body=parser.get_body())
121+
log.info('A new note has been created (from %s using tokenid %s)' %
122+
(tok.user.id, uuid))
123+
return n
124+
return False

mkt/comm/api.py

+5-55
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
from functools import partial
2-
31
from django.db.models import Q
42
from django.http import Http404
53
from django.shortcuts import get_object_or_404
@@ -10,14 +8,13 @@
108
from rest_framework.mixins import (CreateModelMixin, DestroyModelMixin,
119
ListModelMixin, RetrieveModelMixin)
1210
from rest_framework.permissions import BasePermission
13-
from rest_framework.relations import HyperlinkedRelatedField, RelatedField
11+
from rest_framework.relations import RelatedField
1412
from rest_framework.serializers import ModelSerializer, SerializerMethodField
1513

16-
from access import acl
1714
from addons.models import Addon
1815
from users.models import UserProfile
19-
from comm.models import (CommunicationNote, CommunicationThread,
20-
CommunicationThreadCC, CommunicationThreadToken)
16+
from comm.models import CommunicationNote, CommunicationThread
17+
from comm.utils import ThreadObjectPermission
2118
from mkt.api.authentication import (RestOAuthAuthentication,
2219
RestSharedSecretAuthentication)
2320
from mkt.api.base import CORSViewSet
@@ -70,24 +67,12 @@ def get_notes_count(self, obj):
7067
return obj.notes.count()
7168

7269

73-
class ThreadPermission(BasePermission):
70+
class ThreadPermission(BasePermission, ThreadObjectPermission):
7471
"""
7572
Permission wrapper for checking if the authenticated user has the
7673
permission to view the thread.
7774
"""
7875

79-
def check_acls(self, request, obj, acl_type):
80-
if acl_type == 'moz_contact':
81-
return request.user.email == obj.addon.mozilla_contact
82-
elif acl_type == 'admin':
83-
return acl.action_allowed(request, 'Admin', '%')
84-
elif acl_type == 'reviewer':
85-
return acl.action_allowed(request, 'Apps', 'Review')
86-
elif acl_type == 'senior_reviewer':
87-
return acl.action_allowed(request, 'Apps', 'ReviewEscalated')
88-
else:
89-
raise 'Invalid ACL lookup.'
90-
9176
def has_permission(self, request, view):
9277
# Let `has_object_permission` handle the permissions when we retrieve
9378
# an object.
@@ -101,47 +86,12 @@ def has_permission(self, request, view):
10186
def has_object_permission(self, request, view, obj):
10287
"""
10388
Make sure we give correct permissions to read/write the thread.
104-
105-
Developers of the add-on used in the thread, users in the CC list,
106-
and users who post to the thread are allowed to access the object.
107-
108-
Moreover, other object permissions are also checked agaisnt the ACLs
109-
of the user.
11089
"""
11190
if not request.user.is_authenticated() or obj.read_permission_public:
11291
return obj.read_permission_public
11392

11493
profile = request.amo_user
115-
user_post = CommunicationNote.objects.filter(author=profile,
116-
thread=obj)
117-
user_cc = CommunicationThreadCC.objects.filter(user=profile,
118-
thread=obj)
119-
120-
if user_post.exists() or user_cc.exists():
121-
return True
122-
123-
check_acls = partial(self.check_acls, request, obj)
124-
125-
# User is a developer of the add-on and has the permission to read.
126-
user_is_author = profile.addons.filter(pk=obj.addon_id)
127-
if obj.read_permission_developer and user_is_author.exists():
128-
return True
129-
130-
if obj.read_permission_reviewer and check_acls('reviewer'):
131-
return True
132-
133-
if (obj.read_permission_senior_reviewer and check_acls(
134-
'senior_reviewer')):
135-
return True
136-
137-
if (obj.read_permission_mozilla_contact and check_acls(
138-
'moz_contact')):
139-
return True
140-
141-
if obj.read_permission_staff and check_acls('admin'):
142-
return True
143-
144-
return False
94+
return self.user_has_permission(obj, profile)
14595

14696

14797
class NotePermission(ThreadPermission):

mkt/comm/tests/test_api.py

+2-4
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@
77

88
from amo.tests import addon_factory
99
from comm.models import (CommunicationNote, CommunicationThread,
10-
CommunicationThreadCC, CommunicationThreadToken)
10+
CommunicationThreadCC)
1111
from mkt.api.tests.test_oauth import RestOAuth
1212
from mkt.comm.api import ThreadPermission
13-
from mkt.constants import comm as const
1413
from mkt.site.fixtures import fixture
1514
from mkt.webapps.models import Webapp
1615

@@ -75,7 +74,7 @@ def test_read_public(self):
7574
def test_read_moz_contact(self):
7675
thread = CommunicationThread.objects.create(addon=self.addon,
7776
read_permission_mozilla_contact=True)
78-
thread.addon.mozilla_contact = self.user.email
77+
thread.addon.mozilla_contact = self.profile.email
7978
thread.addon.save()
8079
self.thread = thread
8180
assert self.check_permissions()
@@ -139,7 +138,6 @@ def test_addon_filter(self):
139138
eq_(res.status_code, 404)
140139

141140
def test_creation(self):
142-
thread = CommunicationThread.objects.create(addon=self.addon)
143141
version = self.addon.current_version
144142
res = self.client.post(self.list_url, data=json.dumps(
145143
{'addon': self.addon.id, 'version': version.id}))

requirements/prod.txt

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ django-tastypie==0.9.11
4040
django-waffle==0.9.1
4141
easy-thumbnails==1.1
4242
elasticutils==0.7
43+
email-reply-parser==0.1.9
4344
fastchardet==0.2.0
4445
feedparser==5.0.1
4546
fudge==1.0.3

0 commit comments

Comments
 (0)