Skip to content

Commit

Permalink
Merge pull request #2413 from cisagov/meoward/2247-user-contact
Browse files Browse the repository at this point in the history
Issue #2247 - Remove user-contact connection
  • Loading branch information
dave-kennedy-ecs authored Jul 10, 2024
2 parents 4f4d3e7 + d981c41 commit 5cc8d93
Show file tree
Hide file tree
Showing 29 changed files with 195 additions and 900 deletions.
43 changes: 0 additions & 43 deletions docs/developer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -353,49 +353,6 @@ cf env getgov-{app name}

Then, copy the variables under the section labled `s3`.

## Signals
The application uses [Django signals](https://docs.djangoproject.com/en/5.0/topics/signals/). In particular, it uses a subset of prebuilt signals called [model signals](https://docs.djangoproject.com/en/5.0/ref/signals/#module-django.db.models.signals).

Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)"

In other words, signals are a mechanism that allows different parts of an application to communicate with each other by sending and receiving notifications when events occur. When an event occurs (such as creating, updating, or deleting a record), signals can automatically trigger specific actions in response. This allows different parts of an application to stay synchronized without tightly coupling the component.

### Rules of use
When using signals, try to adhere to these guidelines:
1. Don't use signals when you can use another method, such as an override of `save()` or `__init__`.
2. Document its usage in this readme (or another centralized location), as well as briefly on the underlying class it is associated with. For instance, since the `handle_profile` directly affects the class `Contact`, the class description notes this and links to [signals.py](../../src/registrar/signals.py).
3. Where possible, avoid chaining signals together (i.e. a signal that calls a signal). If this has to be done, clearly document the flow.
4. Minimize logic complexity within the signal as much as possible.

### When should you use signals?
Generally, you would use signals when you want an event to be synchronized across multiple areas of code at once (such as with two models or more models at once) in a way that would otherwise be difficult to achieve by overriding functions.

However, in most scenarios, if you can get away with avoiding signals - you should. The reasoning for this is that [signals give the appearance of loose coupling, but they can quickly lead to code that is hard to understand, adjust and debug](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch).

Consider using signals when:
1. Synchronizing events across multiple models or areas of code.
2. Performing logic before or after saving a model to the database (when otherwise difficult through `save()`).
3. Encountering an import loop when overriding functions such as `save()`.
4. You are otherwise unable to achieve the intended behavior by overrides or other means.
5. (Rare) Offloading tasks when multi-threading.

For the vast majority of use cases, the [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) and [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) signals are sufficient in terms of model-to-model management.

### Where should you use them?
This project compiles signals in a unified location to maintain readability. If you are adding a signal or otherwise utilizing one, you should always define them in [signals.py](../../src/registrar/signals.py). Except under rare circumstances, this should be adhered to for the reasons mentioned above.

### How are we currently using signals?
At the time of writing, we currently only use signals for the Contact and User objects when synchronizing data returned from Login.gov. This is because the `Contact` object holds information that the user specified in our system, whereas the `User` object holds information that was specified in Login.gov.

To keep our signal usage coherent and well-documented, add to this document when a new function is added for ease of reference and use.

#### handle_profile
This function is triggered by the post_save event on the User model, designed to manage the synchronization between User and Contact entities. It operates under the following conditions:

1. For New Users: Upon the creation of a new user, it checks for an existing `Contact` by email. If no matching contact is found, it creates a new Contact using the user's details from Login.gov. If a matching contact is found, it associates this contact with the user. In cases where multiple contacts with the same email exist, it logs a warning and associates the first contact found.

2. For Existing Users: For users logging in subsequent times, the function ensures that any updates from Login.gov are applied to the associated User record. However, it does not alter any existing Contact records.

## Disable email sending (toggling the disable_email_sending flag)
1. On the app, navigate to `\admin`.
2. Under models, click `Waffle flags`.
Expand Down
48 changes: 3 additions & 45 deletions src/registrar/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -598,33 +598,6 @@ def get_filters(self, request):
return filters


class UserContactInline(admin.StackedInline):
"""Edit a user's profile on the user page."""

model = models.Contact

# Read only that we'll leverage for CISA Analysts
analyst_readonly_fields = [
"user",
"email",
]

def get_readonly_fields(self, request, obj=None):
"""Set the read-only state on form elements.
We have 1 conditions that determine which fields are read-only:
admin user permissions.
"""

readonly_fields = list(self.readonly_fields)

if request.user.has_perm("registrar.full_access_permission"):
return readonly_fields
# Return restrictive Read-only fields for analysts and
# users who might not belong to groups
readonly_fields.extend([field for field in self.analyst_readonly_fields])
return readonly_fields # Read-only fields for analysts


class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin):
"""Custom user admin class to use our inlines."""

Expand All @@ -641,8 +614,6 @@ class Meta:

_meta = Meta()

inlines = [UserContactInline]

