From e5890740164c38ca0ebf10bf25a297b0b634e204 Mon Sep 17 00:00:00 2001 From: Vitor Baptista Date: Fri, 16 Aug 2013 10:39:25 -0300 Subject: [PATCH] [#1178] Send email to the invited user I removed the time.sleep(0.1) on TestMailer. Looking through the code, I couldn't find anywhere where a timer looked needed. And I ran these tests a hundred times without the timer to see if I could make them fail, but no. So, I guess they're not needed anymore. I also had to move the RESET_LINK_MESSAGE inside get_reset_link_body(). This was because, when importing ckan.lib.mailer in ckan.logic.action.create.py, I got: TypeError: No object (name: translator) has been registered for this thread This seems to be because we were using _() before pylons had a change to set up the translator. Moving it inside the method solves this. --- ckan/lib/mailer.py | 59 +++++++++++++++++++++++---------- ckan/logic/action/create.py | 18 +++++++--- ckan/tests/lib/test_mailer.py | 37 ++++++++++++++------- ckan/tests/logic/test_action.py | 28 ++++++++++++++-- 4 files changed, 105 insertions(+), 37 deletions(-) diff --git a/ckan/lib/mailer.py b/ckan/lib/mailer.py index f0e1978ac2c..72ebe33e2e0 100644 --- a/ckan/lib/mailer.py +++ b/ckan/lib/mailer.py @@ -100,21 +100,37 @@ def mail_user(recipient, subject, body, headers={}): mail_recipient(recipient.display_name, recipient.email, subject, body, headers=headers) +def get_reset_link_body(user): + reset_link_message = _( + '''You have requested your password on %(site_title)s to be reset. -RESET_LINK_MESSAGE = _( -'''You have requested your password on %(site_title)s to be reset. + Please click the following link to confirm this request: -Please click the following link to confirm this request: + %(reset_link)s + ''') - %(reset_link)s -''') + d = { + 'reset_link': get_reset_link(user), + 'site_title': g.site_title + } + return reset_link_message % d -def make_key(): - return uuid.uuid4().hex[:10] +def get_invite_body(user): + invite_message = _( + '''You have been invited to %(site_title)s. A user has already been created to + you with the username %(user_name)s. You can change it later. -def create_reset_key(user): - user.reset_key = unicode(make_key()) - model.repo.commit_and_remove() + To accept this invite, please reset your password at: + + %(reset_link)s + ''') + + d = { + 'reset_link': get_reset_link(user), + 'site_title': g.site_title, + 'user_name': user.name, + } + return invite_message % d def get_reset_link(user): return urljoin(g.site_url, @@ -123,17 +139,24 @@ def get_reset_link(user): id=user.id, key=user.reset_key)) -def get_reset_link_body(user): - d = { - 'reset_link': get_reset_link(user), - 'site_title': g.site_title - } - return RESET_LINK_MESSAGE % d - def send_reset_link(user): create_reset_key(user) body = get_reset_link_body(user) - mail_user(user, _('Reset your password'), body) + subject = _('Reset your password') + mail_user(user, subject, body) + +def send_invite(user): + create_reset_key(user) + body = get_invite_body(user) + subject = _('Invite for {site_title}'.format(site_title=g.site_title)) + mail_user(user, subject, body) + +def create_reset_key(user): + user.reset_key = unicode(make_key()) + model.repo.commit_and_remove() + +def make_key(): + return uuid.uuid4().hex[:10] def verify_reset_link(user, key): if not key: diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 346fd30e680..edef0a95f30 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -18,6 +18,7 @@ import ckan.lib.dictization.model_save as model_save import ckan.lib.navl.dictization_functions import ckan.lib.navl.validators as validators +import ckan.lib.mailer as mailer from ckan.common import _ @@ -840,7 +841,17 @@ def user_create(context, data_dict): return user_dict def user_invite(context, data_dict): - '''docstring''' + '''Invite a new user. + + You must be authorized to invite users. + + :param email: the email address for the new user + :type email: string + + :returns: the newly created yser + :rtype: dictionary + + ''' _check_access('user_invite', context, data_dict) user_invite_schema = { @@ -852,7 +863,6 @@ def user_invite(context, data_dict): while True: try: - import ckan.lib.mailer name = _get_random_username_from_email(data_dict['email']) password = str(random.SystemRandom().random()) data_dict['name'] = name @@ -860,7 +870,7 @@ def user_invite(context, data_dict): data_dict['state'] = ckan.model.State.PENDING user_dict = _get_action('user_create')(context, data_dict) user = ckan.model.User.get(user_dict['id']) - ckan.lib.mailer.create_reset_key(user) + mailer.send_invite(user) return model_dictize.user_dictize(user, context) except ValidationError as e: if 'name' not in e.error_dict: @@ -868,7 +878,7 @@ def user_invite(context, data_dict): def _get_random_username_from_email(email): localpart = email.split('@')[0] - cleaned_localpart = re.sub(r'[^\w]', '', localpart) + cleaned_localpart = re.sub(r'[^\w]', '-', localpart) random_number = random.SystemRandom().random() * 10000 name = '%s-%d' % (cleaned_localpart, random_number) return name diff --git a/ckan/tests/lib/test_mailer.py b/ckan/tests/lib/test_mailer.py index b95d4d290e3..22b3985930b 100644 --- a/ckan/tests/lib/test_mailer.py +++ b/ckan/tests/lib/test_mailer.py @@ -1,13 +1,12 @@ -import time from nose.tools import assert_equal, assert_raises from pylons import config from email.mime.text import MIMEText import hashlib -from ckan import model +import ckan.model as model +import ckan.lib.mailer as mailer from ckan.tests.pylons_controller import PylonsTestCase from ckan.tests.mock_mail_server import SmtpServerHarness -from ckan.lib.mailer import mail_recipient, mail_user, send_reset_link, add_msg_niceties, MailerException, get_reset_link_body, get_reset_link from ckan.lib.create_test_data import CreateTestData from ckan.lib.base import g @@ -35,7 +34,7 @@ def setup(self): def mime_encode(self, msg, recipient_name): sender_name = g.site_title sender_url = g.site_url - body = add_msg_niceties(recipient_name, msg, sender_name, sender_url) + body = mailer.add_msg_niceties(recipient_name, msg, sender_name, sender_url) encoded_body = MIMEText(body.encode('utf-8'), 'plain', 'utf-8').get_payload().strip() return encoded_body @@ -49,8 +48,7 @@ def test_mail_recipient(self): 'subject': 'Meeting', 'body': 'The meeting is cancelled.', 'headers': {'header1': 'value1'}} - mail_recipient(**test_email) - time.sleep(0.1) + mailer.mail_recipient(**test_email) # check it went to the mock smtp server msgs = self.get_smtp_messages() @@ -74,8 +72,7 @@ def test_mail_user(self): 'subject': 'Meeting', 'body': 'The meeting is cancelled.', 'headers': {'header1': 'value1'}} - mail_user(**test_email) - time.sleep(0.1) + mailer.mail_user(**test_email) # check it went to the mock smtp server msgs = self.get_smtp_messages() @@ -96,12 +93,11 @@ def test_mail_user_without_email(self): 'subject': 'Meeting', 'body': 'The meeting is cancelled.', 'headers': {'header1': 'value1'}} - assert_raises(MailerException, mail_user, **test_email) + assert_raises(mailer.MailerException, mailer.mail_user, **test_email) def test_send_reset_email(self): # send email - send_reset_link(model.User.by_name(u'bob')) - time.sleep(0.1) + mailer.send_reset_link(model.User.by_name(u'bob')) # check it went to the mock smtp server msgs = self.get_smtp_messages() @@ -110,7 +106,24 @@ def test_send_reset_email(self): assert_equal(msg[1], config['smtp.mail_from']) assert_equal(msg[2], [model.User.by_name(u'bob').email]) assert 'Reset' in msg[3], msg[3] - test_msg = get_reset_link_body(model.User.by_name(u'bob')) + test_msg = mailer.get_reset_link_body(model.User.by_name(u'bob')) + expected_body = self.mime_encode(test_msg, + u'bob') + assert expected_body in msg[3], '%r not in %r' % (expected_body, msg[3]) + + # reset link tested in user functional test + + def test_send_invite_email(self): + # send email + mailer.send_invite(model.User.by_name(u'bob')) + + # check it went to the mock smtp server + msgs = self.get_smtp_messages() + assert_equal(len(msgs), 1) + msg = msgs[0] + assert_equal(msg[1], config['smtp.mail_from']) + assert_equal(msg[2], [model.User.by_name(u'bob').email]) + test_msg = mailer.get_invite_body(model.User.by_name(u'bob')) expected_body = self.mime_encode(test_msg, u'bob') assert expected_body in msg[3], '%r not in %r' % (expected_body, msg[3]) diff --git a/ckan/tests/logic/test_action.py b/ckan/tests/logic/test_action.py index 7bf17bd094e..b6361803f0e 100644 --- a/ckan/tests/logic/test_action.py +++ b/ckan/tests/logic/test_action.py @@ -562,7 +562,8 @@ def test_12_user_update_errors(self): for expected_message in test_call['messages']: assert expected_message[1] in ''.join(res_obj['error'][expected_message[0]]) - def test_user_invite(self): + @mock.patch('ckan.lib.mailer.mail_user') + def test_user_invite(self, mail_user): email_username = 'invited_user$ckan' email = '%s@email.com' % email_username user_dict = {'email': email} @@ -574,14 +575,32 @@ def test_user_invite(self): res_obj = json.loads(res.body) user = model.User.get(res_obj['result']['id']) - expected_username = email_username.replace('$', '') + expected_username = email_username.replace('$', '-') assert res_obj['success'] is True, res_obj assert user.email == email, (user.email, email) assert user.name.startswith(expected_username), (user.name, expected_username) assert user.is_pending(), user assert user.reset_key is not None, user - def test_user_invite_without_email_raises_error(self): + @mock.patch('ckan.lib.mailer.send_invite') + def test_user_invite_sends_email(self, send_invite): + email_username = 'invited_user' + email = '%s@email.com' % email_username + user_dict = {'email': email} + postparams = '%s=1' % json.dumps(user_dict) + extra_environ = {'Authorization': str(self.sysadmin_user.apikey)} + + res = self.app.post('/api/action/user_invite', params=postparams, + extra_environ=extra_environ) + + res_obj = json.loads(res.body) + user = model.User.get(res_obj['result']['id']) + assert res_obj['success'] is True, res_obj + assert send_invite.called + assert send_invite.call_args[0][0].id == res_obj['result']['id'] + + @mock.patch('ckan.lib.mailer.mail_user') + def test_user_invite_without_email_raises_error(self, mail_user): user_dict = {} postparams = '%s=1' % json.dumps(user_dict) extra_environ = {'Authorization': str(self.sysadmin_user.apikey)} @@ -596,6 +615,8 @@ def test_user_invite_without_email_raises_error(self): @mock.patch('ckan.logic.action.create._get_random_username_from_email') def test_user_invite_should_work_even_if_tried_username_already_exists(self, random_username_mock): + patcher = mock.patch('ckan.lib.mailer.mail_user') + patcher.start() email = 'invited_user@email.com' user_dict = {'email': email} postparams = '%s=1' % json.dumps(user_dict) @@ -610,6 +631,7 @@ def test_user_invite_should_work_even_if_tried_username_already_exists(self, ran res_obj = json.loads(res.body) assert res_obj['success'] is True, res_obj + patcher.stop() def test_user_delete(self): name = 'normal_user'