fix: Fix welcome email bugs

Send only when new user is created
Require email address when user created

Resolves #32

people/admin.py

@@ -8,7 +8,12 @@ from django.contrib.auth.admin import UserAdmin
 from . import models
 
 
-admin.site.register(models.User, UserAdmin)
+@admin.register(models.User)
+class CustomUserAdmin(UserAdmin):
+    """Add email address field to new user form."""
+    add_fieldsets = UserAdmin.add_fieldsets + (
+        ('Details', {'fields': ('email', )}),
+    )  # yapf: disable
 
 
 @admin.register(models.Organisation)

people/apps.py

@@ -5,7 +5,7 @@ from django.conf import settings
 from django.core import serializers
 from django.db.models.signals import post_save
 
-logger = logging.getLogger(__name__)
+logger = logging.getLogger(__name__)  # pylint: disable=invalid-name
 
 
 def load_welcome_template_fixture(fixture_path) -> bool:
@@ -15,7 +15,8 @@ def load_welcome_template_fixture(fixture_path) -> bool:
             for deserialized in serializers.deserialize('json', f):
                 if deserialized.object.name == settings.TEMPLATE_WELCOME_EMAIL_NAME:
                     deserialized.save()
-                    logger.warning('Welcome email template \'%s\' loaded', deserialized.object.name)
+                    logger.warning('Welcome email template \'%s\' loaded',
+                                   deserialized.object.name)
                     return True
 
             return False
@@ -25,39 +26,44 @@ def load_welcome_template_fixture(fixture_path) -> bool:
         return False
 
 
-def send_welcome_email(sender, instance, **kwargs):
+def send_welcome_email(sender, instance, created, **kwargs):
     from post_office import models
 
+    if not created:
+        # If user already exists, don't send welcome message
+        return
+
     try:
         instance.send_welcome_email()
 
     except models.EmailTemplate.DoesNotExist:
-        logger.warning('Welcome email template \'%s\' not found - attempting to load from fixtures',
-                       settings.TEMPLATE_WELCOME_EMAIL_NAME)
-        
+        logger.warning(
+            'Welcome email template \'%s\' not found - attempting to load from fixtures',
+            settings.TEMPLATE_WELCOME_EMAIL_NAME)
+
         is_loaded = False
         if settings.CUSTOMISATION_NAME:
             # Customisation app present - try here first
             is_loaded |= load_welcome_template_fixture(
-                settings.BASE_DIR.joinpath('custom', 'fixtures', 'email_templates.json')
-            )
+                settings.BASE_DIR.joinpath('custom', 'fixtures',
+                                           'email_templates.json'))
 
         # |= operator shortcuts - only try here if we don't already have it
         is_loaded |= load_welcome_template_fixture(
-            settings.BASE_DIR.joinpath('people', 'fixtures', 'email_templates.json')
-        )
+            settings.BASE_DIR.joinpath('people', 'fixtures',
+                                       'email_templates.json'))
 
         if is_loaded:
             instance.send_welcome_email()
 
         else:
-            logger.error('Welcome email template \'%s\' not found', settings.TEMPLATE_WELCOME_EMAIL_NAME)
+            logger.error('Welcome email template \'%s\' not found',
+                         settings.TEMPLATE_WELCOME_EMAIL_NAME)
 
 
 class PeopleConfig(AppConfig):
     name = 'people'
-    
+
     def ready(self) -> None:
         # Activate signal handlers
-        post_save.connect(send_welcome_email,
-                          sender='people.user')
+        post_save.connect(send_welcome_email, sender='people.user')

people/migrations/0018_require_user_email.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.2.10 on 2020-05-28 15:17
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('people', '0017_answerset_replaced_timestamp'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='user',
+            name='email',
+            field=models.EmailField(max_length=254, verbose_name='email address'),
+        ),
+    ]

people/models/person.py

@@ -1,5 +1,8 @@
+import logging
+
 from django.conf import settings
 from django.contrib.auth.models import AbstractUser
+from django.core.exceptions import ValidationError
 from django.db import models
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
@@ -10,6 +13,8 @@ from post_office import mail
 
 from backports.db.models.enums import TextChoices
 
+logger = logging.getLogger(__name__)  # pylint: disable=invalid-name
+
 __all__ = [
     'User',
     'Organisation',
@@ -24,6 +29,8 @@ class User(AbstractUser):
     """
     Custom user model in case we need to make changes later.
     """
+    email = models.EmailField(_('email address'), blank=False, null=False)
+
     def has_person(self) -> bool:
         """
         Does this user have a linked :class:`Person` record?
@@ -38,15 +45,21 @@ class User(AbstractUser):
             'user': self,
         })
 
-        return False
+        logger.info('Sending welcome mail to user \'%s\'', self.username)
 
-        mail.send(
-            [self.email],
-            sender=settings.DEFAULT_FROM_EMAIL,
-            template=settings.TEMPLATE_WELCOME_EMAIL_NAME,
-            context=context,
-            priority='now'  # Send immediately - don't add to queue
-        )
+        try:
+            mail.send(
+                [self.email],
+                sender=settings.DEFAULT_FROM_EMAIL,
+                template=settings.TEMPLATE_WELCOME_EMAIL_NAME,
+                context=context,
+                priority='now'  # Send immediately - don't add to queue
+            )
+
+        except ValidationError:
+            logger.error(
+                'Sending welcome mail failed, invalid email for user \'%s\'',
+                self.username)
 
 
 class Organisation(models.Model):