Fix signup error handling #795

activity/tests/test_registration.py

@@ -0,0 +1,104 @@
+from unittest.mock import patch
+from django.test import TestCase, Client
+from django.contrib.auth.models import User
+from django.urls import reverse
+from workflow.models import Organization, UserInvite
+
+
+class RegistrationFlowTest(TestCase):
+    def setUp(self):
+        self.client = Client()
+        self.register_url = reverse('register', kwargs={'invite_uuid': 'none'})
+
+    def test_registration_page_renders(self):
+        response = self.client.get(self.register_url)
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'registration/register.html')
+
+    def test_successful_signup_flow(self):
+        data = {
+            'first_name': 'Jane',
+            'last_name': 'Doe',
+            'username': 'janedoe',
+            'email_address': 'janedoe@example.com',
+            'password': 'Password123!',
+            'confirm_password': 'Password123!',
+        }
+        with patch('activity.views.send_single_mail') as mock_email:
+            mock_email.return_value = 1
+            response = self.client.post(self.register_url, data)
+            self.assertEqual(response.status_code, 200)
+            self.assertTemplateUsed(response, 'registration/confirm_email.html')
+            self.assertTrue(User.objects.filter(username='janedoe').exists())
+            user = User.objects.get(username='janedoe')
+            self.assertFalse(user.is_active)
+            self.assertEqual(user.email, 'janedoe@example.com')
+
+    def test_signup_duplicate_email_shows_friendly_message(self):
+        User.objects.create_user(
+            username='existinguser',
+            email='duplicate@example.com',
+            password='Password123!'
+        )
+        data = {
+            'first_name': 'Another',
+            'last_name': 'User',
+            'username': 'newuser',
+            'email_address': 'duplicate@example.com',
+            'password': 'Password123!',
+            'confirm_password': 'Password123!',
+        }
+        response = self.client.post(self.register_url, data)
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'registration/register.html')
+        self.assertIn('message_email', response.context)
+        self.assertIn('email already exists', response.context['message_email'].lower())
+
+    def test_signup_duplicate_username_shows_friendly_message(self):
+        User.objects.create_user(
+            username='takenusername',
+            email='first@example.com',
+            password='Password123!'
+        )
+        data = {
+            'first_name': 'Another',
+            'last_name': 'User',
+            'username': 'takenusername',
+            'email_address': 'second@example.com',
+            'password': 'Password123!',
+            'confirm_password': 'Password123!',
+        }
+        response = self.client.post(self.register_url, data)
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'registration/register.html')
+        self.assertIn('message_username', response.context)
+        self.assertIn('username already exists', response.context['message_username'].lower())
+
+    def test_signup_missing_required_fields_shows_error_message(self):
+        data = {
+            'first_name': '',
+            'last_name': '',
+            'username': '',
+            'email_address': '',
+            'password': '',
+            'confirm_password': '',
+        }
+        response = self.client.post(self.register_url, data)
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'registration/register.html')
+
+    def test_signup_mail_failure_handling(self):
+        data = {
+            'first_name': 'Test',
+            'last_name': 'EmailFail',
+            'username': 'emailfailuser',
+            'email_address': 'fail@example.com',
+            'password': 'Password123!',
+            'confirm_password': 'Password123!',
+        }
+        with patch('activity.views.send_single_mail', side_effect=Exception('SMTP Connection Error')):
+            response = self.client.post(self.register_url, data)
+            self.assertEqual(response.status_code, 200)
+            self.assertTemplateUsed(response, 'registration/register.html')
+            # User should be cleaned up on failure
+            self.assertFalse(User.objects.filter(username='emailfailuser').exists())

activity/util.py

@@ -135,7 +135,7 @@ def in_groups(u):
 def group_required(*group_names, url):
     # Requires user membership in at least one of the groups passed in.
     def in_groups(u):
-        if u.is_authenticated():
+        if u.is_authenticated:
             if bool(u.groups.filter(name__in=group_names)) | u.is_superuser:
                 return True
             raise PermissionDenied
@@ -236,7 +236,13 @@ def send_invite_emails(subject, email_from, email_to, data):
         msg.send()
 
 
-def send_single_mail(subject, email_from, email_to, data, email_txt, email_html):
+import logging
+from django.conf import settings
+
+logger = logging.getLogger(__name__)
+
+
+def send_single_mail(subject, email_from, email_to, data, email_txt, email_html, fail_silently=False):
     """
     Send single email
     :param subject: email subject
@@ -245,27 +251,36 @@ def send_single_mail(subject, email_from, email_to, data, email_txt, email_html)
     :param data: context data
     :param email_txt: text email template
     :param email_html: html email template
+    :param fail_silently: whether to suppress exceptions
     """
     email_context = data
-    email_txt = loader.render_to_string(email_txt, email_context)
-    email_html = loader.get_template(email_html)
-    email_html_content = email_html.render(email_context)
+    email_txt_content = loader.render_to_string(email_txt, email_context)
+    email_html_template = loader.get_template(email_html)
+    email_html_content = email_html_template.render(email_context)
+
+    sender = getattr(settings, 'DEFAULT_FROM_EMAIL', None) or email_from or 'team.hikaya@gmail.com'
+    if '<' not in sender:
+        from_email_header = 'Hikaya <{}>'.format(sender)
+    else:
+        from_email_header = sender
 
     msg = EmailMultiAlternatives(
         subject,
-        email_txt,
-        'Hikaya <{}>'.format(email_from),
+        email_txt_content,
+        from_email_header,
         email_to
     )
     msg.attach_alternative(email_html_content, "text/html")
-    msg.send()
+    return msg.send(fail_silently=fail_silently)
 
 
 def user_signup_notification(user):
     url = os.environ.get('SLACK_REGISTRATION_WEBHOOK')
+    if not url:
+        return
     message = ("A new user has signed up on activity")
     title = ("New User Sign Up :zap:")
-    date = user.date_joined.strftime('%d-%m-%Y')
+    date = user.date_joined.strftime('%d-%m-%Y') if user.date_joined else ''
 
     slack_data = {
         "username": "new-user-notification",
@@ -294,8 +309,11 @@ def user_signup_notification(user):
             },
         ]
     }
-    byte_length = str(sys.getsizeof(slack_data))
-    headers = {'Content-Type': "application/json", 'Content-Length': byte_length}
-    response = requests.post(url, data=json.dumps(slack_data), headers=headers)
-    if response.status_code != 200:
-        raise Exception(response.status_code, response.text)
+    headers = {'Content-Type': "application/json"}
+    try:
+        response = requests.post(url, data=json.dumps(slack_data), headers=headers, timeout=5)
+        if response.status_code != 200:
+            logger.warning("Slack notification returned non-200 status: %s %s", response.status_code, response.text)
+    except Exception as e:
+        logger.warning("Slack registration webhook failed: %s", e)
+