list_display = (
"username",
"overridden_email_field",
Expand Down Expand Up @@ -920,30 +891,20 @@ class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin):
list_display = [
"name",
"email",
"user_exists",
]
# this ordering effects the ordering of results
# in autocomplete_fields for user
# in autocomplete_fields
ordering = ["first_name", "last_name", "email"]

fieldsets = [
(
None,
{"fields": ["user", "first_name", "middle_name", "last_name", "title", "email", "phone"]},
{"fields": ["first_name", "middle_name", "last_name", "title", "email", "phone"]},
)
]

autocomplete_fields = ["user"]

change_form_template = "django/admin/email_clipboard_change_form.html"

def user_exists(self, obj):
"""Check if the Contact has a related User"""
return "Yes" if obj.user is not None else "No"

user_exists.short_description = "Is user" # type: ignore
user_exists.admin_order_field = "user" # type: ignore

# We name the custom prop 'contact' because linter
# is not allowing a short_description attr on it
# This gets around the linter limitation, for now.
Expand All @@ -961,10 +922,7 @@ def name(self, obj: models.Contact):
name.admin_order_field = "first_name" # type: ignore

# Read only that we'll leverage for CISA Analysts
analyst_readonly_fields = [
"user",
"email",
]
analyst_readonly_fields: list[str] = ["email"]

def get_readonly_fields(self, request, obj=None):
"""Set the read-only state on form elements.
Expand Down
9 changes: 0 additions & 9 deletions src/registrar/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,3 @@ 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
2 changes: 1 addition & 1 deletion src/registrar/forms/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
NameserverFormset,
DomainSecurityEmailForm,
DomainOrgNameAddressForm,
ContactForm,
UserForm,
SeniorOfficialContactForm,
DomainDnssecForm,
DomainDsdataFormset,
Expand Down
59 changes: 58 additions & 1 deletion src/registrar/forms/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
SecurityEmailErrorCodes,
)

from ..models import Contact, DomainInformation, Domain
from ..models import Contact, DomainInformation, Domain, User
from .common import (
ALGORITHM_CHOICES,
DIGEST_TYPE_CHOICES,
Expand Down Expand Up @@ -203,6 +203,63 @@ def clean(self):
)


class UserForm(forms.ModelForm):
"""Form for updating users."""

email = forms.EmailField(max_length=None)

class Meta:
model = User
fields = ["first_name", "middle_name", "last_name", "title", "email", "phone"]
widgets = {
"first_name": forms.TextInput,
"middle_name": forms.TextInput,
"last_name": forms.TextInput,
"title": forms.TextInput,
"email": forms.EmailInput,
"phone": RegionalPhoneNumberWidget,
}

# the database fields have blank=True so ModelForm doesn't create
# required fields by default. Use this list in __init__ to mark each
# of these fields as required
required = ["first_name", "last_name", "title", "email", "phone"]

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# take off maxlength attribute for the phone number field
# which interferes with out input_with_errors template tag
self.fields["phone"].widget.attrs.pop("maxlength", None)

# Define a custom validator for the email field with a custom error message
email_max_length_validator = MaxLengthValidator(320, message="Response must be less than 320 characters.")
self.fields["email"].validators.append(email_max_length_validator)

for field_name in self.required:
self.fields[field_name].required = True

# Set custom form label
self.fields["middle_name"].label = "Middle name (optional)"

# Set custom error messages
self.fields["first_name"].error_messages = {"required": "Enter your first name / given name."}
self.fields["last_name"].error_messages = {"required": "Enter your last name / family name."}
self.fields["title"].error_messages = {
"required": "Enter your title or role in your organization (e.g., Chief Information Officer)"
}
self.fields["email"].error_messages = {
"required": "Enter your email address in the required format, like name@example.com."
}
self.fields["phone"].error_messages["required"] = "Enter your phone number."
self.domainInfo = None

def set_domain_info(self, domainInfo):
"""Set the domain information for the form.
The form instance is associated with the contact itself. In order to access the associated
domain information object, this needs to be set in the form by the view."""
self.domainInfo = domainInfo


class ContactForm(forms.ModelForm):
"""Form for updating contacts."""

Expand Down
4 changes: 2 additions & 2 deletions src/registrar/forms/user_profile.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django import forms

from registrar.models.contact import Contact
from registrar.models.user import User

from django.core.validators import MaxLengthValidator
from phonenumber_field.widgets import RegionalPhoneNumberWidget
Expand All @@ -13,7 +13,7 @@ class UserProfileForm(forms.ModelForm):
redirect = forms.CharField(widget=forms.HiddenInput(), required=False)

class Meta:
model = Contact
model = User
fields = ["first_name", "middle_name", "last_name", "title", "email", "phone"]
widgets = {
"first_name": forms.TextInput,
Expand Down
Loading

0 comments on commit 5cc8d93

Please sign in to comment.