diff --git a/src/registrar/fixtures_applications.py b/src/registrar/fixtures_applications.py index 3e4e0e362..29e95e02b 100644 --- a/src/registrar/fixtures_applications.py +++ b/src/registrar/fixtures_applications.py @@ -1,7 +1,7 @@ import logging import random from faker import Faker - +from django.db import transaction from registrar.models import ( User, DomainApplication, @@ -184,6 +184,14 @@ class DomainApplicationFixture: logger.warning(e) return + # Lumped under .atomic to ensure we don't make redundant DB calls. + # This bundles them all together, and then saves it in a single call. + with transaction.atomic(): + cls._create_applications(users) + + @classmethod + def _create_applications(cls, users): + """Creates DomainApplications given a list of users""" for user in users: logger.debug("Loading domain applications for %s" % user) for app in cls.DA: @@ -211,8 +219,16 @@ class DomainFixture(DomainApplicationFixture): logger.warning(e) return + # Lumped under .atomic to ensure we don't make redundant DB calls. + # This bundles them all together, and then saves it in a single call. + with transaction.atomic(): + # approve each user associated with `in review` status domains + DomainFixture._approve_applications(users) + + @staticmethod + def _approve_applications(users): + """Approves all provided applications if they are in the state in_review""" for user in users: - # approve one of each users in review status domains application = DomainApplication.objects.filter( creator=user, status=DomainApplication.ApplicationStatus.IN_REVIEW ).last() @@ -220,5 +236,13 @@ class DomainFixture(DomainApplicationFixture): # We don't want fixtures sending out real emails to # fake email addresses, so we just skip that and log it instead + + # All approvals require an investigator, so if there is none, + # assign one. + if application.investigator is None: + # All "users" in fixtures have admin perms per prior config. + # No need to check for that. + application.investigator = random.choice(users) + application.approve(send_email=False) - application.save() + application.save() \ No newline at end of file diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index f03fb025d..830adb5bf 100644 --- a/src/registrar/fixtures_users.py +++ b/src/registrar/fixtures_users.py @@ -1,5 +1,6 @@ import logging from faker import Faker +from django.db import transaction from registrar.models import ( User, @@ -186,5 +187,12 @@ class UserFixture: @classmethod def load(cls): - cls.load_users(cls, cls.ADMINS, "full_access_group") - cls.load_users(cls, cls.STAFF, "cisa_analysts_group") + # Lumped under .atomic to ensure we don't make redundant DB calls. + # This bundles them all together, and then saves it in a single call. + # This is slightly different then bulk_create or bulk_update, in that + # you still get the same behaviour of .save(), but those incremental + # steps now do not need to close/reopen a db connection, + # instead they share one. + with transaction.atomic(): + cls.load_users(cls, cls.ADMINS, "full_access_group") + cls.load_users(cls, cls.STAFF, "cisa_analysts_group") diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 17bc71cbe..5f204d3e3 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -8,6 +8,7 @@ from django.db import models from django_fsm import FSMField, transition # type: ignore from django.utils import timezone from registrar.models.domain import Domain +from registrar.utility.errors import ApplicationStatusError, FSMErrorCodes from .utility.time_stamped_model import TimeStampedModel from ..utility.email import send_templated_email, EmailSendingError @@ -736,8 +737,23 @@ class DomainApplication(TimeStampedModel): # create the domain Domain = apps.get_model("registrar.Domain") + + # == Check that the application is valid == # if Domain.objects.filter(name=self.requested_domain.name).exists(): - raise ValueError("Cannot approve. Requested domain is already in use.") + raise ApplicationStatusError(code=FSMErrorCodes.APPROVE_DOMAIN_IN_USE) + + # Check if an investigator is assigned. No approval is possible without one. + # TODO - add form level error + # TODO - maybe add modal if approver is not investigator as superuser + if self.investigator is None: + raise ApplicationStatusError(code=FSMErrorCodes.APPROVE_NO_INVESTIGATOR) + + # Investigators must be staff users. + # This is handled elsewhere, but we should check here as a precaution. + if not self.investigator.is_staff: + raise ApplicationStatusError(code=FSMErrorCodes.APPROVE_INVESTIGATOR_NOT_STAFF) + + # == Create the domain and related components == # created_domain = Domain.objects.create(name=self.requested_domain.name) self.approved_domain = created_domain @@ -751,6 +767,7 @@ class DomainApplication(TimeStampedModel): user=self.creator, domain=created_domain, role=UserDomainRole.Roles.MANAGER ) + # == Send out an email == # self._send_status_update_email( "application approved", "emails/status_change_approved.txt", diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 03cb81893..21bdeefc6 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -70,6 +70,51 @@ class GenericError(Exception): def get_error_message(self, code=None): return self._error_mapping.get(code) +# (Q for reviewers) What should this be called? +# Not a fan of this name. +class FSMErrorCodes(IntEnum): + """Used when doing FSM transitions. + Overview of generic error codes: + - 1 APPROVE_DOMAIN_IN_USE The domain is already in use + - 2 APPROVE_NO_INVESTIGATOR No investigator is assigned when approving + - 3 APPROVE_INVESTIGATOR_NOT_STAFF Investigator is a non-staff user + """ + APPROVE_DOMAIN_IN_USE = 1 + APPROVE_NO_INVESTIGATOR = 2 + APPROVE_INVESTIGATOR_NOT_STAFF = 3 + + +class ApplicationStatusError(Exception): + """ + Used to raise exceptions when doing FSM Transitions. + Uses `FSMErrorCodes` as an enum. + """ + + _error_mapping = { + FSMErrorCodes.APPROVE_DOMAIN_IN_USE: ( + "Cannot approve. Requested domain is already in use." + ), + FSMErrorCodes.APPROVE_NO_INVESTIGATOR: ( + "Cannot approve. No investigator was assigned." + ), + FSMErrorCodes.APPROVE_INVESTIGATOR_NOT_STAFF: ( + "Cannot approve. Investigator is not a staff user." + ), + } + + def __init__(self, *args, code=None, **kwargs): + super().__init__(*args, **kwargs) + self.code = code + if self.code in self._error_mapping: + self.message = self._error_mapping.get(self.code) + + def __str__(self): + return f"{self.message}" + + @classmethod + def get_error_message(self, code=None): + return self._error_mapping.get(code) + class NameserverErrorCodes(IntEnum): """Used in the NameserverError class for