From ec612038cad3dbb00010905998edcf5257c68526 Mon Sep 17 00:00:00 2001 From: Alisha Mohanty Date: Jul 23 2019 17:29:38 +0000 Subject: Migrate testcases to Pytest, Pass failing Tests, Add unittest for check_recipient_is_sender, Modify clean method(forms.py), Add test for pagination --- diff --git a/happinesspackets/messaging/forms.py b/happinesspackets/messaging/forms.py index 799b891..09a6563 100644 --- a/happinesspackets/messaging/forms.py +++ b/happinesspackets/messaging/forms.py @@ -11,15 +11,19 @@ from django.conf import settings from django.urls import reverse from django.db.models import Q from django.utils import timezone -from email_normalize import normalize import bleach from .models import Message, strip_email logger = logging.getLogger(__name__) +def check_recipient_is_sender(sender_emails, recipient_emails) -> bool: + if not (set(sender_emails).isdisjoint(recipient_emails)): + return True + else: + return False -def validate_email(email): +def validate_email_is_rate_limited(email): timeframe = timezone.now() - timedelta(hours=24) stripped_email = strip_email(email) recent_message_count = Message.objects.filter(Q(sender_email_stripped=stripped_email) | Q(recipient_email_stripped=stripped_email), created__gte=timeframe).count() @@ -46,7 +50,7 @@ class MessageSendForm(forms.ModelForm): self.fields['recipient_name'].label = 'Name' self.fields['recipient_email'].label = 'Email' - self.fields['recipient_email'].validators = [validate_email] + self.fields['recipient_email'].validators = [validate_email_is_rate_limited] self.fields['message'].help_text = 'Writer\'s block? Check out our message inspiration.' % reverse('messaging:inspiration') self.fields['sender_named'].label = 'I agree to share my name and email address with the recipient.' self.fields['sender_approved_public'].label = "I agree to publish this message and display it publicly in the Happiness Archive." @@ -62,20 +66,6 @@ class MessageSendForm(forms.ModelForm): HTML("
"), Submit('submit', 'Send some happiness', css_class='btn-lg centered'), ) - - def is_recipient_email_equals_sender_email(self): - recipient_email = self.cleaned_data.get('recipient_email') - sender_email = self.user.email - sender_username = self.user.username - normalized_sender_email = normalize(sender_email) - normalized_recipient_email = normalize(recipient_email) - #Fedora assigned email to the Sender - sender_fedora_email = sender_username + '@fedoraproject.org' - if normalized_recipient_email in ( - sender_email, sender_fedora_email, normalized_sender_email): - return True - else: - return False def clean_message(self): """ Cleans given HTML with bleach.clean() """ @@ -106,19 +96,28 @@ class MessageSendForm(forms.ModelForm): attributes=allowed_attributes, styles=allowed_styles ) - + def clean(self): super(MessageSendForm, self).clean() - isREEqualsSE = self.is_recipient_email_equals_sender_email() - # if self.cleaned_data.get('hp'): - # raise forms.ValidationError('') - if self.cleaned_data.get('sender_approved_public_named') and not self.cleaned_data.get('sender_approved_public'): - self.add_error('sender_approved_public_named', "If you want us to publish the message including your names, " - "you must also check 'I agree to publish this message and" - "display it publicly in the Happiness Archive'") - if isREEqualsSE: - raise forms.ValidationError("You cannot send a Fedora Happiness Packet to yourself!") - validate_email(self.user.email) + sender_emails = [] + sender_emails.append(self.user.email) + sender_emails.append(strip_email(self.user.email)) + sender_emails.append(self.user.username + '@fedoraproject.org') + recipient_emails = [] + recipient_email = self.cleaned_data.get('recipient_email') + recipient_emails.append(recipient_email) + if recipient_email: #Before clean(), validation is performed, which removes recipient_email field if validation is failed + recipient_emails.append(strip_email(recipient_emails[0])) + + if check_recipient_is_sender(sender_emails, recipient_emails): + raise forms.ValidationError("You cannot send a Fedora Happiness Packet to yourself!") + elif self.cleaned_data.get('sender_approved_public_named') and not self.cleaned_data.get('sender_approved_public'): + self.add_error('sender_approved_public_named', "If you want us to publish the message including your names, " + "you must also check 'I agree to publish this message and" + "display it publicly in the Happiness Archive'") + validate_email_is_rate_limited(self.user.email) + + class MessageRecipientForm(forms.ModelForm): class Meta: diff --git a/happinesspackets/messaging/tests/test_forms.py b/happinesspackets/messaging/tests/test_forms.py new file mode 100644 index 0000000..5aa3289 --- /dev/null +++ b/happinesspackets/messaging/tests/test_forms.py @@ -0,0 +1,19 @@ +from ..forms import check_recipient_is_sender +from django.test import TestCase + + +class TestIsReceipientEqualsSenderEmail(TestCase): + + def test_should_return_true_for_same_sender_and_recipient(self): + sender_email = [] + recipient_email = [] + sender_email.append('user@gmail.com') + recipient_email.append('user@gmail.com') + self.assertTrue(check_recipient_is_sender(sender_email, recipient_email)) + + def test_should_return_false_for_different_sender_and_recipient(self): + sender_email = [] + recipient_email = [] + sender_email.append('user+sender@gmail.com') + recipient_email.append('user2+fedora@gmail.com') + self.assertFalse(check_recipient_is_sender(sender_email, recipient_email)) \ No newline at end of file diff --git a/happinesspackets/messaging/tests/test_models.py b/happinesspackets/messaging/tests/test_models.py index 7abbaf2..b4e7bf1 100644 --- a/happinesspackets/messaging/tests/test_models.py +++ b/happinesspackets/messaging/tests/test_models.py @@ -1,9 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals - import factory -from django.test import TestCase - +import pytest class MessageModelFactory(factory.DjangoModelFactory): class Meta: @@ -24,9 +22,11 @@ class BlacklistedEmailFactory(factory.DjangoModelFactory): email = 'emailemail@null' confirmation_ip = '127.0.0.1' - -class MessageModelTest(TestCase): +@pytest.mark.django_db +class TestMessageModel: + def test_unique_identifier(self): obj1 = MessageModelFactory() obj2 = MessageModelFactory() - self.assertNotEqual(obj1.identifier, obj2.identifier) + assert obj1.identifier != obj2.identifier + diff --git a/happinesspackets/messaging/tests/test_views.py b/happinesspackets/messaging/tests/test_views.py index d1f1b25..7e0c5bb 100644 --- a/happinesspackets/messaging/tests/test_views.py +++ b/happinesspackets/messaging/tests/test_views.py @@ -14,14 +14,11 @@ from .test_models import MessageModelFactory, BlacklistedEmailFactory from ..models import Message, BLACKLIST_HMAC_SALT, BlacklistedEmail -class SearchViewTest(TestCase): +class TestSearchView(TestCase): url = reverse('messaging:search') def test_anonymous_message_indexed(self): - MessageModelFactory(sender_approved_public=True, sender_approved_public_named=False, - recipient_approved_public=True, recipient_approved_public_named=True, - admin_approved_public=True) - msg = MessageModelFactory(sender_approved_public=True, sender_approved_public_named=True, + msg = MessageModelFactory(sender_approved_public=True, sender_approved_public_named=False, recipient_approved_public=True, recipient_approved_public_named=False, admin_approved_public=True) update_index.Command().handle(using=['default']) @@ -33,6 +30,18 @@ class SearchViewTest(TestCase): self.assertNotContains(response, msg.sender_email) self.assertNotContains(response, msg.recipient_email) + def test_pagination_is_ten(self): + number_of_msgs = 11 + for msg in range(number_of_msgs): + msg = MessageModelFactory(sender_approved_public=True, sender_approved_public_named=True, + recipient_approved_public=True, recipient_approved_public_named=True, + admin_approved_public=True) + response = self.client.get(self.url,{'q':msg.message}) + self.assertEqual(response.status_code, 200) + self.assertTrue('is_paginated' in response.context) + self.assertTrue(response.context['is_paginated'] == True) + self.assertTrue(len(response.context['object_list']) == 10) + def test_named_message_indexed(self): msg = MessageModelFactory(sender_approved_public=True, sender_approved_public_named=True, recipient_approved_public=True, recipient_approved_public_named=True, @@ -61,7 +70,7 @@ class SearchViewTest(TestCase): self.assertEqual(response.status_code, 200) self.assertEqual(len(response.context['object_list']),0) -class MessageCounterTest(TestCase): +class TestMessageCounter(TestCase): url = reverse('messaging:start') def test_message_sent_included(self): @@ -80,7 +89,7 @@ class MessageCounterTest(TestCase): self.assertEqual(response.context['packets_sent'],0) -class StartViewTest(TestCase): +class TestStartView(TestCase): url = reverse('messaging:start') def test_renders(self): @@ -99,7 +108,7 @@ class StartViewTest(TestCase): self.assertNotContains(response, msg.recipient_email) -class FaqViewTest(TestCase): +class TestFaqView(TestCase): url = reverse('messaging:faq') def test_renders(self): @@ -107,7 +116,7 @@ class FaqViewTest(TestCase): self.assertEqual(response.status_code, 200) -class InspirationViewTest(TestCase): +class TestInspirationView(TestCase): url = reverse('messaging:inspiration') def test_renders(self): @@ -115,7 +124,7 @@ class InspirationViewTest(TestCase): self.assertEqual(response.status_code, 200) -class ArchiveViewTest(TestCase): +class TestArchiveView(TestCase): url = reverse('messaging:archive') def test_renders_no_public_messages(self): @@ -160,7 +169,7 @@ class ArchiveViewTest(TestCase): self.assertNotContains(response, msg.recipient_email) -class BlacklistViewTest(TestCase): +class TestBlacklistView(TestCase): url_name = 'messaging:blacklist_email' def setUp(self): @@ -190,11 +199,11 @@ class BlacklistViewTest(TestCase): self.assertFalse(BlacklistedEmail.objects.count()) -class SendViewTest(TestCase): +class TestSendView(TestCase): url = reverse('messaging:send') def setUp(self): - super(SendViewTest, self).setUp() + super(TestSendView, self).setUp() self.post_data = { 'sender_name': 'sender name', 'sender_email': 'SEN.DER+FOOBAR@erik.io', @@ -205,9 +214,7 @@ class SendViewTest(TestCase): 'sender_approved_public': True, 'sender_approved_public_named': True, } - self.user = User.objects.create_user("erikio", "sender@erik.io", "helloworld", first_name="Erik",last_name="Doe") - self.user.save() - self.client.login(username = "erikio", password = "helloworld") + self.client.force_login(User.objects.get_or_create(username="erikio",email="sender@erik.io", password="helloworld", first_name="Erik",last_name="Doe")[0]) def test_renders(self): response = self.client.get(self.url) @@ -229,6 +236,7 @@ class SendViewTest(TestCase): response = self.client.post(self.url, self.post_data) self.assertEqual(response.status_code, 200) self.assertEqual(len(response.context['form'].errors), 1) + self.assertFormError(response, 'form', 'sender_approved_public_named', "If you want us to publish the message including your names, you must also check 'I agree to publish this message anddisplay it publicly in the Happiness Archive'") self.assertEqual(len(mail.outbox), 0) def test_post_blacklisted_sender(self): @@ -243,6 +251,7 @@ class SendViewTest(TestCase): response = self.client.post(self.url, self.post_data) self.assertEqual(response.status_code, 200) self.assertEqual(len(response.context['form'].errors), 1) + self.assertFormError(response, 'form', field=None, errors="We can't send emails to this address at this time. You can try again in 24 hours.") self.assertEqual(len(mail.outbox), 0) def test_post_ratelimited_recipient(self): @@ -251,10 +260,18 @@ class SendViewTest(TestCase): response = self.client.post(self.url, self.post_data) self.assertEqual(response.status_code, 200) self.assertEqual(len(response.context['form'].errors), 1) + self.assertFormError(response, 'form', 'recipient_email', "We can't send emails to this address at this time. You can try again in 24 hours.") self.assertEqual(len(mail.outbox), 0) + def test_sender_email_equals_recipient_email(self): + self.post_data['recipient_email'] = 'SEN.DER+FOOBAR@erik.io' + response = self.client.post(self.url, self.post_data) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.context['form'].errors), 1) + self.assertFormError(response, 'form', field=None, errors="You cannot send a Fedora Happiness Packet to yourself!") + self.assertEqual(len(mail.outbox), 0) -class MessageSentViewTest(TestCase): +class TestMessageSentView(TestCase): url = reverse('messaging:sender_confirmation_sent') def test_renders(self): @@ -262,10 +279,11 @@ class MessageSentViewTest(TestCase): self.assertEqual(response.status_code, 200) -class MessageSenderConfirmationView(TestCase): +class TestMessageSenderConfirmationView(TestCase): url_name = 'messaging:sender_confirm' def setUp(self): + self.client.force_login(User.objects.get_or_create(username="erikio",email="sendersender@null", password="helloworld")[0]) self.message = MessageModelFactory(sender_email_token='a-b-c', status=Message.STATUS.pending_sender_confirmation) url_kwargs = {'identifier': self.message.identifier, 'token': self.message.sender_email_token} self.url = reverse(self.url_name, kwargs=url_kwargs) @@ -301,7 +319,7 @@ class MessageSenderConfirmationView(TestCase): self.message.save() response = self.client.get(self.url) - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, 404) self.assertTrue(response.context['not_found']) def test_bad_status(self): @@ -319,7 +337,7 @@ class MessageSenderConfirmationView(TestCase): self.assertEqual(len(mail.outbox), 0) -class MessageSenderConfirmedView(TestCase): +class TestMessageSenderConfirmedView(TestCase): url = reverse('messaging:sender_confirmed') def test_renders(self): @@ -327,7 +345,7 @@ class MessageSenderConfirmedView(TestCase): self.assertEqual(response.status_code, 200) -class MessageRecipientMessageUpdate(TestCase): +class TestMessageRecipientMessageUpdate(TestCase): url_name = 'messaging:recipient_message_update' def setUp(self): diff --git a/happinesspackets/messaging/views.py b/happinesspackets/messaging/views.py index 75ecba1..95050cf 100644 --- a/happinesspackets/messaging/views.py +++ b/happinesspackets/messaging/views.py @@ -36,8 +36,7 @@ logger = logging.getLogger(__name__) class MessageSearchView(SearchView): template_name = 'search/search.html' form_class = SearchForm - paginate_by = 5 - + paginate_by = 10 class ArchiveListView(ListView): model = Message paginate_by = 5 @@ -151,13 +150,17 @@ class MessageSenderConfirmationView(LoginRequiredMixin,TemplateView): try: message = Message.objects.get(identifier=kwargs['identifier'], sender_email_token=kwargs['token']) except Message.DoesNotExist: - return render(request, self.template_name, {'not_found': True}) - + response = render(request, self.template_name, {'not_found': True}) + response.status_code = 404 + return response + if message.sender_email != self.request.user.email: - return render(request, self.template_name, {'not_sender': True}) + response = render(request, self.template_name, {'not_sender': True}) + response.status_code = 400 + return response if message.status != Message.STATUS.pending_sender_confirmation: - return render(request, self.template_name, {'already_confirmed': True}) - + response = render(request, self.template_name, {'already_confirmed': True}) + return response message.send_to_recipient(self.request.is_secure(), self.request.get_host()) sender_name = self.request.user.username if message.sender_named else "Anonymous" @@ -173,9 +176,13 @@ class MessageSenderConfirmationView(LoginRequiredMixin,TemplateView): try: publish(message) except PublishReturned: - return render(request, self.template_name, {'publish_returned': True}) + response = render(request, self.template_name, {'publish_returned': True}) + response.status_code = 500 + return response except ConnectionException: - return render(request, self.template_name, {'connection_exception': True}) + response = render(request, self.template_name, {'connection_exception': True}) + response.status_code = 500 + return response return HttpResponseRedirect(reverse('messaging:sender_confirmed'))