Simplify logic

src/registrar/forms/domain.py

@@ -1,9 +1,9 @@
 """Forms for domain management."""
-
+import logging
 from django import forms
 from django.core.validators import MinValueValidator, MaxValueValidator, RegexValidator
 from django.forms import formset_factory
-
+from registrar.models import DomainApplication
 from phonenumber_field.widgets import RegionalPhoneNumberWidget
 from registrar.utility.errors import (
     NameserverError,
@@ -23,6 +23,9 @@ from .common import (
 import re
 
 
+logger = logging.getLogger(__name__)
+
+
 class DomainAddUserForm(forms.Form):
     """Form for adding a user to a domain."""
 
@@ -205,6 +208,13 @@ class ContactForm(forms.ModelForm):
             "required": "Enter your email address in the required format, like name@example.com."
         }
         self.fields["phone"].error_messages["required"] = "Enter your phone number."
+        self.domainInfo = None
+
+    def set_domain_info(self, domainInfo):
+        """Set the domain information for the form.
+        The form instance is associated with the contact itself. In order to access the associated
+        domain information object, this needs to be set in the form by the view."""
+        self.domainInfo = domainInfo
 
 
 class AuthorizingOfficialContactForm(ContactForm):
@@ -232,20 +242,32 @@ class AuthorizingOfficialContactForm(ContactForm):
         self.fields["email"].error_messages = {
             "required": "Enter an email address in the required format, like name@example.com."
         }
-        self.domainInfo = None
-
-    def set_domain_info(self, domainInfo):
-        """Set the domain information for the form.
-        The form instance is associated with the contact itself. In order to access the associated
-        domain information object, this needs to be set in the form by the view."""
-        self.domainInfo = domainInfo
 
     def save(self, commit=True):
-        """Override the save() method of the BaseModelForm."""
+        """
+        Override the save() method of the BaseModelForm.
+        Used to perform checks on the underlying domain_information object.
+        If this doesn't exist, we just save as normal.
+        """
+
+        # If the underlying Domain doesn't have a domainInfo object,
+        # just let the default super handle it.
+        if not self.domainInfo:
+            return super().save()
+
+        # Determine if the domain is federal or tribal
+        is_federal = self.domainInfo.organization_type == DomainApplication.OrganizationChoices.FEDERAL
+        is_tribal = self.domainInfo.organization_type == DomainApplication.OrganizationChoices.TRIBAL
 
         # Get the Contact object from the db for the Authorizing Official
         db_ao = Contact.objects.get(id=self.instance.id)
-        if self.domainInfo and db_ao.has_more_than_one_join("information_authorizing_official"):
+
+        if (is_federal or is_tribal) and self.has_changed():
+            # This action should be blocked by the UI, as the text fields are readonly.
+            # If they get past this point, we forbid it this way.
+            # This could be malicious, so lets reserve information for the backend only.
+            raise ValueError("Authorizing Official cannot be modified for federal or tribal domains.")
+        elif db_ao.has_more_than_one_join("information_authorizing_official"):
             # Handle the case where the domain information object is available and the AO Contact
             # has more than one joined object.
             # In this case, create a new Contact, and update the new Contact with form data.
@@ -254,6 +276,7 @@ class AuthorizingOfficialContactForm(ContactForm):
             self.domainInfo.authorizing_official = Contact.objects.create(**data)
             self.domainInfo.save()
         else:
+            # If all checks pass, just save normally
             super().save()
 
 
@@ -334,6 +357,36 @@ class DomainOrgNameAddressForm(forms.ModelForm):
         self.fields["state_territory"].widget.attrs.pop("maxlength", None)
         self.fields["zipcode"].widget.attrs.pop("maxlength", None)
     
+    def save(self, commit=True):
+        """Override the save() method of the BaseModelForm."""
+        if self.has_changed():
+            is_federal = self.instance.organization_type == DomainApplication.OrganizationChoices.FEDERAL
+            is_tribal = self.instance.organization_type == DomainApplication.OrganizationChoices.TRIBAL
+
+            # This action should be blocked by the UI, as the text fields are readonly.
+            # If they get past this point, we forbid it this way.
+            # This could be malicious, so lets reserve information for the backend only.
+            if is_federal and not self._field_unchanged("federal_agency"):
+                raise ValueError("federal_agency cannot be modified when the organization_type is federal")
+            elif is_tribal and not self._field_unchanged("organization_name"):
+                raise ValueError("organization_name cannot be modified when the organization_type is tribal")
+
+        else:
+            super().save()
+
+    def _field_unchanged(self, field_name) -> bool:
+        """
+        Checks if a specified field has not changed between the old value
+        and the new value.
+
+        The old value is grabbed from self.initial.
+        The new value is grabbed from self.cleaned_data.
+        """
+        old_value = self.initial.get(field_name, None)
+        new_value = self.cleaned_data.get(field_name, None)
+        return old_value == new_value
+
+
 
 class DomainDnssecForm(forms.Form):
     """Form for enabling and disabling dnssec"""

src/registrar/views/domain.py

@@ -213,43 +213,6 @@ class DomainOrgNameAddressView(DomainFormBaseView):
 
     def form_valid(self, form):
         """The form is valid, save the organization name and mailing address."""
-
-        current_domain_info = self.get_domain_info_from_domain()
-        if current_domain_info is None:
-            messages.error(self.request, "Something went wrong when attempting to save.")
-            return self.form_invalid(form)
-
-        # Get the old and new values to see if a change is occuring
-        old_org_info = form.initial
-        new_org_info = form.cleaned_data
-
-        if old_org_info != new_org_info:
-
-            error_message = None
-            # These actions, aside from the default, should be blocked by the UI, as the field is readonly.
-            # If they get past this point, we forbid it this way.
-            # This could be malicious, but it won't always be.
-            match current_domain_info.organization_type:
-                case DomainApplication.OrganizationChoices.FEDERAL:
-                    old_fed_agency = old_org_info.get("federal_agency", None)
-                    new_fed_agency = new_org_info.get("federal_agency", None)
-                    if old_fed_agency != new_fed_agency:
-                        error_message = "You cannot modify Federal Agency"
-                case DomainApplication.OrganizationChoices.TRIBAL:
-                    old_org_name = old_org_info.get("organization_name", None)
-                    new_org_name = new_org_info.get("organization_name", None)
-                    if old_org_name != new_org_name:
-                        error_message = "You cannot modify Organization Name."
-                case _:
-                    # Do nothing
-                    pass
-            
-            # If we encounter an error, forbid this action.
-            if error_message is not None:
-                logger.warning(f"User {self.request.user} attempted to change org info on {self.object.name}")
-                messages.error(self.request, "You cannot modify the Authorizing Official.")
-                return self.form_invalid(form)
-
         form.save()
 
         messages.success(self.request, "The organization information for this domain has been updated.")
@@ -278,31 +241,9 @@ class DomainAuthorizingOfficialView(DomainFormBaseView):
 
     def form_valid(self, form):
         """The form is valid, save the authorizing official."""
-        # if not self.request.user.is_staff:
-        
-        current_domain_info = self.get_domain_info_from_domain()
-        if current_domain_info is None:
-            messages.error(self.request, "Something went wrong when attempting to save.")
-            return self.form_invalid(form)
-
-        # Determine if the domain is federal or tribal
-        is_federal = current_domain_info.organization_type == DomainApplication.OrganizationChoices.FEDERAL
-        is_tribal = current_domain_info.organization_type == DomainApplication.OrganizationChoices.TRIBAL
-
-        # Get the old and new ao values
-        old_authorizing_official = form.initial
-        new_authorizing_official = form.cleaned_data
-
-        # This action should be blocked by the UI, as the text fields are readonly.
-        # If they get past this point, we forbid it this way.
-        # This could be malicious, but it won't always be.
-        if (is_federal or is_tribal) and old_authorizing_official != new_authorizing_official:
-            logger.warning(f"User {self.request.user} attempted to change AO on {self.object.name}")
-            messages.error(self.request, "You cannot modify the Authorizing Official.")
-            return self.form_invalid(form)
 
         # Set the domain information in the form so that it can be accessible
-        # to associate a new Contact as authorizing official, if new Contact is needed
+        # to associate a new Contact, if a new Contact is needed
         # in the save() method
         form.set_domain_info(self.object.domain_info)
         form.save()