From 5cba82b34382175bfd1914ea20f8bd13fac21036 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 18 Apr 2024 15:30:45 -0600 Subject: [PATCH 01/75] Basic logic --- src/registrar/admin.py | 4 +- src/registrar/models/user.py | 99 ++++++++++++++----- .../admin/includes/contact_detail_list.html | 2 +- .../admin/includes/detail_table_fieldset.html | 4 +- 4 files changed, 83 insertions(+), 26 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b62ea80b8..6869df94a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -490,7 +490,7 @@ class MyUserAdmin(BaseUserAdmin): fieldsets = ( ( None, - {"fields": ("username", "password", "status")}, + {"fields": ("username", "password", "status", "verification_type")}, ), ("Personal Info", {"fields": ("first_name", "last_name", "email")}), ( @@ -508,6 +508,8 @@ class MyUserAdmin(BaseUserAdmin): ("Important dates", {"fields": ("last_login", "date_joined")}), ) + readonly_fields = ("verification_type") + # Hide Username (uuid), Groups and Permissions # Q: Now that we're using Groups and Permissions, # do we expose those to analysts to view? diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 2688ef57f..c69672100 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -1,3 +1,4 @@ +from enum import Enum import logging from django.contrib.auth.models import AbstractUser @@ -23,6 +24,16 @@ class User(AbstractUser): but can be customized later. """ + class VerificationTypeChoices(models.TextChoices): + """ + Users achieve access to our system in a few different ways. + These choices reflect those pathways. + """ + GRANDFATHERED = "grandfathered", "Legacy user" + VERIFIED_BY_STAFF = "verified_by_staff", "Verified by staff" + REGULAR = "regular", "Verified by Login.gov" + INVITED = "invited", "Invited by a domain manager" + # #### Constants for choice fields #### RESTRICTED = "restricted" STATUS_CHOICES = ((RESTRICTED, RESTRICTED),) @@ -48,6 +59,13 @@ class User(AbstractUser): db_index=True, ) + verification_type = models.CharField( + choices=VerificationTypeChoices, + null=True, + blank=True, + help_text="The means through which this user was verified", + ) + def __str__(self): # this info is pulled from Login.gov if self.first_name or self.last_name: @@ -95,6 +113,22 @@ class User(AbstractUser): def has_contact_info(self): return bool(self.contact.title or self.contact.email or self.contact.phone) + + @classmethod + def get_existing_user_from_uuid(cls, uuid): + existing_user = None + try: + existing_user = cls.objects.get(username=uuid) + if existing_user and UserDomainRole.objects.filter(user=existing_user).exists(): + return (False, existing_user) + except cls.DoesNotExist: + # Do nothing when the user is not found, as we're checking for existence. + pass + except Exception as err: + raise err + + return (True, existing_user) + @classmethod def needs_identity_verification(cls, email, uuid): """A method used by our oidc classes to test whether a user needs email/uuid verification @@ -102,33 +136,52 @@ class User(AbstractUser): # An existing user who is a domain manager of a domain (that is, # they have an entry in UserDomainRole for their User) - try: - existing_user = cls.objects.get(username=uuid) - if existing_user and UserDomainRole.objects.filter(user=existing_user).exists(): - return False - except cls.DoesNotExist: - # Do nothing when the user is not found, as we're checking for existence. - pass - except Exception as err: - raise err + user_exists, existing_user = cls.existing_user(uuid) + if not user_exists: + return False - # A new incoming user who is a domain manager for one of the domains - # that we inputted from Verisign (that is, their email address appears - # in the username field of a TransitionDomain) + # The user needs identity verification if they don't meet + # any special criteria, i.e. we are validating them "regularly" + existing_user.verification_type = cls.get_verification_type_from_email(email) + return existing_user.verification_type == cls.VerificationTypeChoices.REGULAR + + @classmethod + def get_verification_type_from_email(cls, email, invitation_status=DomainInvitation.DomainInvitationStatus.INVITED): + """Retrieves the verification type based off of a provided email address""" + + verification_type = None if TransitionDomain.objects.filter(username=email).exists(): - return False + # A new incoming user who is a domain manager for one of the domains + # that we inputted from Verisign (that is, their email address appears + # in the username field of a TransitionDomain) + verification_type = cls.VerificationTypeChoices.GRANDFATHERED + elif VerifiedByStaff.objects.filter(email=email).exists(): + # New users flagged by Staff to bypass ial2 + verification_type = cls.VerificationTypeChoices.VERIFIED_BY_STAFF + elif DomainInvitation.objects.filter(email=email, status=invitation_status).exists(): + # A new incoming user who is being invited to be a domain manager (that is, + # their email address is in DomainInvitation for an invitation that is not yet "retrieved"). + verification_type = cls.VerificationTypeChoices.INVITED + else: + verification_type = cls.VerificationTypeChoices.REGULAR + + return verification_type - # New users flagged by Staff to bypass ial2 - if VerifiedByStaff.objects.filter(email=email).exists(): - return False + def user_verification_type(self, check_if_user_exists=False): + if self.verification_type is None: + # Would need to check audit log + retrieved = DomainInvitation.DomainInvitationStatus.RETRIEVED + user_exists, _ = self.existing_user(self.username) + verification_type = self.get_verification_type_from_email(self.email, invitation_status=retrieved) - # A new incoming user who is being invited to be a domain manager (that is, - # their email address is in DomainInvitation for an invitation that is not yet "retrieved"). - invited = DomainInvitation.DomainInvitationStatus.INVITED - if DomainInvitation.objects.filter(email=email, status=invited).exists(): - return False - - return True + # This should check if the type is unknown, use check_if_user_exists? + if verification_type == self.VerificationTypeChoices.REGULAR and not user_exists: + raise ValueError(f"No verification_type was found for {self} with id: {self.pk}") + else: + self.verification_type = verification_type + return self.verification_type + else: + return self.verification_type def check_domain_invitations_on_login(self): """When a user first arrives on the site, we need to retrieve any domain diff --git a/src/registrar/templates/django/admin/includes/contact_detail_list.html b/src/registrar/templates/django/admin/includes/contact_detail_list.html index 0ac9c4c49..5ac5452e3 100644 --- a/src/registrar/templates/django/admin/includes/contact_detail_list.html +++ b/src/registrar/templates/django/admin/includes/contact_detail_list.html @@ -3,7 +3,7 @@
{% if show_formatted_name %} - {% if contact.get_formatted_name %} + {% if user.get_formatted_name %} {{ user.get_formatted_name }}
{% else %} None
diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index fb7303352..7b468faec 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -6,7 +6,9 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endcomment %} {% block field_readonly %} {% with all_contacts=original_object.other_contacts.all %} - {% if field.field.name == "other_contacts" %} + {% if field.field.name == "creator" %} +
{{ field.contents }} ({{ user.verification_type }})
+ {% elif field.field.name == "other_contacts" %} {% if all_contacts.count > 2 %}
{% for contact in all_contacts %} From 3e7f143a1e8911f6dd70878a01154a3b8873fcd0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 19 Apr 2024 15:30:37 -0600 Subject: [PATCH 02/75] Fix verification type --- src/djangooidc/views.py | 3 ++ src/registrar/admin.py | 15 ++++++---- src/registrar/fixtures_users.py | 4 +++ .../migrations/0085_user_verification_type.py | 28 +++++++++++++++++++ src/registrar/models/user.py | 18 ++++++------ 5 files changed, 53 insertions(+), 15 deletions(-) create mode 100644 src/registrar/migrations/0085_user_verification_type.py diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index ab81ccff1..b887b7a14 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -99,8 +99,11 @@ def login_callback(request): return CLIENT.create_authn_request(request.session) user = authenticate(request=request, **userinfo) if user: + # Set the verification type + user.set_user_verification_type() login(request, user) logger.info("Successfully logged in user %s" % user) + # Clear the flag if the exception is not caught request.session.pop("redirect_attempted", None) return redirect(request.session.get("next", "/")) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 6869df94a..dd016b470 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -508,7 +508,7 @@ class MyUserAdmin(BaseUserAdmin): ("Important dates", {"fields": ("last_login", "date_joined")}), ) - readonly_fields = ("verification_type") + readonly_fields = ("verification_type",) # Hide Username (uuid), Groups and Permissions # Q: Now that we're using Groups and Permissions, @@ -516,7 +516,7 @@ class MyUserAdmin(BaseUserAdmin): analyst_fieldsets = ( ( None, - {"fields": ("password", "status")}, + {"fields": ("password", "status", "verification_type")}, ), ("Personal Info", {"fields": ("first_name", "last_name", "email")}), ( @@ -636,11 +636,14 @@ class MyUserAdmin(BaseUserAdmin): return [] def get_readonly_fields(self, request, obj=None): + readonly_fields = list(self.readonly_fields) + if request.user.has_perm("registrar.full_access_permission"): - return () # No read-only fields for all access users - # Return restrictive Read-only fields for analysts and - # users who might not belong to groups - return self.analyst_readonly_fields + return readonly_fields + else: + # Return restrictive Read-only fields for analysts and + # users who might not belong to groups + return self.analyst_readonly_fields class HostIPInline(admin.StackedInline): diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index 99fe4910e..4c2b85233 100644 --- a/src/registrar/fixtures_users.py +++ b/src/registrar/fixtures_users.py @@ -6,6 +6,7 @@ from registrar.models import ( User, UserGroup, ) +from registrar.models.verified_by_staff import VerifiedByStaff fake = Faker() logger = logging.getLogger(__name__) @@ -187,6 +188,9 @@ class UserFixture: logger.info(f"Going to load {len(users)} users in group {group_name}") for user_data in users: try: + + # TODO - Add the fixture user to the VerifiedByStaff table + # (To track how this user was verified) user, _ = User.objects.get_or_create(username=user_data["username"]) user.is_superuser = False user.first_name = user_data["first_name"] diff --git a/src/registrar/migrations/0085_user_verification_type.py b/src/registrar/migrations/0085_user_verification_type.py new file mode 100644 index 000000000..2790ea3b7 --- /dev/null +++ b/src/registrar/migrations/0085_user_verification_type.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.10 on 2024-04-19 21:02 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0084_create_groups_v11"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="verification_type", + field=models.CharField( + blank=True, + choices=[ + ("grandfathered", "Legacy user"), + ("verified_by_staff", "Verified by staff"), + ("regular", "Verified by Login.gov"), + ("invited", "Invited by a domain manager"), + ], + help_text="The means through which this user was verified", + null=True, + ), + ), + ] diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index c69672100..9320a80ff 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -60,7 +60,7 @@ class User(AbstractUser): ) verification_type = models.CharField( - choices=VerificationTypeChoices, + choices=VerificationTypeChoices.choices, null=True, blank=True, help_text="The means through which this user was verified", @@ -115,19 +115,19 @@ class User(AbstractUser): @classmethod - def get_existing_user_from_uuid(cls, uuid): + def existing_user(cls, uuid): existing_user = None try: existing_user = cls.objects.get(username=uuid) if existing_user and UserDomainRole.objects.filter(user=existing_user).exists(): - return (False, existing_user) + return False except cls.DoesNotExist: # Do nothing when the user is not found, as we're checking for existence. pass except Exception as err: raise err - return (True, existing_user) + return True @classmethod def needs_identity_verification(cls, email, uuid): @@ -136,14 +136,14 @@ class User(AbstractUser): # An existing user who is a domain manager of a domain (that is, # they have an entry in UserDomainRole for their User) - user_exists, existing_user = cls.existing_user(uuid) + user_exists = cls.existing_user(uuid) if not user_exists: return False # The user needs identity verification if they don't meet # any special criteria, i.e. we are validating them "regularly" - existing_user.verification_type = cls.get_verification_type_from_email(email) - return existing_user.verification_type == cls.VerificationTypeChoices.REGULAR + verification_type = cls.get_verification_type_from_email(email) + return verification_type == cls.VerificationTypeChoices.REGULAR @classmethod def get_verification_type_from_email(cls, email, invitation_status=DomainInvitation.DomainInvitationStatus.INVITED): @@ -167,11 +167,11 @@ class User(AbstractUser): return verification_type - def user_verification_type(self, check_if_user_exists=False): + def set_user_verification_type(self): if self.verification_type is None: # Would need to check audit log retrieved = DomainInvitation.DomainInvitationStatus.RETRIEVED - user_exists, _ = self.existing_user(self.username) + user_exists = self.existing_user(self.username) verification_type = self.get_verification_type_from_email(self.email, invitation_status=retrieved) # This should check if the type is unknown, use check_if_user_exists? From a047380b59111916522de1d3ef605b9f793b0467 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 19 Apr 2024 15:44:26 -0600 Subject: [PATCH 03/75] Fix logic --- src/djangooidc/views.py | 7 +++++-- src/registrar/models/user.py | 18 ++++-------------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index b887b7a14..12b82f242 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -99,8 +99,11 @@ def login_callback(request): return CLIENT.create_authn_request(request.session) user = authenticate(request=request, **userinfo) if user: - # Set the verification type - user.set_user_verification_type() + # Set the verification type if it doesn't already exist + if not user.verification_type: + user.set_user_verification_type() + user.save() + login(request, user) logger.info("Successfully logged in user %s" % user) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 9320a80ff..423f93595 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -168,20 +168,10 @@ class User(AbstractUser): return verification_type def set_user_verification_type(self): - if self.verification_type is None: - # Would need to check audit log - retrieved = DomainInvitation.DomainInvitationStatus.RETRIEVED - user_exists = self.existing_user(self.username) - verification_type = self.get_verification_type_from_email(self.email, invitation_status=retrieved) - - # This should check if the type is unknown, use check_if_user_exists? - if verification_type == self.VerificationTypeChoices.REGULAR and not user_exists: - raise ValueError(f"No verification_type was found for {self} with id: {self.pk}") - else: - self.verification_type = verification_type - return self.verification_type - else: - return self.verification_type + # Would need to check audit log + retrieved = DomainInvitation.DomainInvitationStatus.RETRIEVED + verification_type = self.get_verification_type_from_email(self.email, invitation_status=retrieved) + self.verification_type = verification_type def check_domain_invitations_on_login(self): """When a user first arrives on the site, we need to retrieve any domain From 8b27c44b89f30632018f627fa6d6df882f5b3868 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 19 Apr 2024 15:47:03 -0600 Subject: [PATCH 04/75] Cleanup --- src/registrar/models/user.py | 40 +++++++++++++++--------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 423f93595..b67421d3f 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -113,10 +113,13 @@ class User(AbstractUser): def has_contact_info(self): return bool(self.contact.title or self.contact.email or self.contact.phone) - @classmethod - def existing_user(cls, uuid): - existing_user = None + def needs_identity_verification(cls, email, uuid): + """A method used by our oidc classes to test whether a user needs email/uuid verification + or the full identity PII verification""" + + # An existing user who is a domain manager of a domain (that is, + # they have an entry in UserDomainRole for their User) try: existing_user = cls.objects.get(username=uuid) if existing_user and UserDomainRole.objects.filter(user=existing_user).exists(): @@ -126,24 +129,21 @@ class User(AbstractUser): pass except Exception as err: raise err - - return True - @classmethod - def needs_identity_verification(cls, email, uuid): - """A method used by our oidc classes to test whether a user needs email/uuid verification - or the full identity PII verification""" - - # An existing user who is a domain manager of a domain (that is, - # they have an entry in UserDomainRole for their User) - user_exists = cls.existing_user(uuid) - if not user_exists: - return False + # We can't set the verification type here because the user may not + # always exist at this point. We do it down the line. + verification_type = cls.get_verification_type_from_email(email) # The user needs identity verification if they don't meet # any special criteria, i.e. we are validating them "regularly" - verification_type = cls.get_verification_type_from_email(email) - return verification_type == cls.VerificationTypeChoices.REGULAR + needs_verification = verification_type == cls.VerificationTypeChoices.REGULAR + return needs_verification + + def set_user_verification_type(self): + # Would need to check audit log + retrieved = DomainInvitation.DomainInvitationStatus.RETRIEVED + verification_type = self.get_verification_type_from_email(self.email, invitation_status=retrieved) + self.verification_type = verification_type @classmethod def get_verification_type_from_email(cls, email, invitation_status=DomainInvitation.DomainInvitationStatus.INVITED): @@ -167,12 +167,6 @@ class User(AbstractUser): return verification_type - def set_user_verification_type(self): - # Would need to check audit log - retrieved = DomainInvitation.DomainInvitationStatus.RETRIEVED - verification_type = self.get_verification_type_from_email(self.email, invitation_status=retrieved) - self.verification_type = verification_type - def check_domain_invitations_on_login(self): """When a user first arrives on the site, we need to retrieve any domain invitations that match their email address.""" From aa9127875ad29181c838953520d15d858693fb48 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:40:21 -0600 Subject: [PATCH 05/75] Refine logic for fixture users --- src/djangooidc/views.py | 12 ++++++++++-- src/registrar/fixtures_users.py | 7 ++++--- src/registrar/models/user.py | 4 ++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index 12b82f242..815df4ecf 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -99,8 +99,16 @@ def login_callback(request): return CLIENT.create_authn_request(request.session) user = authenticate(request=request, **userinfo) if user: - # Set the verification type if it doesn't already exist - if not user.verification_type: + + # Fixture users kind of exist in a superposition of verification types, + # because while the system "verified" them, if they login, + # we don't know how the user themselves was verified through login.gov until + # they actually try logging in. This edge-case only matters in non-production environments. + fixture_user = User.VerificationTypeChoices.FIXTURE_USER + is_fixture_user = user.verification_type and user.verification_type == fixture_user + + # Set the verification type if it doesn't already exist or if its a fixture user + if not user.verification_type or is_fixture_user: user.set_user_verification_type() user.save() diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index 4c2b85233..a901b83e6 100644 --- a/src/registrar/fixtures_users.py +++ b/src/registrar/fixtures_users.py @@ -188,9 +188,6 @@ class UserFixture: logger.info(f"Going to load {len(users)} users in group {group_name}") for user_data in users: try: - - # TODO - Add the fixture user to the VerifiedByStaff table - # (To track how this user was verified) user, _ = User.objects.get_or_create(username=user_data["username"]) user.is_superuser = False user.first_name = user_data["first_name"] @@ -199,6 +196,10 @@ class UserFixture: user.email = user_data["email"] user.is_staff = True user.is_active = True + # This verification type will get reverted to "regular" (or whichever is applicables) + # once the user logs in for the first time (as they then got verified through different means). + # In the meantime, we can still describe how the user got here in the first place. + user.verification_type = User.VerificationTypeChoices.FIXTURE_USER group = UserGroup.objects.get(name=group_name) user.groups.add(group) user.save() diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index b67421d3f..79bf905b9 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -33,6 +33,10 @@ class User(AbstractUser): VERIFIED_BY_STAFF = "verified_by_staff", "Verified by staff" REGULAR = "regular", "Verified by Login.gov" INVITED = "invited", "Invited by a domain manager" + # We need a type for fixture users (rather than using verified by staff) + # because those users still do get "verified" through normal means + # after they login. + FIXTURE_USER = "fixture_user", "Created by fixtures" # #### Constants for choice fields #### RESTRICTED = "restricted" From 68e44eb98eb936ad2a1e8c0c219a92eb86b48d47 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:41:08 -0600 Subject: [PATCH 06/75] Fix migrations after merge --- ...r_verification_type.py => 0087_user_verification_type.py} | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) rename src/registrar/migrations/{0085_user_verification_type.py => 0087_user_verification_type.py} (78%) diff --git a/src/registrar/migrations/0085_user_verification_type.py b/src/registrar/migrations/0087_user_verification_type.py similarity index 78% rename from src/registrar/migrations/0085_user_verification_type.py rename to src/registrar/migrations/0087_user_verification_type.py index 2790ea3b7..599d067d5 100644 --- a/src/registrar/migrations/0085_user_verification_type.py +++ b/src/registrar/migrations/0087_user_verification_type.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-04-19 21:02 +# Generated by Django 4.2.10 on 2024-04-22 16:40 from django.db import migrations, models @@ -6,7 +6,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("registrar", "0084_create_groups_v11"), + ("registrar", "0086_domaininformation_updated_federal_agency_and_more"), ] operations = [ @@ -20,6 +20,7 @@ class Migration(migrations.Migration): ("verified_by_staff", "Verified by staff"), ("regular", "Verified by Login.gov"), ("invited", "Invited by a domain manager"), + ("fixture_user", "Created by fixtures"), ], help_text="The means through which this user was verified", null=True, From 2a7eb6db817d0bd38b4c08431bc788ba02358969 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 22 Apr 2024 11:08:59 -0600 Subject: [PATCH 07/75] Fix test + lint --- src/registrar/fixtures_users.py | 2 +- src/registrar/models/user.py | 9 +++++---- src/registrar/tests/test_admin.py | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index a901b83e6..9669ce071 100644 --- a/src/registrar/fixtures_users.py +++ b/src/registrar/fixtures_users.py @@ -6,7 +6,7 @@ from registrar.models import ( User, UserGroup, ) -from registrar.models.verified_by_staff import VerifiedByStaff + fake = Faker() logger = logging.getLogger(__name__) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 20a84bec4..adc198d3a 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -29,12 +29,13 @@ class User(AbstractUser): Users achieve access to our system in a few different ways. These choices reflect those pathways. """ + GRANDFATHERED = "grandfathered", "Legacy user" VERIFIED_BY_STAFF = "verified_by_staff", "Verified by staff" REGULAR = "regular", "Verified by Login.gov" INVITED = "invited", "Invited by a domain manager" # We need a type for fixture users (rather than using verified by staff) - # because those users still do get "verified" through normal means + # because those users still do get "verified" through normal means # after they login. FIXTURE_USER = "fixture_user", "Created by fixtures" @@ -153,7 +154,7 @@ class User(AbstractUser): @classmethod def get_verification_type_from_email(cls, email, invitation_status=DomainInvitation.DomainInvitationStatus.INVITED): """Retrieves the verification type based off of a provided email address""" - + verification_type = None if TransitionDomain.objects.filter(username=email).exists(): # A new incoming user who is a domain manager for one of the domains @@ -164,12 +165,12 @@ class User(AbstractUser): # New users flagged by Staff to bypass ial2 verification_type = cls.VerificationTypeChoices.VERIFIED_BY_STAFF elif DomainInvitation.objects.filter(email=email, status=invitation_status).exists(): - # A new incoming user who is being invited to be a domain manager (that is, + # A new incoming user who is being invited to be a domain manager (that is, # their email address is in DomainInvitation for an invitation that is not yet "retrieved"). verification_type = cls.VerificationTypeChoices.INVITED else: verification_type = cls.VerificationTypeChoices.REGULAR - + return verification_type def check_domain_invitations_on_login(self): diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index bca1a94cb..20ada4d14 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2820,7 +2820,7 @@ class MyUserAdminTest(TestCase): request.user = create_user() fieldsets = self.admin.get_fieldsets(request) expected_fieldsets = ( - (None, {"fields": ("password", "status")}), + (None, {"fields": ("password", "status", "verification_type")}), ("Personal Info", {"fields": ("first_name", "last_name", "email")}), ("Permissions", {"fields": ("is_active", "groups")}), ("Important dates", {"fields": ("last_login", "date_joined")}), From 2f494466fc5d2f5b1e2320d043fa08968aae9c56 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 22 Apr 2024 13:09:16 -0600 Subject: [PATCH 08/75] Check created at date for invited --- src/registrar/models/user.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index adc198d3a..f775c77ad 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -28,6 +28,13 @@ class User(AbstractUser): """ Users achieve access to our system in a few different ways. These choices reflect those pathways. + + Overview of verification types: + - GRANDFATHERED: User exists in the `TransitionDomain` table + - VERIFIED_BY_STAFF: User exists in the `VerifiedByStaff` table + - INVITED: User exists in the `DomainInvitation` table + - REGULAR: User was verified through IAL2 + - FIXTURE_USER: User was created by fixtures """ GRANDFATHERED = "grandfathered", "Legacy user" @@ -146,9 +153,23 @@ class User(AbstractUser): return needs_verification def set_user_verification_type(self): - # Would need to check audit log + """ + Given pre-existing data from TransitionDomain, VerifiedByStaff, and DomainInvitation, + set the verification "type" defined in VerificationTypeChoices. + """ retrieved = DomainInvitation.DomainInvitationStatus.RETRIEVED verification_type = self.get_verification_type_from_email(self.email, invitation_status=retrieved) + + # An existing user may have been invited to a domain after they got verified. + # We need to check for this condition. + if verification_type == User.VerificationTypeChoices.INVITED: + invitation = DomainInvitation.objects.filter(email=self.email, status=retrieved).order_by("created_at").first() + + # If you joined BEFORE the oldest invitation was created, then you were verified normally. + # (See logic in get_verification_type_from_email) + if self.date_joined < invitation.created_at: + verification_type = User.VerificationTypeChoices.REGULAR + self.verification_type = verification_type @classmethod From c24a235d8ec6e2c3c23dfd66c807bf7669f279c5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 22 Apr 2024 13:51:22 -0600 Subject: [PATCH 09/75] Add script to populate data --- docs/operations/data_migration.md | 34 ++++++++++++-- .../commands/populate_verification_type.py | 46 +++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 src/registrar/management/commands/populate_verification_type.py diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 0846208de..e4543a28c 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -602,18 +602,18 @@ That data are synthesized from the generic_org_type field and the is_election_bo The latest domain_election_board csv can be found [here](https://drive.google.com/file/d/1aDeCqwHmBnXBl2arvoFCN0INoZmsEGsQ/view). After downloading this file, place it in `src/migrationdata` -#### Step 2: Upload the domain_election_board file to your sandbox +#### Step 3: Upload the domain_election_board file to your sandbox Follow [Step 1: Transfer data to sandboxes](#step-1-transfer-data-to-sandboxes) and [Step 2: Transfer uploaded files to the getgov directory](#step-2-transfer-uploaded-files-to-the-getgov-directory) from the [Set Up Migrations on Sandbox](#set-up-migrations-on-sandbox) portion of this doc. -#### Step 2: SSH into your environment +#### Step 4: SSH into your environment ```cf ssh getgov-{space}``` Example: `cf ssh getgov-za` -#### Step 3: Create a shell instance +#### Step 5: Create a shell instance ```/tmp/lifecycle/shell``` -#### Step 4: Running the script +#### Step 6: Running the script ```./manage.py populate_organization_type {domain_election_board_filename}``` - The domain_election_board_filename file must adhere to this format: @@ -642,3 +642,29 @@ Example (assuming that this is being ran from src/): | | Parameter | Description | |:-:|:------------------------------------|:-------------------------------------------------------------------| | 1 | **domain_election_board_filename** | A file containing every domain that is an election office. + + +## Populate Verification Type +This section outlines how to run the `populate_verification_type` script. +The script is used to update the verification_type field on User when it is None. + +### Running on sandboxes + +#### Step 1: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 2: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-za` + +#### Step 3: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 4: Running the script +```./manage.py populate_verification_type``` + +### Running locally + +#### Step 1: Running the script +```docker-compose exec app ./manage.py populate_verification_type``` diff --git a/src/registrar/management/commands/populate_verification_type.py b/src/registrar/management/commands/populate_verification_type.py new file mode 100644 index 000000000..959fefe6c --- /dev/null +++ b/src/registrar/management/commands/populate_verification_type.py @@ -0,0 +1,46 @@ +import argparse +import logging +from typing import List +from django.core.management import BaseCommand +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper, ScriptDataHelper +from registrar.models import User + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Loops through each valid User object and updates its verification_type value" + + def handle(self, **kwargs): + """Loops through each valid User object and updates its verification_type value""" + + users = User.objects.filter(verification_type__isnull=True) + + # Code execution will stop here if the user prompts "N" + TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + info_to_inspect=f""" + ==Proposed Changes== + Number of User objects to change: {len(users)} + This field will be updated on each record: verification_type + """, + prompt_title="Do you wish to patch verification_type data?", + ) + logger.info("Updating...") + + user_to_update: List[User] = [] + user_failed_to_update: List[User] = [] + for user in users: + try: + user.set_user_verification_type() + user_to_update.append(user) + except Exception as err: + user_failed_to_update.append(user) + logger.error(err) + logger.error(f"{TerminalColors.FAIL}" f"Failed to update {user}" f"{TerminalColors.ENDC}") + + # Do a bulk update on the first_ready field + ScriptDataHelper.bulk_update_fields(User, user_to_update, ["verification_type"]) + + # Log what happened + TerminalHelper.log_script_run_summary(user_to_update, user_failed_to_update, skipped=[], debug=True) From 5aa4e8f484295f1270613bdd7a0478eeff27fb11 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 22 Apr 2024 14:32:42 -0600 Subject: [PATCH 10/75] Add script template --- .../commands/populate_verification_type.py | 46 ++++------------- .../commands/utility/terminal_helper.py | 51 +++++++++++++++++++ 2 files changed, 62 insertions(+), 35 deletions(-) diff --git a/src/registrar/management/commands/populate_verification_type.py b/src/registrar/management/commands/populate_verification_type.py index 959fefe6c..ceaaccc2e 100644 --- a/src/registrar/management/commands/populate_verification_type.py +++ b/src/registrar/management/commands/populate_verification_type.py @@ -1,46 +1,22 @@ -import argparse import logging from typing import List from django.core.management import BaseCommand -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper, ScriptDataHelper +from registrar.management.commands.utility.terminal_helper import ScriptTemplate, TerminalColors from registrar.models import User logger = logging.getLogger(__name__) -class Command(BaseCommand): +class Command(ScriptTemplate): help = "Loops through each valid User object and updates its verification_type value" - def handle(self, **kwargs): + def handle(self): """Loops through each valid User object and updates its verification_type value""" - - users = User.objects.filter(verification_type__isnull=True) - - # Code execution will stop here if the user prompts "N" - TerminalHelper.prompt_for_execution( - system_exit_on_terminate=True, - info_to_inspect=f""" - ==Proposed Changes== - Number of User objects to change: {len(users)} - This field will be updated on each record: verification_type - """, - prompt_title="Do you wish to patch verification_type data?", - ) - logger.info("Updating...") - - user_to_update: List[User] = [] - user_failed_to_update: List[User] = [] - for user in users: - try: - user.set_user_verification_type() - user_to_update.append(user) - except Exception as err: - user_failed_to_update.append(user) - logger.error(err) - logger.error(f"{TerminalColors.FAIL}" f"Failed to update {user}" f"{TerminalColors.ENDC}") - - # Do a bulk update on the first_ready field - ScriptDataHelper.bulk_update_fields(User, user_to_update, ["verification_type"]) - - # Log what happened - TerminalHelper.log_script_run_summary(user_to_update, user_failed_to_update, skipped=[], debug=True) + filter_condition = { + "verification_type__isnull": True + } + ScriptTemplate.mass_populate_field(User, filter_condition, ["verification_type"]) + + def populate_field(self, field_to_update): + """Defines how we update the verification_type field""" + field_to_update.set_user_verification_type() diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index b54209750..f42ebc1cb 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -2,6 +2,7 @@ import logging import sys from django.core.paginator import Paginator from typing import List +from django.core.management import BaseCommand from registrar.utility.enums import LogCode logger = logging.getLogger(__name__) @@ -58,6 +59,56 @@ class ScriptDataHelper: model_class.objects.bulk_update(page.object_list, fields_to_update) +class ScriptTemplate(BaseCommand): + """ + Contains common script actions for our scripts which can be prefilled as templates. + """ + + @staticmethod + def mass_populate_field(sender, filter_conditions, fields_to_update): + """Loops through each valid "sender" object - specified by filter_conditions - and + updates fields defined by fields_to_update using populate_function. + + You must define populate_field before you can use this function. + """ + + objects = sender.objects.filter(**filter_conditions) + + # Code execution will stop here if the user prompts "N" + TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + info_to_inspect=f""" + ==Proposed Changes== + Number of {sender} objects to change: {len(objects)} + These fields will be updated on each record: {fields_to_update} + """, + prompt_title="Do you wish to patch this data?", + ) + logger.info("Updating...") + + to_update: List[sender] = [] + failed_to_update: List[sender] = [] + for updated_object in objects: + try: + ScriptTemplate.populate_field(updated_object) + to_update.append(updated_object) + except Exception as err: + to_update.append(updated_object) + logger.error(err) + logger.error(f"{TerminalColors.FAIL}" f"Failed to update {updated_object}" f"{TerminalColors.ENDC}") + + # Do a bulk update on the first_ready field + ScriptDataHelper.bulk_update_fields(sender, to_update, fields_to_update) + + # Log what happened + TerminalHelper.log_script_run_summary(to_update, failed_to_update, skipped=[], debug=True) + + @staticmethod + def populate_field(field_to_update): + """Defines how we update each field. Must be defined before using mass_populate_field.""" + raise NotImplementedError("This method should be implemented by the child class.") + + class TerminalHelper: @staticmethod def log_script_run_summary(to_update, failed_to_update, skipped, debug: bool, log_header=None): From 705febe37a7b254952c9a7cce6c68e221bbfdc2d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 22 Apr 2024 15:25:53 -0600 Subject: [PATCH 11/75] Add kwargs --- src/registrar/management/commands/populate_verification_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/populate_verification_type.py b/src/registrar/management/commands/populate_verification_type.py index ceaaccc2e..a59f16570 100644 --- a/src/registrar/management/commands/populate_verification_type.py +++ b/src/registrar/management/commands/populate_verification_type.py @@ -10,7 +10,7 @@ logger = logging.getLogger(__name__) class Command(ScriptTemplate): help = "Loops through each valid User object and updates its verification_type value" - def handle(self): + def handle(self, **kwargs): """Loops through each valid User object and updates its verification_type value""" filter_condition = { "verification_type__isnull": True From 9b3466475f0fe7f2dbe0008a860acd648d13bd32 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 22 Apr 2024 15:34:03 -0600 Subject: [PATCH 12/75] Change override --- .../management/commands/populate_verification_type.py | 2 +- .../management/commands/utility/terminal_helper.py | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/registrar/management/commands/populate_verification_type.py b/src/registrar/management/commands/populate_verification_type.py index a59f16570..63ef641d8 100644 --- a/src/registrar/management/commands/populate_verification_type.py +++ b/src/registrar/management/commands/populate_verification_type.py @@ -15,7 +15,7 @@ class Command(ScriptTemplate): filter_condition = { "verification_type__isnull": True } - ScriptTemplate.mass_populate_field(User, filter_condition, ["verification_type"]) + self.mass_populate_field(User, filter_condition, ["verification_type"]) def populate_field(self, field_to_update): """Defines how we update the verification_type field""" diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index f42ebc1cb..305be8d8d 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -64,8 +64,7 @@ class ScriptTemplate(BaseCommand): Contains common script actions for our scripts which can be prefilled as templates. """ - @staticmethod - def mass_populate_field(sender, filter_conditions, fields_to_update): + def mass_populate_field(self, sender, filter_conditions, fields_to_update): """Loops through each valid "sender" object - specified by filter_conditions - and updates fields defined by fields_to_update using populate_function. @@ -90,7 +89,7 @@ class ScriptTemplate(BaseCommand): failed_to_update: List[sender] = [] for updated_object in objects: try: - ScriptTemplate.populate_field(updated_object) + self.populate_field(updated_object) to_update.append(updated_object) except Exception as err: to_update.append(updated_object) @@ -102,9 +101,8 @@ class ScriptTemplate(BaseCommand): # Log what happened TerminalHelper.log_script_run_summary(to_update, failed_to_update, skipped=[], debug=True) - - @staticmethod - def populate_field(field_to_update): + + def populate_field(self, field_to_update): """Defines how we update each field. Must be defined before using mass_populate_field.""" raise NotImplementedError("This method should be implemented by the child class.") From 8f99c2eafa73a4412bd75bdbebef5361cf608e57 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 22 Apr 2024 18:39:09 -0400 Subject: [PATCH 13/75] Add a link on empty domains tables --- src/registrar/assets/sass/_theme/_links.scss | 19 + src/registrar/assets/sass/_theme/_tables.scss | 16 - src/registrar/assets/sass/_theme/styles.scss | 1 + src/registrar/templates/home.html | 8 + src/registrar/tests/test_views.py | 372 +++++++++++++++++- src/registrar/tests/test_views_request.py | 363 ----------------- 6 files changed, 399 insertions(+), 380 deletions(-) create mode 100644 src/registrar/assets/sass/_theme/_links.scss diff --git a/src/registrar/assets/sass/_theme/_links.scss b/src/registrar/assets/sass/_theme/_links.scss new file mode 100644 index 000000000..aa5a85f01 --- /dev/null +++ b/src/registrar/assets/sass/_theme/_links.scss @@ -0,0 +1,19 @@ +@use "uswds-core" as *; + +p, .dotgov-table { + a { + display: flex; + align-items: flex-start; + color: color('primary'); + + &:visited { + color: color('primary'); + } + + .usa-icon { + // align icon with x height + margin-top: units(0.5); + margin-right: units(0.5); + } + } +} diff --git a/src/registrar/assets/sass/_theme/_tables.scss b/src/registrar/assets/sass/_theme/_tables.scss index 0d58b5878..8d4ebbc59 100644 --- a/src/registrar/assets/sass/_theme/_tables.scss +++ b/src/registrar/assets/sass/_theme/_tables.scss @@ -56,22 +56,6 @@ .dotgov-table { width: 100%; - a { - display: flex; - align-items: flex-start; - color: color('primary'); - - &:visited { - color: color('primary'); - } - - .usa-icon { - // align icon with x height - margin-top: units(0.5); - margin-right: units(0.5); - } - } - th[data-sortable]:not([aria-sort]) .usa-table__header__button { right: auto; } diff --git a/src/registrar/assets/sass/_theme/styles.scss b/src/registrar/assets/sass/_theme/styles.scss index 942501110..64b113a29 100644 --- a/src/registrar/assets/sass/_theme/styles.scss +++ b/src/registrar/assets/sass/_theme/styles.scss @@ -10,6 +10,7 @@ --- Custom Styles ---------------------------------*/ @forward "base"; @forward "typography"; +@forward "links"; @forward "lists"; @forward "buttons"; @forward "forms"; diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index ea9276b9f..6254fc3bb 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -98,6 +98,14 @@ >
{% else %}

