From e0b30235206f0fde5cd56945402afec141803c14 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 22 Sep 2022 12:13:53 -0500 Subject: [PATCH 1/6] Add User and UserProfile models --- src/.flake8 | 2 + src/registrar/admin.py | 15 ++ src/registrar/apps.py | 17 ++ src/registrar/config/settings.py | 7 + src/registrar/migrations/0001_initial.py | 168 ++++++++++++++++++ ...file_cc_alter_userprofile_city_and_more.py | 63 +++++++ .../0003_alter_userprofile_street1.py | 18 ++ src/registrar/migrations/__init__.py | 0 src/registrar/models/__init__.py | 3 + src/registrar/models/models.py | 81 +++++++++ src/registrar/signals.py | 17 ++ 11 files changed, 391 insertions(+) create mode 100644 src/registrar/admin.py create mode 100644 src/registrar/apps.py create mode 100644 src/registrar/migrations/0001_initial.py create mode 100644 src/registrar/migrations/0002_alter_userprofile_cc_alter_userprofile_city_and_more.py create mode 100644 src/registrar/migrations/0003_alter_userprofile_street1.py create mode 100644 src/registrar/migrations/__init__.py create mode 100644 src/registrar/models/__init__.py create mode 100644 src/registrar/models/models.py create mode 100644 src/registrar/signals.py diff --git a/src/.flake8 b/src/.flake8 index 42ef88a71..8c4d4851a 100644 --- a/src/.flake8 +++ b/src/.flake8 @@ -2,3 +2,5 @@ max-line-length = 88 max-complexity = 10 extend-ignore = E203 +# migrations are auto-generated and often break rules +exclude=registrar/migrations/* diff --git a/src/registrar/admin.py b/src/registrar/admin.py new file mode 100644 index 000000000..dab331c22 --- /dev/null +++ b/src/registrar/admin.py @@ -0,0 +1,15 @@ +from django.contrib import admin +from django.contrib.auth.admin import UserAdmin +from .models import User, UserProfile + + +# edit a user's profile on the user page +class UserProfileInline(admin.StackedInline): + model = UserProfile + + +class MyUserAdmin(UserAdmin): + inlines = [UserProfileInline] + + +admin.site.register(User, MyUserAdmin) diff --git a/src/registrar/apps.py b/src/registrar/apps.py new file mode 100644 index 000000000..9f1b186ad --- /dev/null +++ b/src/registrar/apps.py @@ -0,0 +1,17 @@ +from django.apps import AppConfig + + +class RegistrarConfig(AppConfig): + + """Configure signal handling for our registrar Django application.""" + + name = "registrar" + + def ready(self): + """Runs when all Django applications have been loaded. + + We use it here to load signals that connect related models. + """ + # noqa here because we are importing something to make the signals + # get registered, but not using what we import + from . import signals # noqa diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 03dba9bc1..699bec794 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -54,6 +54,9 @@ BASE_DIR = path.resolve().parent.parent # SECURITY WARNING: don't run with debug turned on in production! DEBUG = env_debug +# Use our user model instead of the default +AUTH_USER_MODEL = "registrar.User" + # Applications are modular pieces of code. # They are provided by Django, by third-parties, or by yourself. @@ -504,6 +507,10 @@ if DEBUG: INSTALLED_APPS += ("nplusone.ext.django",) MIDDLEWARE += ("nplusone.ext.django.NPlusOneMiddleware",) NPLUSONE_RAISE = True + NPLUSONE_WHITELIST = [ + {"model": "admin.LogEntry", "field": "user"}, + {"model": "registrar.UserProfile"}, + ] # insert the amazing django-debug-toolbar INSTALLED_APPS += ("debug_toolbar",) diff --git a/src/registrar/migrations/0001_initial.py b/src/registrar/migrations/0001_initial.py new file mode 100644 index 000000000..df709410c --- /dev/null +++ b/src/registrar/migrations/0001_initial.py @@ -0,0 +1,168 @@ +# Generated by Django 4.1.1 on 2022-09-22 16:05 + +from django.conf import settings +import django.contrib.auth.models +import django.contrib.auth.validators +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ("auth", "0012_alter_user_first_name_max_length"), + ] + + operations = [ + migrations.CreateModel( + name="User", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("password", models.CharField(max_length=128, verbose_name="password")), + ( + "last_login", + models.DateTimeField( + blank=True, null=True, verbose_name="last login" + ), + ), + ( + "is_superuser", + models.BooleanField( + default=False, + help_text="Designates that this user has all permissions without explicitly assigning them.", + verbose_name="superuser status", + ), + ), + ( + "username", + models.CharField( + error_messages={ + "unique": "A user with that username already exists." + }, + help_text="Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.", + max_length=150, + unique=True, + validators=[ + django.contrib.auth.validators.UnicodeUsernameValidator() + ], + verbose_name="username", + ), + ), + ( + "first_name", + models.CharField( + blank=True, max_length=150, verbose_name="first name" + ), + ), + ( + "last_name", + models.CharField( + blank=True, max_length=150, verbose_name="last name" + ), + ), + ( + "email", + models.EmailField( + blank=True, max_length=254, verbose_name="email address" + ), + ), + ( + "is_staff", + models.BooleanField( + default=False, + help_text="Designates whether the user can log into this admin site.", + verbose_name="staff status", + ), + ), + ( + "is_active", + models.BooleanField( + default=True, + help_text="Designates whether this user should be treated as active. Unselect this instead of deleting accounts.", + verbose_name="active", + ), + ), + ( + "date_joined", + models.DateTimeField( + default=django.utils.timezone.now, verbose_name="date joined" + ), + ), + ( + "groups", + models.ManyToManyField( + blank=True, + help_text="The groups this user belongs to. A user will get all permissions granted to each of their groups.", + related_name="user_set", + related_query_name="user", + to="auth.group", + verbose_name="groups", + ), + ), + ( + "user_permissions", + models.ManyToManyField( + blank=True, + help_text="Specific permissions for this user.", + related_name="user_set", + related_query_name="user", + to="auth.permission", + verbose_name="user permissions", + ), + ), + ], + options={ + "verbose_name": "user", + "verbose_name_plural": "users", + "abstract": False, + }, + managers=[("objects", django.contrib.auth.models.UserManager())], + ), + migrations.CreateModel( + name="UserProfile", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ("street1", models.CharField(max_length=2)), + ("street2", models.CharField(max_length=2)), + ("street3", models.CharField(max_length=2)), + ("city", models.CharField(max_length=2)), + ("sp", models.CharField(max_length=2)), + ("pc", models.CharField(max_length=2)), + ("cc", models.CharField(max_length=2)), + ("voice", models.CharField(max_length=2)), + ("fax", models.CharField(max_length=2)), + ("email", models.CharField(max_length=2)), + ("display_name", models.TextField()), + ( + "user", + models.OneToOneField( + null=True, + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={"abstract": False}, + ), + ] diff --git a/src/registrar/migrations/0002_alter_userprofile_cc_alter_userprofile_city_and_more.py b/src/registrar/migrations/0002_alter_userprofile_cc_alter_userprofile_city_and_more.py new file mode 100644 index 000000000..e42b14ae1 --- /dev/null +++ b/src/registrar/migrations/0002_alter_userprofile_cc_alter_userprofile_city_and_more.py @@ -0,0 +1,63 @@ +# Generated by Django 4.1.1 on 2022-09-22 16:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0001_initial"), + ] + + operations = [ + migrations.AlterField( + model_name="userprofile", + name="cc", + field=models.TextField(blank=True), + ), + migrations.AlterField( + model_name="userprofile", + name="city", + field=models.TextField(blank=True), + ), + migrations.AlterField( + model_name="userprofile", + name="email", + field=models.TextField(blank=True), + ), + migrations.AlterField( + model_name="userprofile", + name="fax", + field=models.TextField(blank=True), + ), + migrations.AlterField( + model_name="userprofile", + name="pc", + field=models.TextField(blank=True), + ), + migrations.AlterField( + model_name="userprofile", + name="sp", + field=models.TextField(blank=True), + ), + migrations.AlterField( + model_name="userprofile", + name="street1", + field=models.TextField(), + ), + migrations.AlterField( + model_name="userprofile", + name="street2", + field=models.TextField(blank=True), + ), + migrations.AlterField( + model_name="userprofile", + name="street3", + field=models.TextField(blank=True), + ), + migrations.AlterField( + model_name="userprofile", + name="voice", + field=models.TextField(blank=True), + ), + ] diff --git a/src/registrar/migrations/0003_alter_userprofile_street1.py b/src/registrar/migrations/0003_alter_userprofile_street1.py new file mode 100644 index 000000000..84e7f485a --- /dev/null +++ b/src/registrar/migrations/0003_alter_userprofile_street1.py @@ -0,0 +1,18 @@ +# Generated by Django 4.1.1 on 2022-09-22 16:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0002_alter_userprofile_cc_alter_userprofile_city_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="userprofile", + name="street1", + field=models.TextField(blank=True), + ), + ] diff --git a/src/registrar/migrations/__init__.py b/src/registrar/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py new file mode 100644 index 000000000..852dbad16 --- /dev/null +++ b/src/registrar/models/__init__.py @@ -0,0 +1,3 @@ +from .models import User, UserProfile + +__all__ = [User, UserProfile] diff --git a/src/registrar/models/models.py b/src/registrar/models/models.py new file mode 100644 index 000000000..5f96fe0ff --- /dev/null +++ b/src/registrar/models/models.py @@ -0,0 +1,81 @@ +from django.contrib.auth.models import AbstractUser +from django.db import models + + +class User(AbstractUser): + """ + A custom user model that performs identically to the default user model + but can be customized later. + """ + + def __str__(self): + if self.userprofile.display_name: + return self.userprofile.display_name + else: + return self.username + + +class TimeStampedModel(models.Model): + """ + An abstract base model that provides self-updating + `created_at` and `updated_at` fields. + """ + + created_at = models.DateTimeField(auto_now_add=True) + updated_at = models.DateTimeField(auto_now=True) + + class Meta: + abstract = True + # don't put anything else here, it will be ignored + + +class AddressModel(models.Model): + """ + An abstract base model that provides common fields + for postal addresses. + """ + + # contact's street (null ok) + street1 = models.TextField(blank=True) + # contact's street (null ok) + street2 = models.TextField(blank=True) + # contact's street (null ok) + street3 = models.TextField(blank=True) + # contact's city + city = models.TextField(blank=True) + # contact's state or province (null ok) + sp = models.TextField(blank=True) + # contact's postal code (null ok) + pc = models.TextField(blank=True) + # contact's country code + cc = models.TextField(blank=True) + + class Meta: + abstract = True + # don't put anything else here, it will be ignored + + +class ContactModel(models.Model): + """ + An abstract base model that provides common fields + for contact information. + """ + + voice = models.TextField(blank=True) + fax = models.TextField(blank=True) + email = models.TextField(blank=True) + + class Meta: + abstract = True + # don't put anything else here, it will be ignored + + +class UserProfile(TimeStampedModel, ContactModel, AddressModel): + user = models.OneToOneField(User, null=True, on_delete=models.CASCADE) + display_name = models.TextField() + + def __str__(self): + if self.display_name: + return self.display_name + else: + return self.user.username diff --git a/src/registrar/signals.py b/src/registrar/signals.py new file mode 100644 index 000000000..913e07004 --- /dev/null +++ b/src/registrar/signals.py @@ -0,0 +1,17 @@ +from django.db.models.signals import post_save +from django.contrib.auth.models import User +from django.dispatch import receiver + +from .models import UserProfile + + +@receiver(post_save, sender=User) +def create_profile(sender, instance, created, **kwargs): + if created: + UserProfile.objects.create(user=instance) + + +@receiver(post_save, sender=User) +def save_profile(sender, instance, **kwargs): + # instance is a User, it has a profile from the one-to-one relation + instance.userprofile.save() From 5c3460782e970af0562d355b32615f41c6b04860 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 23 Sep 2022 13:27:57 -0500 Subject: [PATCH 2/6] Add page for editing UserProfile's display name. --- src/registrar/config/urls.py | 12 ++++-------- src/registrar/forms.py | 10 ++++++++++ src/registrar/templates/profile.html | 26 ++++++++++++++++++++++++++ src/registrar/tests/test_views.py | 21 +++++++++++++++++++++ src/registrar/views/profile.py | 20 ++++++++++++++++++++ 5 files changed, 81 insertions(+), 8 deletions(-) create mode 100644 src/registrar/forms.py create mode 100644 src/registrar/templates/profile.html create mode 100644 src/registrar/views/profile.py diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 613917001..e20bedc87 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -8,17 +8,13 @@ from django.conf import settings from django.contrib import admin from django.urls import include, path -from registrar.views import health, index +from registrar.views import health, index, profile urlpatterns = [ path("admin/", admin.site.urls), path("", index.index), path("health/", health.health), + path("edit_profile/", profile.edit_profile, name="edit-profile"), + # these views respect the DEBUG setting + path("__debug__/", include("debug_toolbar.urls")), ] - -if settings.DEBUG: - import debug_toolbar - - urlpatterns = [ - path("__debug__/", include(debug_toolbar.urls)), - ] + urlpatterns diff --git a/src/registrar/forms.py b/src/registrar/forms.py new file mode 100644 index 000000000..8e8e48fb9 --- /dev/null +++ b/src/registrar/forms.py @@ -0,0 +1,10 @@ +from django import forms + +from .models import UserProfile + +class EditProfileForm(forms.ModelForm): + display_name = forms.CharField(widget=forms.TextInput(attrs={'class': 'usa-input'}), label="Display Name") + + class Meta: + model = UserProfile + fields = ['display_name'] diff --git a/src/registrar/templates/profile.html b/src/registrar/templates/profile.html new file mode 100644 index 000000000..7539abb2f --- /dev/null +++ b/src/registrar/templates/profile.html @@ -0,0 +1,26 @@ +{% extends 'base.html' %} + +{% block title %} +Edit your User Profile +{% endblock title %} + +{% block content %} +
+ {% csrf_token %} +
+ Your profile +