You don't have any registered domains.

+

+ + + Why don't I see my domain when I sign in to the registrar? + +

{% endif %} diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index b8055f288..217e24b9a 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1,12 +1,16 @@ +from datetime import date from django.test import Client, TestCase, override_settings from django.contrib.auth import get_user_model from api.tests.common import less_console_noise_decorator +from registrar.models.contact import Contact from registrar.models.domain import Domain +from registrar.models.draft_domain import DraftDomain +from registrar.models.user import User from registrar.models.user_domain_role import UserDomainRole from registrar.views.domain import DomainNameserversView -from .common import MockEppLib # type: ignore +from .common import MockEppLib, less_console_noise # type: ignore from unittest.mock import patch from django.urls import reverse @@ -135,3 +139,369 @@ class TestEnvironmentVariablesEffects(TestCase): self.assertEqual(contact_page_500.status_code, 500) self.assertNotContains(contact_page_500, "You are on a test site.") + + +class HomeTests(TestWithUser): + """A series of tests that target the two tables on home.html""" + + def setUp(self): + super().setUp() + self.client.force_login(self.user) + + def tearDown(self): + super().tearDown() + Contact.objects.all().delete() + + def test_empty_domain_table(self): + response = self.client.get("/") + self.assertContains(response, "You don't have any registered domains.") + self.assertContains(response, "Why don't I see my domain when I sign in to the registrar?") + + def test_home_lists_domain_requests(self): + response = self.client.get("/") + self.assertNotContains(response, "igorville.gov") + site = DraftDomain.objects.create(name="igorville.gov") + domain_request = DomainRequest.objects.create(creator=self.user, requested_domain=site) + response = self.client.get("/") + + # count = 7 because of screenreader content + self.assertContains(response, "igorville.gov", count=7) + + # clean up + domain_request.delete() + + def test_state_help_text(self): + """Tests if each domain state has help text""" + + # Get the expected text content of each state + deleted_text = "This domain has been removed and " "is no longer registered to your organization." + dns_needed_text = "Before this domain can be used, " "you’ll need to add name server addresses." + ready_text = "This domain has name servers and is ready for use." + on_hold_text = ( + "This domain is administratively paused, " + "so it can’t be edited and won’t resolve in DNS. " + "Contact help@get.gov for details." + ) + deleted_text = "This domain has been removed and " "is no longer registered to your organization." + # Generate a mapping of domain names, the state, and expected messages for the subtest + test_cases = [ + ("deleted.gov", Domain.State.DELETED, deleted_text), + ("dnsneeded.gov", Domain.State.DNS_NEEDED, dns_needed_text), + ("unknown.gov", Domain.State.UNKNOWN, dns_needed_text), + ("onhold.gov", Domain.State.ON_HOLD, on_hold_text), + ("ready.gov", Domain.State.READY, ready_text), + ] + for domain_name, state, expected_message in test_cases: + with self.subTest(domain_name=domain_name, state=state, expected_message=expected_message): + # Create a domain and a UserRole with the given params + test_domain, _ = Domain.objects.get_or_create(name=domain_name, state=state) + test_domain.expiration_date = date.today() + test_domain.save() + + user_role, _ = UserDomainRole.objects.get_or_create( + user=self.user, domain=test_domain, role=UserDomainRole.Roles.MANAGER + ) + + # Grab the home page + response = self.client.get("/") + + # Make sure the user can actually see the domain. + # We expect two instances because of SR content. + self.assertContains(response, domain_name, count=2) + + # Check that we have the right text content. + self.assertContains(response, expected_message, count=1) + + # Delete the role and domain to ensure we're testing in isolation + user_role.delete() + test_domain.delete() + + def test_state_help_text_expired(self): + """Tests if each domain state has help text when expired""" + expired_text = "This domain has expired, but it is still online. " "To renew this domain, contact help@get.gov." + test_domain, _ = Domain.objects.get_or_create(name="expired.gov", state=Domain.State.READY) + test_domain.expiration_date = date(2011, 10, 10) + test_domain.save() + + UserDomainRole.objects.get_or_create(user=self.user, domain=test_domain, role=UserDomainRole.Roles.MANAGER) + + # Grab the home page + response = self.client.get("/") + + # Make sure the user can actually see the domain. + # We expect two instances because of SR content. + self.assertContains(response, "expired.gov", count=2) + + # Check that we have the right text content. + self.assertContains(response, expired_text, count=1) + + def test_state_help_text_no_expiration_date(self): + """Tests if each domain state has help text when expiration date is None""" + + # == Test a expiration of None for state ready. This should be expired. == # + expired_text = "This domain has expired, but it is still online. " "To renew this domain, contact help@get.gov." + test_domain, _ = Domain.objects.get_or_create(name="imexpired.gov", state=Domain.State.READY) + test_domain.expiration_date = None + test_domain.save() + + UserDomainRole.objects.get_or_create(user=self.user, domain=test_domain, role=UserDomainRole.Roles.MANAGER) + + # Grab the home page + response = self.client.get("/") + + # Make sure the user can actually see the domain. + # We expect two instances because of SR content. + self.assertContains(response, "imexpired.gov", count=2) + + # Make sure the expiration date is None + self.assertEqual(test_domain.expiration_date, None) + + # Check that we have the right text content. + self.assertContains(response, expired_text, count=1) + + # == Test a expiration of None for state unknown. This should not display expired text. == # + unknown_text = "Before this domain can be used, " "you’ll need to add name server addresses." + test_domain_2, _ = Domain.objects.get_or_create(name="notexpired.gov", state=Domain.State.UNKNOWN) + test_domain_2.expiration_date = None + test_domain_2.save() + + UserDomainRole.objects.get_or_create(user=self.user, domain=test_domain_2, role=UserDomainRole.Roles.MANAGER) + + # Grab the home page + response = self.client.get("/") + + # Make sure the user can actually see the domain. + # We expect two instances because of SR content. + self.assertContains(response, "notexpired.gov", count=2) + + # Make sure the expiration date is None + self.assertEqual(test_domain_2.expiration_date, None) + + # Check that we have the right text content. + self.assertContains(response, unknown_text, count=1) + + def test_home_deletes_withdrawn_domain_request(self): + """Tests if the user can delete a DomainRequest in the 'withdrawn' status""" + + site = DraftDomain.objects.create(name="igorville.gov") + domain_request = DomainRequest.objects.create( + creator=self.user, requested_domain=site, status=DomainRequest.DomainRequestStatus.WITHDRAWN + ) + + # Ensure that igorville.gov exists on the page + home_page = self.client.get("/") + self.assertContains(home_page, "igorville.gov") + + # Check if the delete button exists. We can do this by checking for its id and text content. + self.assertContains(home_page, "Delete") + self.assertContains(home_page, "button-toggle-delete-domain-alert-1") + + # Trigger the delete logic + response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True) + + self.assertNotContains(response, "igorville.gov") + + # clean up + domain_request.delete() + + def test_home_deletes_started_domain_request(self): + """Tests if the user can delete a DomainRequest in the 'started' status""" + + site = DraftDomain.objects.create(name="igorville.gov") + domain_request = DomainRequest.objects.create( + creator=self.user, requested_domain=site, status=DomainRequest.DomainRequestStatus.STARTED + ) + + # Ensure that igorville.gov exists on the page + home_page = self.client.get("/") + self.assertContains(home_page, "igorville.gov") + + # Check if the delete button exists. We can do this by checking for its id and text content. + self.assertContains(home_page, "Delete") + self.assertContains(home_page, "button-toggle-delete-domain-alert-1") + + # Trigger the delete logic + response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True) + + self.assertNotContains(response, "igorville.gov") + + # clean up + domain_request.delete() + + def test_home_doesnt_delete_other_domain_requests(self): + """Tests to ensure the user can't delete domain requests not in the status of STARTED or WITHDRAWN""" + + # Given that we are including a subset of items that can be deleted while excluding the rest, + # subTest is appropriate here as otherwise we would need many duplicate tests for the same reason. + with less_console_noise(): + draft_domain = DraftDomain.objects.create(name="igorville.gov") + for status in DomainRequest.DomainRequestStatus: + if status not in [ + DomainRequest.DomainRequestStatus.STARTED, + DomainRequest.DomainRequestStatus.WITHDRAWN, + ]: + with self.subTest(status=status): + domain_request = DomainRequest.objects.create( + creator=self.user, requested_domain=draft_domain, status=status + ) + + # Trigger the delete logic + response = self.client.post( + reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True + ) + + # Check for a 403 error - the end user should not be allowed to do this + self.assertEqual(response.status_code, 403) + + desired_domain_request = DomainRequest.objects.filter(requested_domain=draft_domain) + + # Make sure the DomainRequest wasn't deleted + self.assertEqual(desired_domain_request.count(), 1) + + # clean up + domain_request.delete() + + def test_home_deletes_domain_request_and_orphans(self): + """Tests if delete for DomainRequest deletes orphaned Contact objects""" + + # Create the site and contacts to delete (orphaned) + contact = Contact.objects.create( + first_name="Henry", + last_name="Mcfakerson", + ) + contact_shared = Contact.objects.create( + first_name="Relative", + last_name="Aether", + ) + + # Create two non-orphaned contacts + contact_2 = Contact.objects.create( + first_name="Saturn", + last_name="Mars", + ) + + # Attach a user object to a contact (should not be deleted) + contact_user, _ = Contact.objects.get_or_create(user=self.user) + + site = DraftDomain.objects.create(name="igorville.gov") + domain_request = DomainRequest.objects.create( + creator=self.user, + requested_domain=site, + status=DomainRequest.DomainRequestStatus.WITHDRAWN, + authorizing_official=contact, + submitter=contact_user, + ) + domain_request.other_contacts.set([contact_2]) + + # Create a second domain request to attach contacts to + site_2 = DraftDomain.objects.create(name="teaville.gov") + domain_request_2 = DomainRequest.objects.create( + creator=self.user, + requested_domain=site_2, + status=DomainRequest.DomainRequestStatus.STARTED, + authorizing_official=contact_2, + submitter=contact_shared, + ) + domain_request_2.other_contacts.set([contact_shared]) + + # Ensure that igorville.gov exists on the page + home_page = self.client.get("/") + self.assertContains(home_page, "igorville.gov") + + # Trigger the delete logic + response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True) + + # igorville is now deleted + self.assertNotContains(response, "igorville.gov") + + # Check if the orphaned contact was deleted + orphan = Contact.objects.filter(id=contact.id) + self.assertFalse(orphan.exists()) + + # All non-orphan contacts should still exist and are unaltered + try: + current_user = Contact.objects.filter(id=contact_user.id).get() + except Contact.DoesNotExist: + self.fail("contact_user (a non-orphaned contact) was deleted") + + self.assertEqual(current_user, contact_user) + try: + edge_case = Contact.objects.filter(id=contact_2.id).get() + except Contact.DoesNotExist: + self.fail("contact_2 (a non-orphaned contact) was deleted") + + self.assertEqual(edge_case, contact_2) + + def test_home_deletes_domain_request_and_shared_orphans(self): + """Test the edge case for an object that will become orphaned after a delete + (but is not an orphan at the time of deletion)""" + + # Create the site and contacts to delete (orphaned) + contact = Contact.objects.create( + first_name="Henry", + last_name="Mcfakerson", + ) + contact_shared = Contact.objects.create( + first_name="Relative", + last_name="Aether", + ) + + # Create two non-orphaned contacts + contact_2 = Contact.objects.create( + first_name="Saturn", + last_name="Mars", + ) + + # Attach a user object to a contact (should not be deleted) + contact_user, _ = Contact.objects.get_or_create(user=self.user) + + site = DraftDomain.objects.create(name="igorville.gov") + domain_request = DomainRequest.objects.create( + creator=self.user, + requested_domain=site, + status=DomainRequest.DomainRequestStatus.WITHDRAWN, + authorizing_official=contact, + submitter=contact_user, + ) + domain_request.other_contacts.set([contact_2]) + + # Create a second domain request to attach contacts to + site_2 = DraftDomain.objects.create(name="teaville.gov") + domain_request_2 = DomainRequest.objects.create( + creator=self.user, + requested_domain=site_2, + status=DomainRequest.DomainRequestStatus.STARTED, + authorizing_official=contact_2, + submitter=contact_shared, + ) + domain_request_2.other_contacts.set([contact_shared]) + + home_page = self.client.get("/") + self.assertContains(home_page, "teaville.gov") + + # Trigger the delete logic + response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request_2.pk}), follow=True) + + self.assertNotContains(response, "teaville.gov") + + # Check if the orphaned contact was deleted + orphan = Contact.objects.filter(id=contact_shared.id) + self.assertFalse(orphan.exists()) + + def test_domain_request_form_view(self): + response = self.client.get("/request/", follow=True) + self.assertContains( + response, + "You’re about to start your .gov domain request.", + ) + + def test_domain_request_form_with_ineligible_user(self): + """Domain request form not accessible for an ineligible user. + This test should be solid enough since all domain request wizard + views share the same permissions class""" + self.user.status = User.RESTRICTED + self.user.save() + + with less_console_noise(): + response = self.client.get("/request/", follow=True) + self.assertEqual(response.status_code, 403) diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index a4cb210bc..51e1b753f 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -3,7 +3,6 @@ from unittest.mock import Mock from django.conf import settings from django.urls import reverse -from datetime import date from .common import MockSESClient, completed_domain_request # type: ignore from django_webtest import WebTest # type: ignore @@ -17,7 +16,6 @@ from registrar.models import ( Contact, User, Website, - UserDomainRole, ) from registrar.views.domain_request import DomainRequestWizard, Step @@ -2328,364 +2326,3 @@ class TestWizardUnlockingSteps(TestWithUser, WebTest): else: self.fail(f"Expected a redirect, but got a different response: {response}") - - -class HomeTests(TestWithUser): - """A series of tests that target the two tables on home.html""" - - def setUp(self): - super().setUp() - self.client.force_login(self.user) - - def tearDown(self): - super().tearDown() - Contact.objects.all().delete() - - def test_home_lists_domain_requests(self): - response = self.client.get("/") - self.assertNotContains(response, "igorville.gov") - site = DraftDomain.objects.create(name="igorville.gov") - domain_request = DomainRequest.objects.create(creator=self.user, requested_domain=site) - response = self.client.get("/") - - # count = 7 because of screenreader content - self.assertContains(response, "igorville.gov", count=7) - - # clean up - domain_request.delete() - - def test_state_help_text(self): - """Tests if each domain state has help text""" - - # Get the expected text content of each state - deleted_text = "This domain has been removed and " "is no longer registered to your organization." - dns_needed_text = "Before this domain can be used, " "you’ll need to add name server addresses." - ready_text = "This domain has name servers and is ready for use." - on_hold_text = ( - "This domain is administratively paused, " - "so it can’t be edited and won’t resolve in DNS. " - "Contact help@get.gov for details." - ) - deleted_text = "This domain has been removed and " "is no longer registered to your organization." - # Generate a mapping of domain names, the state, and expected messages for the subtest - test_cases = [ - ("deleted.gov", Domain.State.DELETED, deleted_text), - ("dnsneeded.gov", Domain.State.DNS_NEEDED, dns_needed_text), - ("unknown.gov", Domain.State.UNKNOWN, dns_needed_text), - ("onhold.gov", Domain.State.ON_HOLD, on_hold_text), - ("ready.gov", Domain.State.READY, ready_text), - ] - for domain_name, state, expected_message in test_cases: - with self.subTest(domain_name=domain_name, state=state, expected_message=expected_message): - # Create a domain and a UserRole with the given params - test_domain, _ = Domain.objects.get_or_create(name=domain_name, state=state) - test_domain.expiration_date = date.today() - test_domain.save() - - user_role, _ = UserDomainRole.objects.get_or_create( - user=self.user, domain=test_domain, role=UserDomainRole.Roles.MANAGER - ) - - # Grab the home page - response = self.client.get("/") - - # Make sure the user can actually see the domain. - # We expect two instances because of SR content. - self.assertContains(response, domain_name, count=2) - - # Check that we have the right text content. - self.assertContains(response, expected_message, count=1) - - # Delete the role and domain to ensure we're testing in isolation - user_role.delete() - test_domain.delete() - - def test_state_help_text_expired(self): - """Tests if each domain state has help text when expired""" - expired_text = "This domain has expired, but it is still online. " "To renew this domain, contact help@get.gov." - test_domain, _ = Domain.objects.get_or_create(name="expired.gov", state=Domain.State.READY) - test_domain.expiration_date = date(2011, 10, 10) - test_domain.save() - - UserDomainRole.objects.get_or_create(user=self.user, domain=test_domain, role=UserDomainRole.Roles.MANAGER) - - # Grab the home page - response = self.client.get("/") - - # Make sure the user can actually see the domain. - # We expect two instances because of SR content. - self.assertContains(response, "expired.gov", count=2) - - # Check that we have the right text content. - self.assertContains(response, expired_text, count=1) - - def test_state_help_text_no_expiration_date(self): - """Tests if each domain state has help text when expiration date is None""" - - # == Test a expiration of None for state ready. This should be expired. == # - expired_text = "This domain has expired, but it is still online. " "To renew this domain, contact help@get.gov." - test_domain, _ = Domain.objects.get_or_create(name="imexpired.gov", state=Domain.State.READY) - test_domain.expiration_date = None - test_domain.save() - - UserDomainRole.objects.get_or_create(user=self.user, domain=test_domain, role=UserDomainRole.Roles.MANAGER) - - # Grab the home page - response = self.client.get("/") - - # Make sure the user can actually see the domain. - # We expect two instances because of SR content. - self.assertContains(response, "imexpired.gov", count=2) - - # Make sure the expiration date is None - self.assertEqual(test_domain.expiration_date, None) - - # Check that we have the right text content. - self.assertContains(response, expired_text, count=1) - - # == Test a expiration of None for state unknown. This should not display expired text. == # - unknown_text = "Before this domain can be used, " "you’ll need to add name server addresses." - test_domain_2, _ = Domain.objects.get_or_create(name="notexpired.gov", state=Domain.State.UNKNOWN) - test_domain_2.expiration_date = None - test_domain_2.save() - - UserDomainRole.objects.get_or_create(user=self.user, domain=test_domain_2, role=UserDomainRole.Roles.MANAGER) - - # Grab the home page - response = self.client.get("/") - - # Make sure the user can actually see the domain. - # We expect two instances because of SR content. - self.assertContains(response, "notexpired.gov", count=2) - - # Make sure the expiration date is None - self.assertEqual(test_domain_2.expiration_date, None) - - # Check that we have the right text content. - self.assertContains(response, unknown_text, count=1) - - def test_home_deletes_withdrawn_domain_request(self): - """Tests if the user can delete a DomainRequest in the 'withdrawn' status""" - - site = DraftDomain.objects.create(name="igorville.gov") - domain_request = DomainRequest.objects.create( - creator=self.user, requested_domain=site, status=DomainRequest.DomainRequestStatus.WITHDRAWN - ) - - # Ensure that igorville.gov exists on the page - home_page = self.client.get("/") - self.assertContains(home_page, "igorville.gov") - - # Check if the delete button exists. We can do this by checking for its id and text content. - self.assertContains(home_page, "Delete") - self.assertContains(home_page, "button-toggle-delete-domain-alert-1") - - # Trigger the delete logic - response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True) - - self.assertNotContains(response, "igorville.gov") - - # clean up - domain_request.delete() - - def test_home_deletes_started_domain_request(self): - """Tests if the user can delete a DomainRequest in the 'started' status""" - - site = DraftDomain.objects.create(name="igorville.gov") - domain_request = DomainRequest.objects.create( - creator=self.user, requested_domain=site, status=DomainRequest.DomainRequestStatus.STARTED - ) - - # Ensure that igorville.gov exists on the page - home_page = self.client.get("/") - self.assertContains(home_page, "igorville.gov") - - # Check if the delete button exists. We can do this by checking for its id and text content. - self.assertContains(home_page, "Delete") - self.assertContains(home_page, "button-toggle-delete-domain-alert-1") - - # Trigger the delete logic - response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True) - - self.assertNotContains(response, "igorville.gov") - - # clean up - domain_request.delete() - - def test_home_doesnt_delete_other_domain_requests(self): - """Tests to ensure the user can't delete domain requests not in the status of STARTED or WITHDRAWN""" - - # Given that we are including a subset of items that can be deleted while excluding the rest, - # subTest is appropriate here as otherwise we would need many duplicate tests for the same reason. - with less_console_noise(): - draft_domain = DraftDomain.objects.create(name="igorville.gov") - for status in DomainRequest.DomainRequestStatus: - if status not in [ - DomainRequest.DomainRequestStatus.STARTED, - DomainRequest.DomainRequestStatus.WITHDRAWN, - ]: - with self.subTest(status=status): - domain_request = DomainRequest.objects.create( - creator=self.user, requested_domain=draft_domain, status=status - ) - - # Trigger the delete logic - response = self.client.post( - reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True - ) - - # Check for a 403 error - the end user should not be allowed to do this - self.assertEqual(response.status_code, 403) - - desired_domain_request = DomainRequest.objects.filter(requested_domain=draft_domain) - - # Make sure the DomainRequest wasn't deleted - self.assertEqual(desired_domain_request.count(), 1) - - # clean up - domain_request.delete() - - def test_home_deletes_domain_request_and_orphans(self): - """Tests if delete for DomainRequest deletes orphaned Contact objects""" - - # Create the site and contacts to delete (orphaned) - contact = Contact.objects.create( - first_name="Henry", - last_name="Mcfakerson", - ) - contact_shared = Contact.objects.create( - first_name="Relative", - last_name="Aether", - ) - - # Create two non-orphaned contacts - contact_2 = Contact.objects.create( - first_name="Saturn", - last_name="Mars", - ) - - # Attach a user object to a contact (should not be deleted) - contact_user, _ = Contact.objects.get_or_create(user=self.user) - - site = DraftDomain.objects.create(name="igorville.gov") - domain_request = DomainRequest.objects.create( - creator=self.user, - requested_domain=site, - status=DomainRequest.DomainRequestStatus.WITHDRAWN, - authorizing_official=contact, - submitter=contact_user, - ) - domain_request.other_contacts.set([contact_2]) - - # Create a second domain request to attach contacts to - site_2 = DraftDomain.objects.create(name="teaville.gov") - domain_request_2 = DomainRequest.objects.create( - creator=self.user, - requested_domain=site_2, - status=DomainRequest.DomainRequestStatus.STARTED, - authorizing_official=contact_2, - submitter=contact_shared, - ) - domain_request_2.other_contacts.set([contact_shared]) - - # Ensure that igorville.gov exists on the page - home_page = self.client.get("/") - self.assertContains(home_page, "igorville.gov") - - # Trigger the delete logic - response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True) - - # igorville is now deleted - self.assertNotContains(response, "igorville.gov") - - # Check if the orphaned contact was deleted - orphan = Contact.objects.filter(id=contact.id) - self.assertFalse(orphan.exists()) - - # All non-orphan contacts should still exist and are unaltered - try: - current_user = Contact.objects.filter(id=contact_user.id).get() - except Contact.DoesNotExist: - self.fail("contact_user (a non-orphaned contact) was deleted") - - self.assertEqual(current_user, contact_user) - try: - edge_case = Contact.objects.filter(id=contact_2.id).get() - except Contact.DoesNotExist: - self.fail("contact_2 (a non-orphaned contact) was deleted") - - self.assertEqual(edge_case, contact_2) - - def test_home_deletes_domain_request_and_shared_orphans(self): - """Test the edge case for an object that will become orphaned after a delete - (but is not an orphan at the time of deletion)""" - - # Create the site and contacts to delete (orphaned) - contact = Contact.objects.create( - first_name="Henry", - last_name="Mcfakerson", - ) - contact_shared = Contact.objects.create( - first_name="Relative", - last_name="Aether", - ) - - # Create two non-orphaned contacts - contact_2 = Contact.objects.create( - first_name="Saturn", - last_name="Mars", - ) - - # Attach a user object to a contact (should not be deleted) - contact_user, _ = Contact.objects.get_or_create(user=self.user) - - site = DraftDomain.objects.create(name="igorville.gov") - domain_request = DomainRequest.objects.create( - creator=self.user, - requested_domain=site, - status=DomainRequest.DomainRequestStatus.WITHDRAWN, - authorizing_official=contact, - submitter=contact_user, - ) - domain_request.other_contacts.set([contact_2]) - - # Create a second domain request to attach contacts to - site_2 = DraftDomain.objects.create(name="teaville.gov") - domain_request_2 = DomainRequest.objects.create( - creator=self.user, - requested_domain=site_2, - status=DomainRequest.DomainRequestStatus.STARTED, - authorizing_official=contact_2, - submitter=contact_shared, - ) - domain_request_2.other_contacts.set([contact_shared]) - - home_page = self.client.get("/") - self.assertContains(home_page, "teaville.gov") - - # Trigger the delete logic - response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request_2.pk}), follow=True) - - self.assertNotContains(response, "teaville.gov") - - # Check if the orphaned contact was deleted - orphan = Contact.objects.filter(id=contact_shared.id) - self.assertFalse(orphan.exists()) - - def test_domain_request_form_view(self): - response = self.client.get("/request/", follow=True) - self.assertContains( - response, - "You’re about to start your .gov domain request.", - ) - - def test_domain_request_form_with_ineligible_user(self): - """Domain request form not accessible for an ineligible user. - This test should be solid enough since all domain request wizard - views share the same permissions class""" - self.user.status = User.RESTRICTED - self.user.save() - - with less_console_noise(): - response = self.client.get("/request/", follow=True) - self.assertEqual(response.status_code, 403) From 3f6a9ec1ce08ca5e441a1919ecc651c11e5e6c91 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 23 Apr 2024 12:13:55 -0600 Subject: [PATCH 14/75] Fix unit test --- src/registrar/tests/test_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 20ada4d14..8e9701d59 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2820,7 +2820,7 @@ class MyUserAdminTest(TestCase): request.user = create_user() fieldsets = self.admin.get_fieldsets(request) expected_fieldsets = ( - (None, {"fields": ("password", "status", "verification_type")}), + (None, {"fields": ("status", "verification_type")}), ("Personal Info", {"fields": ("first_name", "last_name", "email")}), ("Permissions", {"fields": ("is_active", "groups")}), ("Important dates", {"fields": ("last_login", "date_joined")}), From 9ce10c370e48f07880b9d3822c26fdaf7b51560b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 23 Apr 2024 12:18:08 -0600 Subject: [PATCH 15/75] Revert "Fix unit test" This reverts commit 3f6a9ec1ce08ca5e441a1919ecc651c11e5e6c91. --- src/registrar/tests/test_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 8e9701d59..20ada4d14 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2820,7 +2820,7 @@ class MyUserAdminTest(TestCase): request.user = create_user() fieldsets = self.admin.get_fieldsets(request) expected_fieldsets = ( - (None, {"fields": ("status", "verification_type")}), + (None, {"fields": ("password", "status", "verification_type")}), ("Personal Info", {"fields": ("first_name", "last_name", "email")}), ("Permissions", {"fields": ("is_active", "groups")}), ("Important dates", {"fields": ("last_login", "date_joined")}), From 54b615d7f88cbce3efc18bed285954bc299395ed Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 23 Apr 2024 13:46:20 -0600 Subject: [PATCH 16/75] Unit tests --- src/djangooidc/tests/test_views.py | 132 +++++++++++++++++++++++++++++ src/registrar/models/user.py | 3 +- 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/src/djangooidc/tests/test_views.py b/src/djangooidc/tests/test_views.py index f10afcbaf..fc93db82e 100644 --- a/src/djangooidc/tests/test_views.py +++ b/src/djangooidc/tests/test_views.py @@ -4,8 +4,10 @@ from django.http import HttpResponse from django.test import Client, TestCase, RequestFactory from django.urls import reverse +from api.tests.common import less_console_noise_decorator from djangooidc.exceptions import StateMismatch, InternalError from ..views import login_callback +from registrar.models import User, Contact, VerifiedByStaff, DomainInvitation, TransitionDomain, Domain from .common import less_console_noise @@ -15,6 +17,14 @@ class ViewsTest(TestCase): def setUp(self): self.client = Client() self.factory = RequestFactory() + + def tearDown(self): + User.objects.all().delete() + Contact.objects.all().delete() + DomainInvitation.objects.all().delete() + VerifiedByStaff.objects.all().delete() + TransitionDomain.objects.all().delete() + Domain.objects.all().delete() def say_hi(*args): return HttpResponse("Hi") @@ -228,6 +238,128 @@ class ViewsTest(TestCase): # assert that redirect is to / when no 'next' is set self.assertEqual(response.status_code, 302) self.assertEqual(response.url, "/") + + @less_console_noise_decorator + def test_login_callback_sets_verification_type_regular(self, mock_client): + """Test that openid sets the verification type to regular on the returned user""" + # SETUP + session = self.client.session + session.save() + # MOCK + # mock that callback returns user_info; this is the expected behavior + mock_client.callback.side_effect = self.user_info + # patch that the request does not require step up auth + with patch("djangooidc.views._requires_step_up_auth", return_value=False), patch("djangooidc.views._initialize_client") as mock_init_client: + with patch("djangooidc.views._client_is_none", return_value=True): + # TEST + # test the login callback url + response = self.client.get(reverse("openid_login_callback")) + + # assert that _initialize_client was called + mock_init_client.assert_called_once() + + # Assert that we get a redirect + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/") + + # Test the created user object + created_user = User.objects.get(email="test@example.com") + self.assertEqual(created_user.verification_type, User.VerificationTypeChoices.REGULAR) + + @less_console_noise_decorator + def test_login_callback_sets_verification_type_invited(self, mock_client): + """Test that openid sets the verification type to invited on the returned user + when they exist in the DomainInvitation table""" + # SETUP + session = self.client.session + session.save() + + domain, _ = Domain.objects.get_or_create(name="test123.gov") + invitation, _ = DomainInvitation.objects.get_or_create(email="test@example.com", domain=domain) + # MOCK + # mock that callback returns user_info; this is the expected behavior + mock_client.callback.side_effect = self.user_info + # patch that the request does not require step up auth + with patch("djangooidc.views._requires_step_up_auth", return_value=False), patch("djangooidc.views._initialize_client") as mock_init_client: + with patch("djangooidc.views._client_is_none", return_value=True): + # TEST + # test the login callback url + response = self.client.get(reverse("openid_login_callback")) + + # assert that _initialize_client was called + mock_init_client.assert_called_once() + + # Assert that we get a redirect + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/") + + # Test the created user object + created_user = User.objects.get(email="test@example.com") + self.assertEqual(created_user.email, invitation.email) + self.assertEqual(created_user.verification_type, User.VerificationTypeChoices.INVITED) + + @less_console_noise_decorator + def test_login_callback_sets_verification_type_grandfathered(self, mock_client): + """Test that openid sets the verification type to grandfathered on a user which exists in our TransitionDomain table""" + # SETUP + session = self.client.session + session.save() + # MOCK + # mock that callback returns user_info; this is the expected behavior + mock_client.callback.side_effect = self.user_info + + td, _ = TransitionDomain.objects.get_or_create(username="test@example.com", domain_name="test123.gov") + + # patch that the request does not require step up auth + with patch("djangooidc.views._requires_step_up_auth", return_value=False), patch("djangooidc.views._initialize_client") as mock_init_client: + with patch("djangooidc.views._client_is_none", return_value=True): + # TEST + # test the login callback url + response = self.client.get(reverse("openid_login_callback")) + + # assert that _initialize_client was called + mock_init_client.assert_called_once() + + # Assert that we get a redirect + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/") + + # Test the created user object + created_user = User.objects.get(email="test@example.com") + self.assertEqual(created_user.email, td.username) + self.assertEqual(created_user.verification_type, User.VerificationTypeChoices.GRANDFATHERED) + + @less_console_noise_decorator + def test_login_callback_sets_verification_type_verified_by_staff(self, mock_client): + """Test that openid sets the verification type to verified_by_staff + on a user which exists in our VerifiedByStaff table""" + # SETUP + session = self.client.session + session.save() + # MOCK + # mock that callback returns user_info; this is the expected behavior + mock_client.callback.side_effect = self.user_info + + vip, _ = VerifiedByStaff.objects.get_or_create(email="test@example.com") + + # patch that the request does not require step up auth + with patch("djangooidc.views._requires_step_up_auth", return_value=False), patch("djangooidc.views._initialize_client") as mock_init_client: + with patch("djangooidc.views._client_is_none", return_value=True): + # TEST + # test the login callback url + response = self.client.get(reverse("openid_login_callback")) + + # assert that _initialize_client was called + mock_init_client.assert_called_once() + + # Assert that we get a redirect + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/") + + # Test the created user object + created_user = User.objects.get(email="test@example.com") + self.assertEqual(created_user.email, vip.email) + self.assertEqual(created_user.verification_type, User.VerificationTypeChoices.VERIFIED_BY_STAFF) def test_login_callback_no_step_up_auth(self, mock_client): """Walk through login_callback when _requires_step_up_auth returns False diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index f775c77ad..45532f8ea 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -3,6 +3,7 @@ import logging from django.contrib.auth.models import AbstractUser from django.db import models +from django.db.models import Q from registrar.models.user_domain_role import UserDomainRole @@ -177,7 +178,7 @@ class User(AbstractUser): """Retrieves the verification type based off of a provided email address""" verification_type = None - if TransitionDomain.objects.filter(username=email).exists(): + if TransitionDomain.objects.filter(Q(username=email) | Q(email=email)).exists(): # A new incoming user who is a domain manager for one of the domains # that we inputted from Verisign (that is, their email address appears # in the username field of a TransitionDomain) From d1fcb922c635b087d5c356e0f344845a0b3ab63a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 23 Apr 2024 14:17:08 -0600 Subject: [PATCH 17/75] Unit tests --- .../commands/populate_verification_type.py | 9 +- .../commands/utility/terminal_helper.py | 2 +- src/registrar/models/user.py | 9 +- .../tests/test_management_scripts.py | 91 ++++++++++++++++++- 4 files changed, 103 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/populate_verification_type.py b/src/registrar/management/commands/populate_verification_type.py index 63ef641d8..da493b8cf 100644 --- a/src/registrar/management/commands/populate_verification_type.py +++ b/src/registrar/management/commands/populate_verification_type.py @@ -12,11 +12,12 @@ class Command(ScriptTemplate): def handle(self, **kwargs): """Loops through each valid User object and updates its verification_type value""" - filter_condition = { - "verification_type__isnull": True - } + filter_condition = {"verification_type__isnull": True} self.mass_populate_field(User, filter_condition, ["verification_type"]) - + def populate_field(self, field_to_update): """Defines how we update the verification_type field""" field_to_update.set_user_verification_type() + logger.info( + f"{TerminalColors.OKCYAN}Updating {field_to_update} => {field_to_update.verification_type}{TerminalColors.OKCYAN}" + ) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 305be8d8d..1d411e403 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -65,7 +65,7 @@ class ScriptTemplate(BaseCommand): """ def mass_populate_field(self, sender, filter_conditions, fields_to_update): - """Loops through each valid "sender" object - specified by filter_conditions - and + """Loops through each valid "sender" object - specified by filter_conditions - and updates fields defined by fields_to_update using populate_function. You must define populate_field before you can use this function. diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 45532f8ea..eb21971f8 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -158,13 +158,18 @@ class User(AbstractUser): Given pre-existing data from TransitionDomain, VerifiedByStaff, and DomainInvitation, set the verification "type" defined in VerificationTypeChoices. """ + email_or_username = self.email or self.username retrieved = DomainInvitation.DomainInvitationStatus.RETRIEVED - verification_type = self.get_verification_type_from_email(self.email, invitation_status=retrieved) + verification_type = self.get_verification_type_from_email(email_or_username, invitation_status=retrieved) # An existing user may have been invited to a domain after they got verified. # We need to check for this condition. if verification_type == User.VerificationTypeChoices.INVITED: - invitation = DomainInvitation.objects.filter(email=self.email, status=retrieved).order_by("created_at").first() + invitation = ( + DomainInvitation.objects.filter(email=email_or_username, status=retrieved) + .order_by("created_at") + .first() + ) # If you joined BEFORE the oldest invitation was created, then you were verified normally. # (See logic in get_verification_type_from_email) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 26161b272..68b7f04c9 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -14,8 +14,9 @@ from registrar.models import ( TransitionDomain, DomainInformation, UserDomainRole, + VerifiedByStaff, + PublicContact, ) -from registrar.models.public_contact import PublicContact from django.core.management import call_command from unittest.mock import patch, call @@ -25,6 +26,94 @@ from .common import MockEppLib, less_console_noise, completed_domain_request from api.tests.common import less_console_noise_decorator +class TestPopulateVerificationType(MockEppLib): + """Tests for the populate_organization_type script""" + + def setUp(self): + """Creates a fake domain object""" + super().setUp() + + # Get the domain requests + self.domain_request_1 = completed_domain_request( + name="lasers.gov", + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + is_election_board=True, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + ) + + # Approve the request + self.domain_request_1.approve() + + # Get the domains + self.domain_1 = Domain.objects.get(name="lasers.gov") + + # Get users + self.regular_user, _ = User.objects.get_or_create(username="testuser@igormail.gov") + + vip, _ = VerifiedByStaff.objects.get_or_create(email="vipuser@igormail.gov") + self.verified_by_staff_user, _ = User.objects.get_or_create(username="vipuser@igormail.gov") + + grandfathered, _ = TransitionDomain.objects.get_or_create( + username="grandpa@igormail.gov", domain_name=self.domain_1.name + ) + self.grandfathered_user, _ = User.objects.get_or_create(username="grandpa@igormail.gov") + + invited, _ = DomainInvitation.objects.get_or_create(email="invited@igormail.gov", domain=self.domain_1) + self.invited_user, _ = User.objects.get_or_create(username="invited@igormail.gov") + + self.untouched_user, _ = User.objects.get_or_create( + username="iaminvincible@igormail.gov", verification_type=User.VerificationTypeChoices.GRANDFATHERED + ) + + def tearDown(self): + """Deletes all DB objects related to migrations""" + super().tearDown() + + # Delete domains and related information + Domain.objects.all().delete() + DomainInformation.objects.all().delete() + DomainRequest.objects.all().delete() + User.objects.all().delete() + Contact.objects.all().delete() + Website.objects.all().delete() + + @less_console_noise_decorator + def run_populate_verification_type(self): + """ + This method executes the populate_organization_type command. + + The 'call_command' function from Django's management framework is then used to + execute the populate_organization_type command with the specified arguments. + """ + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa + return_value=True, + ): + call_command("populate_verification_type") + + @less_console_noise_decorator + def test_verification_type_script_populates_data(self): + """Ensures that the verification type script actually populates data""" + + # Run the script + self.run_populate_verification_type() + + # Scripts don't work as we'd expect in our test environment, we need to manually + # trigger the refresh event + self.regular_user.refresh_from_db() + self.grandfathered_user.refresh_from_db() + self.invited_user.refresh_from_db() + self.verified_by_staff_user.refresh_from_db() + self.untouched_user.refresh_from_db() + + # Test all users + self.assertEqual(self.regular_user.verification_type, User.VerificationTypeChoices.REGULAR) + self.assertEqual(self.grandfathered_user.verification_type, User.VerificationTypeChoices.GRANDFATHERED) + self.assertEqual(self.invited_user.verification_type, User.VerificationTypeChoices.INVITED) + self.assertEqual(self.verified_by_staff_user.verification_type, User.VerificationTypeChoices.VERIFIED_BY_STAFF) + self.assertEqual(self.untouched_user.verification_type, User.VerificationTypeChoices.GRANDFATHERED) + + class TestPopulateOrganizationType(MockEppLib): """Tests for the populate_organization_type script""" From 883595ba446a3df7c431897f39afcea6b9acb614 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 23 Apr 2024 14:25:48 -0600 Subject: [PATCH 18/75] Add unit tests --- src/registrar/models/user.py | 4 ++-- src/registrar/tests/test_management_scripts.py | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index eb21971f8..3a37f5b61 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -158,7 +158,7 @@ class User(AbstractUser): Given pre-existing data from TransitionDomain, VerifiedByStaff, and DomainInvitation, set the verification "type" defined in VerificationTypeChoices. """ - email_or_username = self.email or self.username + email_or_username = self.email if self.email else self.username retrieved = DomainInvitation.DomainInvitationStatus.RETRIEVED verification_type = self.get_verification_type_from_email(email_or_username, invitation_status=retrieved) @@ -173,7 +173,7 @@ class User(AbstractUser): # If you joined BEFORE the oldest invitation was created, then you were verified normally. # (See logic in get_verification_type_from_email) - if self.date_joined < invitation.created_at: + if invitation is not None and self.date_joined < invitation.created_at: verification_type = User.VerificationTypeChoices.REGULAR self.verification_type = verification_type diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 68b7f04c9..617e305a1 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -58,13 +58,21 @@ class TestPopulateVerificationType(MockEppLib): ) self.grandfathered_user, _ = User.objects.get_or_create(username="grandpa@igormail.gov") - invited, _ = DomainInvitation.objects.get_or_create(email="invited@igormail.gov", domain=self.domain_1) + invited, _ = DomainInvitation.objects.get_or_create( + email="invited@igormail.gov", domain=self.domain_1, status=DomainInvitation.DomainInvitationStatus.RETRIEVED + ) self.invited_user, _ = User.objects.get_or_create(username="invited@igormail.gov") self.untouched_user, _ = User.objects.get_or_create( username="iaminvincible@igormail.gov", verification_type=User.VerificationTypeChoices.GRANDFATHERED ) + # Fixture users should be untouched by the script. These will auto update once the + # user logs in / creates an account. + self.fixture_user, _ = User.objects.get_or_create( + username="fixture@igormail.gov", verification_type=User.VerificationTypeChoices.FIXTURE_USER + ) + def tearDown(self): """Deletes all DB objects related to migrations""" super().tearDown() @@ -112,6 +120,7 @@ class TestPopulateVerificationType(MockEppLib): self.assertEqual(self.invited_user.verification_type, User.VerificationTypeChoices.INVITED) self.assertEqual(self.verified_by_staff_user.verification_type, User.VerificationTypeChoices.VERIFIED_BY_STAFF) self.assertEqual(self.untouched_user.verification_type, User.VerificationTypeChoices.GRANDFATHERED) + self.assertEqual(self.fixture_user.verification_type, User.VerificationTypeChoices.FIXTURE_USER) class TestPopulateOrganizationType(MockEppLib): From aad29096c1dbd6c279964094d67a6f88026e2018 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 24 Apr 2024 09:19:15 -0600 Subject: [PATCH 19/75] Fix migrations --- ...ication_type.py => 0088_user_verification_type.py} | 4 ++-- .../django/admin/includes/detail_table_fieldset.html | 11 +++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) rename src/registrar/migrations/{0087_user_verification_type.py => 0088_user_verification_type.py} (84%) diff --git a/src/registrar/migrations/0087_user_verification_type.py b/src/registrar/migrations/0088_user_verification_type.py similarity index 84% rename from src/registrar/migrations/0087_user_verification_type.py rename to src/registrar/migrations/0088_user_verification_type.py index 599d067d5..7fac95a3d 100644 --- a/src/registrar/migrations/0087_user_verification_type.py +++ b/src/registrar/migrations/0088_user_verification_type.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-04-22 16:40 +# Generated by Django 4.2.10 on 2024-04-23 20:47 from django.db import migrations, models @@ -6,7 +6,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("registrar", "0086_domaininformation_updated_federal_agency_and_more"), + ("registrar", "0087_alter_domain_deleted_alter_domain_expiration_date_and_more"), ] operations = [ diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 6374406c1..0f4202b7e 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -4,10 +4,11 @@ {% comment %} This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endcomment %} + {% block field_readonly %} {% with all_contacts=original_object.other_contacts.all %} {% if field.field.name == "creator" %} -
{{ field.contents }} ({{ user.verification_type }})
+
{{ field.contents }} ({{ user.get_verification_type_display }})
{% elif field.field.name == "other_contacts" %} {% if all_contacts.count > 2 %}
@@ -68,9 +69,15 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endblock field_readonly %} {% block help_text %} -
+
{{ field.field.help_text|safe }}
+ + {% if not field.is_readonly and field.field.name == "creator" %} +
+
({{ user.get_verification_type_display }})
+
+ {% endif %} {% endblock help_text %} From b27fce713b06d69161357991482ba75bed19b648 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 24 Apr 2024 12:01:06 -0400 Subject: [PATCH 20/75] Make collapsible fieldsets toggle accessible --- src/registrar/admin.py | 20 +++++++++++------- src/registrar/assets/sass/_theme/_admin.scss | 18 +++++++++++----- src/registrar/templates/admin/base_site.html | 1 + src/registrar/templates/admin/fieldset.html | 18 ++++++++++++++-- src/registrar/tests/test_admin.py | 22 ++++++++++++++++++++ 5 files changed, 64 insertions(+), 15 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 42d73f10d..1e232d217 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1001,9 +1001,10 @@ class DomainInformationAdmin(ListHeaderAdmin): }, ), ( - "More details", + "Show details", { - "classes": ["collapse"], + "classes": ["collapse--dotgov"], + "description": "Extends type of organization", "fields": [ "federal_type", # "updated_federal_agency", @@ -1026,9 +1027,10 @@ class DomainInformationAdmin(ListHeaderAdmin): }, ), ( - "More details", + "Show details", { - "classes": ["collapse"], + "classes": ["collapse--dotgov"], + "description": "Extends organization name and mailing address", "fields": [ "address_line1", "address_line2", @@ -1242,9 +1244,10 @@ class DomainRequestAdmin(ListHeaderAdmin): }, ), ( - "More details", + "Show details", { - "classes": ["collapse"], + "classes": ["collapse--dotgov"], + "description": "Extends type of organization", "fields": [ "federal_type", # "updated_federal_agency", @@ -1267,9 +1270,10 @@ class DomainRequestAdmin(ListHeaderAdmin): }, ), ( - "More details", + "Show details", { - "classes": ["collapse"], + "classes": ["collapse--dotgov"], + "description": "Extends organization name and mailing address", "fields": [ "address_line1", "address_line2", diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index f5717d067..7684b78a1 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -525,17 +525,25 @@ address.dja-address-contact-list { } // Collapse button styles for fieldsets -.module.collapse { +.module.collapse--dotgov { margin-top: -35px; padding-top: 0; border: none; - h2 { + button { background: none; - color: var(--body-fg)!important; text-transform: none; - } - a { color: var(--link-fg); + margin-top: 8px; + margin-left: 10px; + span { + text-decoration: underline; + } + } +} +.collapse--dotgov.collapsed .collapse-toggle--dotgov { + display: inline-block!important; + * { + display: inline-block; } } diff --git a/src/registrar/templates/admin/base_site.html b/src/registrar/templates/admin/base_site.html index 58843421a..dd680cec5 100644 --- a/src/registrar/templates/admin/base_site.html +++ b/src/registrar/templates/admin/base_site.html @@ -23,6 +23,7 @@ + {% endblock %} {% block title %}{% if subtitle %}{{ subtitle }} | {% endif %}{{ title }} | {{ site_title|default:_('Django site admin') }}{% endblock %} diff --git a/src/registrar/templates/admin/fieldset.html b/src/registrar/templates/admin/fieldset.html index 8b8972e08..98860f264 100644 --- a/src/registrar/templates/admin/fieldset.html +++ b/src/registrar/templates/admin/fieldset.html @@ -6,9 +6,23 @@ It is not inherently customizable on its own, so we can modify this instead. https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/includes/fieldset.html {% endcomment %}
- {% if fieldset.name %}

{{ fieldset.name }}

{% endif %} + {% if fieldset.name %} + {# Customize the markup for the collapse toggle #} + {% if 'collapse--dotgov' in fieldset.classes %} + + {{ fieldset.description }} + {% else %} +

{{ fieldset.name }}

+ {% endif %} + {% endif %} - {% if fieldset.description %} + {# Customize the markup for the collapse toggle: Do not show a description for the collapse fieldsets, instead we're using the description as a screen reader only legend #} + {% if fieldset.description and 'collapse--dotgov' not in fieldset.classes %}
{{ fieldset.description|safe }}
{% endif %} diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 42baae6ef..251a769c3 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -887,6 +887,28 @@ class TestDomainRequestAdmin(MockEppLib): ] self.test_helper.assert_response_contains_distinct_values(response, expected_values) + @less_console_noise_decorator + def test_collaspe_toggle_button_markup(self): + """ + Tests for the correct collapse toggle button markup + """ + + # Create a fake domain request and domain + domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) + + p = "adminpass" + self.client.login(username="superuser", password=p) + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain_request.requested_domain.name) + + self.test_helper.assertContains(response, "Show details") + @less_console_noise_decorator def test_analyst_can_see_and_edit_alternative_domain(self): """Tests if an analyst can still see and edit the alternative domain field""" From f754dfcfad44959df3680983c91df36aff5c70d2 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 24 Apr 2024 13:15:07 -0400 Subject: [PATCH 21/75] Fix icon --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 6254fc3bb..14dc7b3e2 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -101,7 +101,7 @@

Why don't I see my domain when I sign in to the registrar? From 87f2dea92cb547f92fee768986b9d0b0c3df6953 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 24 Apr 2024 11:18:31 -0600 Subject: [PATCH 22/75] Fixed width bottom table --- src/registrar/assets/sass/_theme/_tables.scss | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_tables.scss b/src/registrar/assets/sass/_theme/_tables.scss index 0d58b5878..3f6c56447 100644 --- a/src/registrar/assets/sass/_theme/_tables.scss +++ b/src/registrar/assets/sass/_theme/_tables.scss @@ -108,8 +108,24 @@ padding: units(2) units(2) units(2) 0; } - th:first-of-type { - padding-left: 0; + th:nth-of-type(1) { + width: 200px; + } + + th:nth-of-type(2) { + width: 158px; + } + + th:nth-of-type(3) { + width: 120px; + } + + th:nth-of-type(4) { + width: 95px; + } + + th:nth-of-type(5) { + width: 85px; } thead tr:first-child th:first-child { From fdadd68b969edd37cda9a3739a15a01688053d34 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 24 Apr 2024 11:26:43 -0600 Subject: [PATCH 23/75] More widths --- src/registrar/assets/sass/_theme/_tables.scss | 60 ++++++++++++------- src/registrar/templates/home.html | 4 +- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_tables.scss b/src/registrar/assets/sass/_theme/_tables.scss index 3f6c56447..7214ffee0 100644 --- a/src/registrar/assets/sass/_theme/_tables.scss +++ b/src/registrar/assets/sass/_theme/_tables.scss @@ -108,28 +108,48 @@ padding: units(2) units(2) units(2) 0; } - th:nth-of-type(1) { - width: 200px; - } - - th:nth-of-type(2) { - width: 158px; - } - - th:nth-of-type(3) { - width: 120px; - } - - th:nth-of-type(4) { - width: 95px; - } - - th:nth-of-type(5) { - width: 85px; - } - thead tr:first-child th:first-child { border-top: none; } } } + +.dotgov-table__domain-requests { + th:nth-of-type(1) { + width: 200px; + } + + th:nth-of-type(2) { + width: 158px; + } + + th:nth-of-type(3) { + width: 120px; + } + + th:nth-of-type(4) { + width: 95px; + } + + th:nth-of-type(5) { + width: 85px; + } +} + +.dotgov-table__registered-domains { + th:nth-of-type(1) { + width: 200px; + } + + th:nth-of-type(2) { + width: 158px; + } + + th:nth-of-type(3) { + width: 215px; + } + + th:nth-of-type(4) { + width: 95px; + } +} diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index ea9276b9f..5c1bc893a 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -26,7 +26,7 @@

Domains

{% if domains %} - +
@@ -104,7 +104,7 @@

Domain requests

{% if domain_requests %} -
Your registered domains
+
From f589dfaa8e4dd8023bf44b9594584cbef7de601d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 24 Apr 2024 11:59:42 -0600 Subject: [PATCH 24/75] max width --- src/registrar/assets/sass/_theme/_tables.scss | 61 ++++++++++--------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_tables.scss b/src/registrar/assets/sass/_theme/_tables.scss index 7214ffee0..37536d2be 100644 --- a/src/registrar/assets/sass/_theme/_tables.scss +++ b/src/registrar/assets/sass/_theme/_tables.scss @@ -113,43 +113,46 @@ } } } +@media (max-width: 1040px){ + .dotgov-table__domain-requests { + th:nth-of-type(1) { + width: 200px; + } -.dotgov-table__domain-requests { - th:nth-of-type(1) { - width: 200px; - } + th:nth-of-type(2) { + width: 158px; + } - th:nth-of-type(2) { - width: 158px; - } + th:nth-of-type(3) { + width: 120px; + } - th:nth-of-type(3) { - width: 120px; - } + th:nth-of-type(4) { + width: 95px; + } - th:nth-of-type(4) { - width: 95px; - } - - th:nth-of-type(5) { - width: 85px; + th:nth-of-type(5) { + width: 85px; + } } } -.dotgov-table__registered-domains { - th:nth-of-type(1) { - width: 200px; - } +@media (max-width: 1040px){ + .dotgov-table__registered-domains { + th:nth-of-type(1) { + width: 200px; + } - th:nth-of-type(2) { - width: 158px; - } + th:nth-of-type(2) { + width: 158px; + } - th:nth-of-type(3) { - width: 215px; - } + th:nth-of-type(3) { + width: 215px; + } - th:nth-of-type(4) { - width: 95px; + th:nth-of-type(4) { + width: 95px; + } } -} +} \ No newline at end of file From 94a9ab1d03116b3b8ce77794f694a468f40ac2aa Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 24 Apr 2024 12:13:22 -0600 Subject: [PATCH 25/75] Change max width to min width --- src/registrar/assets/sass/_theme/_tables.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_tables.scss b/src/registrar/assets/sass/_theme/_tables.scss index 37536d2be..5dc69e149 100644 --- a/src/registrar/assets/sass/_theme/_tables.scss +++ b/src/registrar/assets/sass/_theme/_tables.scss @@ -113,7 +113,7 @@ } } } -@media (max-width: 1040px){ +@media (min-width: 1040px){ .dotgov-table__domain-requests { th:nth-of-type(1) { width: 200px; @@ -137,7 +137,7 @@ } } -@media (max-width: 1040px){ +@media (min-width: 1040px){ .dotgov-table__registered-domains { th:nth-of-type(1) { width: 200px; From 3d8e99cf5bae540876b2598e02f10a0c3f621da3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 24 Apr 2024 16:36:41 -0400 Subject: [PATCH 26/75] tweaks --- src/registrar/assets/sass/_theme/_links.scss | 10 +++++----- src/registrar/templates/home.html | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_links.scss b/src/registrar/assets/sass/_theme/_links.scss index aa5a85f01..1d51014c5 100644 --- a/src/registrar/assets/sass/_theme/_links.scss +++ b/src/registrar/assets/sass/_theme/_links.scss @@ -5,11 +5,7 @@ p, .dotgov-table { display: flex; align-items: flex-start; color: color('primary'); - - &:visited { - color: color('primary'); - } - + .usa-icon { // align icon with x height margin-top: units(0.5); @@ -17,3 +13,7 @@ p, .dotgov-table { } } } + +.dotgov-table a:visited { + color: color('primary'); +} diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 14dc7b3e2..fd727963b 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -99,7 +99,7 @@ {% else %}

You don't have any registered domains.

- + From aef8e818afc529133ea6982d3c4ba8fefd0bfcb2 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 24 Apr 2024 16:56:12 -0400 Subject: [PATCH 27/75] Cleanup --- src/registrar/assets/sass/_theme/_links.scss | 20 +++++++++++++------- src/registrar/templates/home.html | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_links.scss b/src/registrar/assets/sass/_theme/_links.scss index 1d51014c5..0a3321be0 100644 --- a/src/registrar/assets/sass/_theme/_links.scss +++ b/src/registrar/assets/sass/_theme/_links.scss @@ -1,19 +1,25 @@ @use "uswds-core" as *; -p, .dotgov-table { +.dotgov-table { a { display: flex; align-items: flex-start; color: color('primary'); - .usa-icon { - // align icon with x height - margin-top: units(0.5); - margin-right: units(0.5); + &:visited { + color: color('primary'); } } } -.dotgov-table a:visited { - color: color('primary'); +a { + + // display: flex; + // align-items: flex-start; + + .usa-icon { + // align icon with x height + margin-top: units(0.5); + margin-right: units(0.5); + } } diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index fd727963b..7d73ba473 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -99,7 +99,7 @@ {% else %}

You don't have any registered domains.

- + From 13a4782688cf4d51a1b9b069c17c36e6fca2a0d0 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 24 Apr 2024 17:01:00 -0400 Subject: [PATCH 28/75] Cleanp --- src/registrar/assets/sass/_theme/_links.scss | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_links.scss b/src/registrar/assets/sass/_theme/_links.scss index 0a3321be0..6d2e75a68 100644 --- a/src/registrar/assets/sass/_theme/_links.scss +++ b/src/registrar/assets/sass/_theme/_links.scss @@ -13,10 +13,6 @@ } a { - - // display: flex; - // align-items: flex-start; - .usa-icon { // align icon with x height margin-top: units(0.5); From 4fa777d671bfd46b233b67dd8c9dc34c74072bda Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 25 Apr 2024 09:58:45 -0600 Subject: [PATCH 29/75] Add verification type --- .../admin/includes/contact_detail_list.html | 7 ++++++- .../admin/includes/detail_table_fieldset.html | 19 ++----------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/contact_detail_list.html b/src/registrar/templates/django/admin/includes/contact_detail_list.html index 5ac5452e3..3b49e62a4 100644 --- a/src/registrar/templates/django/admin/includes/contact_detail_list.html +++ b/src/registrar/templates/django/admin/includes/contact_detail_list.html @@ -47,7 +47,12 @@ {% else %} None
{% endif %} + {% else %} - No additional contact information found. + No additional contact information found.
+ {% endif %} + + {% if user_verification_type %} + {{ user_verification_type }} {% endif %} diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 0f4202b7e..c85bfd27a 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -7,9 +7,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block field_readonly %} {% with all_contacts=original_object.other_contacts.all %} - {% if field.field.name == "creator" %} -

{{ field.contents }} ({{ user.get_verification_type_display }})
- {% elif field.field.name == "other_contacts" %} + {% if field.field.name == "other_contacts" %} {% if all_contacts.count > 2 %}
{% for contact in all_contacts %} @@ -68,24 +66,11 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endwith %} {% endblock field_readonly %} -{% block help_text %} -
-
{{ field.field.help_text|safe }}
-
- - {% if not field.is_readonly and field.field.name == "creator" %} -
-
({{ user.get_verification_type_display }})
-
- {% endif %} -{% endblock help_text %} - - {% block after_help_text %} {% if field.field.name == "creator" %}
- {% include "django/admin/includes/contact_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly %} + {% include "django/admin/includes/contact_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly user_verification_type=original_object.creator.get_verification_type_display%}
{% include "django/admin/includes/user_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly %} {% elif field.field.name == "submitter" %} From 07ac2b8fa4f2f15fb62b5f550c44c28b275fb1fa Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 25 Apr 2024 10:06:23 -0600 Subject: [PATCH 30/75] Add padding --- src/registrar/assets/sass/_theme/_admin.scss | 4 ++++ .../django/admin/includes/detail_table_fieldset.html | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index f5717d067..e22ec905e 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -617,3 +617,7 @@ address.dja-address-contact-list { .usa-button__small-text { font-size: small; } + +.padding-left-0__important { + padding-left: 0px !important; +} diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index c85bfd27a..123773313 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -66,9 +66,15 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endwith %} {% endblock field_readonly %} +{% block help_text %} +
+
{{ field.field.help_text|safe }}
+
+{% endblock help_text %} + {% block after_help_text %} {% if field.field.name == "creator" %} -
+
{% include "django/admin/includes/contact_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly user_verification_type=original_object.creator.get_verification_type_display%}
From 2a10e76c5d7b244e63c18b51c6f0e5e7ecf32a4c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 25 Apr 2024 10:12:50 -0600 Subject: [PATCH 31/75] Add additional padding --- .../django/admin/includes/detail_table_fieldset.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 123773313..d96fa469c 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -74,13 +74,13 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block after_help_text %} {% if field.field.name == "creator" %} -
+
{% include "django/admin/includes/contact_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly user_verification_type=original_object.creator.get_verification_type_display%}
{% include "django/admin/includes/user_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly %} {% elif field.field.name == "submitter" %} -
+
{% include "django/admin/includes/contact_detail_list.html" with user=original_object.submitter no_title_top_padding=field.is_readonly %}
From 9e6cddb02e94e950ef4a01d7a0d9c6e0e17c8b22 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 25 Apr 2024 10:18:25 -0600 Subject: [PATCH 32/75] Margin top 2 --- .../django/admin/includes/detail_table_fieldset.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index d96fa469c..9ec5e500d 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -74,13 +74,13 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block after_help_text %} {% if field.field.name == "creator" %} -
+
{% include "django/admin/includes/contact_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly user_verification_type=original_object.creator.get_verification_type_display%}
{% include "django/admin/includes/user_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly %} {% elif field.field.name == "submitter" %} -
+
{% include "django/admin/includes/contact_detail_list.html" with user=original_object.submitter no_title_top_padding=field.is_readonly %}
From 9d7a1db22659663e1c15be94532f1235ace5d740 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 25 Apr 2024 16:48:43 -0400 Subject: [PATCH 33/75] Design tweaks --- src/registrar/assets/sass/_theme/_admin.scss | 5 +++++ src/registrar/templates/admin/fieldset.html | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 7684b78a1..2dcbfbb06 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -537,6 +537,11 @@ address.dja-address-contact-list { margin-left: 10px; span { text-decoration: underline; + font-size: 13px; + font-feature-settings: "kern"; + font-kerning: normal; + line-height: 13px; + font-family: -apple-system, "system-ui", "Segoe UI", system-ui, Roboto, "Helvetica Neue", Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji"; } } } diff --git a/src/registrar/templates/admin/fieldset.html b/src/registrar/templates/admin/fieldset.html index 98860f264..b58bd0241 100644 --- a/src/registrar/templates/admin/fieldset.html +++ b/src/registrar/templates/admin/fieldset.html @@ -9,7 +9,7 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/ {% if fieldset.name %} {# Customize the markup for the collapse toggle #} {% if 'collapse--dotgov' in fieldset.classes %} -
Your domain requests