+ Required fields are marked with an asterisk (*). +

+ {% for field in profile_form %} + + {{ field }} + {% endfor %} +
+ +
+{% endblock content %} diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 72aa19efc..1f88c99d4 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1,4 +1,7 @@ from django.test import Client, TestCase +from django.contrib.auth import get_user_model + +from registrar.models import UserProfile class HealthTest(TestCase): @@ -9,3 +12,21 @@ class HealthTest(TestCase): response = self.client.get("/health/") self.assertEqual(response.status_code, 200) self.assertContains(response, "OK") + + +class LoggedInTests(TestCase): + + def setUp(self): + username = "test_user" + first_name = "First" + last_name = "Last" + email = "info@example.com" + self.user = get_user_model().objects.create( + username=username, first_name=first_name, last_name=last_name, email=email + ) + profile = UserProfile.objects.create(user=self.user) + self.client.force_login(self.user) + + def test_edit_profile(self): + response = self.client.get("/edit_profile/") + self.assertContains(response, "Display Name") diff --git a/src/registrar/views/profile.py b/src/registrar/views/profile.py new file mode 100644 index 000000000..fad387e9e --- /dev/null +++ b/src/registrar/views/profile.py @@ -0,0 +1,20 @@ + +from django.shortcuts import redirect, render +from django.contrib.auth.decorators import login_required +from django.contrib import messages + +from ..forms import EditProfileForm + + +@login_required +def edit_profile(request): + if request.method == "POST": + # post to this view when changes are made + profile_form = EditProfileForm(request.POST, instance=request.user.userprofile) + if profile_form.is_valid(): + profile_form.save() + messages.success(request, 'Your profile is updated successfully') + return redirect(to='edit-profile') + else: + profile_form = EditProfileForm(instance=request.user.userprofile) + return render(request, 'profile.html', {'profile_form': profile_form}) From 60f282752bde8ab053494b131d6ffecf5f93b53b Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Mon, 26 Sep 2022 11:51:15 -0500 Subject: [PATCH 3/6] Respond to review feedback --- src/registrar/admin.py | 7 ++- src/registrar/config/settings.py | 6 +- src/registrar/config/urls.py | 1 - src/registrar/forms.py | 15 ++++- src/registrar/migrations/0001_initial.py | 30 +++++---- ...file_cc_alter_userprofile_city_and_more.py | 63 ------------------- .../0003_alter_userprofile_street1.py | 18 ------ src/registrar/models/__init__.py | 2 +- src/registrar/models/models.py | 10 ++- src/registrar/signals.py | 23 ++++--- src/registrar/tests/test_views.py | 3 +- src/registrar/views/profile.py | 10 +-- 12 files changed, 69 insertions(+), 119 deletions(-) delete mode 100644 src/registrar/migrations/0002_alter_userprofile_cc_alter_userprofile_city_and_more.py delete mode 100644 src/registrar/migrations/0003_alter_userprofile_street1.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index dab331c22..f2ada24e5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3,12 +3,17 @@ from django.contrib.auth.admin import UserAdmin from .models import User, UserProfile -# edit a user's profile on the user page class UserProfileInline(admin.StackedInline): + + """Edit a user's profile on the user page.""" + model = UserProfile class MyUserAdmin(UserAdmin): + + """Custom user admin class to use our inlines.""" + inlines = [UserProfileInline] diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 699bec794..e582da4ff 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -54,9 +54,6 @@ BASE_DIR = path.resolve().parent.parent # SECURITY WARNING: don't run with debug turned on in production! DEBUG = env_debug -# Use our user model instead of the default -AUTH_USER_MODEL = "registrar.User" - # Applications are modular pieces of code. # They are provided by Django, by third-parties, or by yourself. @@ -185,6 +182,9 @@ DATABASES = { # Specify default field type to use for primary keys DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField" +# Use our user model instead of the default +AUTH_USER_MODEL = "registrar.User" + # endregion # region: Email-------------------------------------------------------------### diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index e20bedc87..226e25beb 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -4,7 +4,6 @@ For more information see: https://docs.djangoproject.com/en/4.0/topics/http/urls/ """ -from django.conf import settings from django.contrib import admin from django.urls import include, path diff --git a/src/registrar/forms.py b/src/registrar/forms.py index 8e8e48fb9..09bbb72a6 100644 --- a/src/registrar/forms.py +++ b/src/registrar/forms.py @@ -2,9 +2,20 @@ from django import forms from .models import UserProfile + class EditProfileForm(forms.ModelForm): - display_name = forms.CharField(widget=forms.TextInput(attrs={'class': 'usa-input'}), label="Display Name") + + """Custom form class for editing a UserProfile. + + We can add whatever fields we want to this form and customize how they + are displayed. The form is rendered into a template `profile.html` by a + view called `edit_profile` in `profile.py`. + """ + + display_name = forms.CharField( + widget=forms.TextInput(attrs={"class": "usa-input"}), label="Display Name" + ) class Meta: model = UserProfile - fields = ['display_name'] + fields = ["display_name"] diff --git a/src/registrar/migrations/0001_initial.py b/src/registrar/migrations/0001_initial.py index df709410c..4440576a1 100644 --- a/src/registrar/migrations/0001_initial.py +++ b/src/registrar/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 4.1.1 on 2022-09-22 16:05 +# Generated by Django 4.1.1 on 2022-09-26 15:26 from django.conf import settings import django.contrib.auth.models @@ -127,7 +127,9 @@ class Migration(migrations.Migration): "verbose_name_plural": "users", "abstract": False, }, - managers=[("objects", django.contrib.auth.models.UserManager())], + managers=[ + ("objects", django.contrib.auth.models.UserManager()), + ], ), migrations.CreateModel( name="UserProfile", @@ -143,16 +145,16 @@ class Migration(migrations.Migration): ), ("created_at", models.DateTimeField(auto_now_add=True)), ("updated_at", models.DateTimeField(auto_now=True)), - ("street1", models.CharField(max_length=2)), - ("street2", models.CharField(max_length=2)), - ("street3", models.CharField(max_length=2)), - ("city", models.CharField(max_length=2)), - ("sp", models.CharField(max_length=2)), - ("pc", models.CharField(max_length=2)), - ("cc", models.CharField(max_length=2)), - ("voice", models.CharField(max_length=2)), - ("fax", models.CharField(max_length=2)), - ("email", models.CharField(max_length=2)), + ("street1", models.TextField(blank=True)), + ("street2", models.TextField(blank=True)), + ("street3", models.TextField(blank=True)), + ("city", models.TextField(blank=True)), + ("sp", models.TextField(blank=True)), + ("pc", models.TextField(blank=True)), + ("cc", models.TextField(blank=True)), + ("voice", models.TextField(blank=True)), + ("fax", models.TextField(blank=True)), + ("email", models.TextField(blank=True)), ("display_name", models.TextField()), ( "user", @@ -163,6 +165,8 @@ class Migration(migrations.Migration): ), ), ], - options={"abstract": False}, + options={ + "abstract": False, + }, ), ] diff --git a/src/registrar/migrations/0002_alter_userprofile_cc_alter_userprofile_city_and_more.py b/src/registrar/migrations/0002_alter_userprofile_cc_alter_userprofile_city_and_more.py deleted file mode 100644 index e42b14ae1..000000000 --- a/src/registrar/migrations/0002_alter_userprofile_cc_alter_userprofile_city_and_more.py +++ /dev/null @@ -1,63 +0,0 @@ -# Generated by Django 4.1.1 on 2022-09-22 16:51 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0001_initial"), - ] - - operations = [ - migrations.AlterField( - model_name="userprofile", - name="cc", - field=models.TextField(blank=True), - ), - migrations.AlterField( - model_name="userprofile", - name="city", - field=models.TextField(blank=True), - ), - migrations.AlterField( - model_name="userprofile", - name="email", - field=models.TextField(blank=True), - ), - migrations.AlterField( - model_name="userprofile", - name="fax", - field=models.TextField(blank=True), - ), - migrations.AlterField( - model_name="userprofile", - name="pc", - field=models.TextField(blank=True), - ), - migrations.AlterField( - model_name="userprofile", - name="sp", - field=models.TextField(blank=True), - ), - migrations.AlterField( - model_name="userprofile", - name="street1", - field=models.TextField(), - ), - migrations.AlterField( - model_name="userprofile", - name="street2", - field=models.TextField(blank=True), - ), - migrations.AlterField( - model_name="userprofile", - name="street3", - field=models.TextField(blank=True), - ), - migrations.AlterField( - model_name="userprofile", - name="voice", - field=models.TextField(blank=True), - ), - ] diff --git a/src/registrar/migrations/0003_alter_userprofile_street1.py b/src/registrar/migrations/0003_alter_userprofile_street1.py deleted file mode 100644 index 84e7f485a..000000000 --- a/src/registrar/migrations/0003_alter_userprofile_street1.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 4.1.1 on 2022-09-22 16:53 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0002_alter_userprofile_cc_alter_userprofile_city_and_more"), - ] - - operations = [ - migrations.AlterField( - model_name="userprofile", - name="street1", - field=models.TextField(blank=True), - ), - ] diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index 852dbad16..4723061e4 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -1,3 +1,3 @@ from .models import User, UserProfile -__all__ = [User, UserProfile] +__all__ = ["User", "UserProfile"] diff --git a/src/registrar/models/models.py b/src/registrar/models/models.py index 5f96fe0ff..bf6148dbf 100644 --- a/src/registrar/models/models.py +++ b/src/registrar/models/models.py @@ -1,3 +1,4 @@ +from django.core.exceptions import ObjectDoesNotExist from django.contrib.auth.models import AbstractUser from django.db import models @@ -9,9 +10,9 @@ class User(AbstractUser): """ def __str__(self): - if self.userprofile.display_name: + try: return self.userprofile.display_name - else: + except ObjectDoesNotExist: return self.username @@ -78,4 +79,7 @@ class UserProfile(TimeStampedModel, ContactModel, AddressModel): if self.display_name: return self.display_name else: - return self.user.username + try: + return self.user.username + except ObjectDoesNotExist: + return "No username" diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 913e07004..f227c6473 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -6,12 +6,19 @@ from .models import UserProfile @receiver(post_save, sender=User) -def create_profile(sender, instance, created, **kwargs): - if created: +def handle_profile(sender, instance, **kwargs): + + """Method for when a User is saved. + + If the user is being created, then create a matching UserProfile. Otherwise + save an updated profile or create one if it doesn't exist. + """ + + if kwargs.get("created", False): UserProfile.objects.create(user=instance) - - -@receiver(post_save, sender=User) -def save_profile(sender, instance, **kwargs): - # instance is a User, it has a profile from the one-to-one relation - instance.userprofile.save() + else: + # the user is not being created. + if hasattr(instance, "userprofile"): + instance.userprofile.save() + else: + UserProfile.objects.create(user=instance) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 1f88c99d4..981cae87a 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -15,7 +15,6 @@ class HealthTest(TestCase): class LoggedInTests(TestCase): - def setUp(self): username = "test_user" first_name = "First" @@ -24,7 +23,7 @@ class LoggedInTests(TestCase): self.user = get_user_model().objects.create( username=username, first_name=first_name, last_name=last_name, email=email ) - profile = UserProfile.objects.create(user=self.user) + self.profile = UserProfile.objects.create(user=self.user) self.client.force_login(self.user) def test_edit_profile(self): diff --git a/src/registrar/views/profile.py b/src/registrar/views/profile.py index fad387e9e..cfb7d63a0 100644 --- a/src/registrar/views/profile.py +++ b/src/registrar/views/profile.py @@ -1,4 +1,3 @@ - from django.shortcuts import redirect, render from django.contrib.auth.decorators import login_required from django.contrib import messages @@ -8,13 +7,16 @@ from ..forms import EditProfileForm @login_required def edit_profile(request): + + """View for a profile editing page.""" + if request.method == "POST": # post to this view when changes are made profile_form = EditProfileForm(request.POST, instance=request.user.userprofile) if profile_form.is_valid(): profile_form.save() - messages.success(request, 'Your profile is updated successfully') - return redirect(to='edit-profile') + messages.success(request, "Your profile is updated successfully") + return redirect(to="edit-profile") else: profile_form = EditProfileForm(instance=request.user.userprofile) - return render(request, 'profile.html', {'profile_form': profile_form}) + return render(request, "profile.html", {"profile_form": profile_form}) From c5f5b3ce31a5fbae3d969f0a68503180554b0ecc Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Tue, 27 Sep 2022 13:23:41 -0500 Subject: [PATCH 4/6] Fix signal handling with correct User class --- src/registrar/signals.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/signals.py b/src/registrar/signals.py index f227c6473..a39a90397 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -1,8 +1,7 @@ from django.db.models.signals import post_save -from django.contrib.auth.models import User from django.dispatch import receiver -from .models import UserProfile +from .models import User, UserProfile @receiver(post_save, sender=User) From c397597ae6585e7d3eeb5e1fcae6156e8d735501 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Tue, 27 Sep 2022 13:31:42 -0500 Subject: [PATCH 5/6] ADR to go with this PR --- .../decisions/0010-user-models.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 docs/architecture/decisions/0010-user-models.md diff --git a/docs/architecture/decisions/0010-user-models.md b/docs/architecture/decisions/0010-user-models.md new file mode 100644 index 000000000..d4d9e77aa --- /dev/null +++ b/docs/architecture/decisions/0010-user-models.md @@ -0,0 +1,40 @@ +# 10. Use custom User model with separate UserProfile + +Date: 2022-09-26 + +## Status + +Proposed + +## Context + +Django strongly recommends that a new project use a custom User model in their +first migration +. +This allows for future customization which would not be possible at a later +date if it isn’t done first. + +In order to separate authentication concerns from various user-related details +we might want to store, we want to decide how and where to store that +additional information. + +## Decision + +We use a custom user model derived from Django’s django.contrib.auth.User as +recommended along with a one-to-one related UserProfile model where we can +separately store any particular information about a user that we want to. That +includes contact information and the name that a person wants to use in the +application. + +Because the UserProfile is a place to store additional information about a +particular user, we mark each row in the UserProfile table to “cascade” deletes +so that when a single user is deleted, the matching UserProfile will also be +deleted. + +## Consequences + +If a user in our application is deleted (we don’t know at this point how or +when that might happen) then their profile would disappear. That means if the +same person returns to the application and makes a new account, there will be +no way to get back their UserProfile information and they will have to re-enter +it. From 86e57771d4c77e2bc42f1ca3f622fccac6aebba3 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Tue, 27 Sep 2022 13:39:24 -0500 Subject: [PATCH 6/6] Fix tests and re-number ADR --- .../decisions/{0010-user-models.md => 0012-user-models.md} | 2 +- src/registrar/tests/test_views.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) rename docs/architecture/decisions/{0010-user-models.md => 0012-user-models.md} (96%) diff --git a/docs/architecture/decisions/0010-user-models.md b/docs/architecture/decisions/0012-user-models.md similarity index 96% rename from docs/architecture/decisions/0010-user-models.md rename to docs/architecture/decisions/0012-user-models.md index d4d9e77aa..3e404106e 100644 --- a/docs/architecture/decisions/0010-user-models.md +++ b/docs/architecture/decisions/0012-user-models.md @@ -1,4 +1,4 @@ -# 10. Use custom User model with separate UserProfile +# 12. Use custom User model with separate UserProfile Date: 2022-09-26 diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 981cae87a..b13c84aad 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -23,7 +23,6 @@ class LoggedInTests(TestCase): self.user = get_user_model().objects.create( username=username, first_name=first_name, last_name=last_name, email=email ) - self.profile = UserProfile.objects.create(user=self.user) self.client.force_login(self.user) def test_edit_profile(self):