diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 2a9d0e3c3..586100d66 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -13,6 +13,7 @@ ) from django.db.models.functions import Concat, Coalesce +from django.core.exceptions import ValidationError from django.http import HttpResponseRedirect from registrar.models.federal_agency import FederalAgency from registrar.models.portfolio_invitation import PortfolioInvitation @@ -71,6 +72,7 @@ from django.contrib.admin.widgets import FilteredSelectMultiple from django.utils.html import format_html from django.utils.translation import gettext_lazy as _ +from django.utils.dateparse import parse_datetime logger = logging.getLogger(__name__) @@ -3088,6 +3090,20 @@ def get_fieldsets(self, request, obj=None): modified_fieldsets.append((name, {**data, "fields": fields})) return modified_fieldsets + def _has_version_conflict(self, request, obj): + """ + Returns True if there is a version conflict, False otherwise. + """ + version = request.POST.get("version") + snap = parse_datetime(version) if version else None + current = type(obj).objects.only("updated_at").filter(pk=obj.pk).values_list("updated_at", flat=True).first() + + if snap and current and current != snap: + return True + else: + obj._original_updated_at = snap + return False + # Trigger action when a fieldset is changed def save_model(self, request, obj, form, change): """Custom save_model definition that handles edge cases""" @@ -3118,8 +3134,12 @@ def save_model(self, request, obj, form, change): return None - # == Check if we're making a change or not == # + # == OPTIMISTIC LOCK: Temp fix check for version conflict == # + if self._has_version_conflict(request, obj): + request._domain_request_version_conflict = True + return None + # == Check if we're making a change or not == # # If we're not making a change (adding a record), run save model as we do normally if not change: return super().save_model(request, obj, form, change) @@ -3147,6 +3167,13 @@ def save_model(self, request, obj, form, change): if should_save: return super().save_model(request, obj, form, change) + def response_change(self, request, obj): + if getattr(request, "_domain_request_version_conflict", False): + change_url = reverse("admin:registrar_domainrequest_change", args=[obj.pk]) + self.message_user(request, "A newer version of this form exists. Please try again.", messages.ERROR) + return HttpResponseRedirect(change_url) + return super().response_change(request, obj) + def _handle_custom_emails(self, obj): if obj.status == DomainRequest.DomainRequestStatus.ACTION_NEEDED: if obj.action_needed_reason and not obj.action_needed_reason_email: @@ -4310,6 +4337,12 @@ def do_extend_expiration_date(self, request, obj): "Error connecting to the registry. No expiration date was found.", messages.ERROR, ) + except ValidationError: + self.message_user( + request, + "A newer version of this form exists. Please try again.", + messages.ERROR, + ) except Exception as err: logger.error(err, stack_info=True) self.message_user(request, "Could not delete: An unspecified error occured", messages.ERROR) @@ -4333,7 +4366,7 @@ def do_delete_domain(self, request, obj): try: obj.deletedInEpp() - obj.save() + obj.save(optimistic_lock=True) except RegistryError as err: # Using variables to get past the linter message1 = f"Cannot delete Domain when in state {obj.state}" @@ -4369,6 +4402,8 @@ def do_delete_domain(self, request, obj): ), messages.ERROR, ) + except ValidationError: + self.message_user(request, "A newer version of this form exists. Please try again.", messages.ERROR) except Exception: self.message_user( request, @@ -4396,9 +4431,12 @@ def do_get_status(self, request, obj): return HttpResponseRedirect(".") def do_place_client_hold(self, request, obj): + try: obj.place_client_hold() - obj.save() + obj.save(optimistic_lock=True) + except ValidationError: + self.message_user(request, "A newer version of this form exists. Please try again.", messages.ERROR) except Exception as err: # if error is an error from the registry, display useful # and readable error @@ -4427,7 +4465,9 @@ def do_place_client_hold(self, request, obj): def do_remove_client_hold(self, request, obj): try: obj.revert_client_hold() - obj.save() + obj.save(optimistic_lock=True) + except ValidationError: + self.message_user(request, "A newer version of this form exists. Please try again.", messages.ERROR) except Exception as err: # if error is an error from the registry, display useful # and readable error diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index f49da80ec..b35f302a1 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -580,7 +580,7 @@ def to_database(self, obj): else: requested_domain = DraftDomain.objects.create(name=f"{domain}.gov") obj.requested_domain = requested_domain - obj.save() + obj.save(optimistic_lock=True) obj.save() diff --git a/src/registrar/forms/utility/wizard_form_helper.py b/src/registrar/forms/utility/wizard_form_helper.py index 73006806a..cfca77ef8 100644 --- a/src/registrar/forms/utility/wizard_form_helper.py +++ b/src/registrar/forms/utility/wizard_form_helper.py @@ -33,7 +33,11 @@ def to_database(self, obj: DomainRequest | Contact): return for name, value in self.cleaned_data.items(): setattr(obj, name, value) - obj.save() + + if isinstance(obj, DomainRequest): + obj.save(optimistic_lock=True) + else: + obj.save() @classmethod def from_database(cls, obj: DomainRequest | Contact | None): @@ -101,7 +105,7 @@ def _to_database( """ if not self.is_valid(): return - obj.save() + obj.save(optimistic_lock=True) query = getattr(obj, join).order_by("created_at").all() # order matters @@ -209,7 +213,7 @@ def to_database(self, obj): else: for name, value in self.cleaned_data.items(): setattr(obj, name, value) - obj.save() + obj.save(optimistic_lock=True) class BaseYesNoForm(RegistrarForm): diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 4f8a193a1..442cce214 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -10,6 +10,7 @@ from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignore from django.utils import timezone from functools import cached_property +from django.core.exceptions import ValidationError from typing import Any from registrar.models.domain_invitation import DomainInvitation from registrar.models.host import Host @@ -88,6 +89,7 @@ class Meta: def __init__(self, *args, **kwargs): self._cache = {} super(Domain, self).__init__(*args, **kwargs) + self._original_updated_at = self.__dict__.get("updated_at", None) class Status(models.TextChoices): """ @@ -241,12 +243,19 @@ def __delete__(self, obj): """Called during delete. Example: `del domain.registrant`.""" super().__delete__(obj) - def save(self, force_insert=False, force_update=False, using=None, update_fields=None): + def save(self, force_insert=False, force_update=False, using=None, update_fields=None, optimistic_lock=False): + # -------- Optimistic locking (quick-fix) -------- + if optimistic_lock and self.pk: + current_updated_at = type(self).objects.only("updated_at").get(pk=self.pk).updated_at + if getattr(self, "_original_updated_at", None) and current_updated_at != self._original_updated_at: + raise ValidationError("A newer version of this form exists. Please try again.") + # If the domain is deleted we don't want the expiration date to be set if self.state == self.State.DELETED and self.expiration_date: self.expiration_date = None super().save(force_insert, force_update, using, update_fields) + self._original_updated_at = self.updated_at @classmethod def available(cls, domain: str) -> bool: @@ -369,7 +378,9 @@ def registry_expiration_date(self, ex_date: date): To update the expiration date, use renew_domain method.""" raise NotImplementedError() - def renew_domain(self, length: int = 1, unit: epp.Unit = epp.Unit.YEAR): + def renew_domain( + self, length: int = 1, unit: epp.Unit = epp.Unit.YEAR, persist: bool = True, optimistic_lock: bool = False + ) -> bool: """ Renew the domain to a length and unit of time relative to the current expiration date. @@ -392,8 +403,13 @@ def renew_domain(self, length: int = 1, unit: epp.Unit = epp.Unit.YEAR): # expiration date in the registrar, and in the cache self._cache["ex_date"] = registry.send(request, cleaned=True).res_data[0].ex_date self.expiration_date = self._cache["ex_date"] - self.save() - + if persist: + if optimistic_lock: + self.save(optimistic_lock=True) + else: + self.save() + return True + return False except RegistryError as err: # if registry error occurs, log the error, and raise it as well logger.error(f"Registry error renewing domain '{self.name}': {err}") diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index b21434019..d5b0052ca 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -723,6 +723,7 @@ def _cache_status_and_status_reasons(self): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + self._original_updated_at = self.__dict__.get("updated_at", None) # Store original values for caching purposes. Used to compare them on save. self._cache_status_and_status_reasons() @@ -779,8 +780,21 @@ def clean(self): ) raise ValidationError(errors) - def save(self, *args, **kwargs): + def save(self, *args, optimistic_lock=False, **kwargs): """Save override for custom properties""" + if optimistic_lock and self.pk is not None: + # Get the current DB value to compare with our snapshot + current_updated_at = ( + type(self).objects.only("updated_at").filter(pk=self.pk).values_list("updated_at", flat=True).first() + ) + # If someone else saved after we loaded, block the save + if ( + self._original_updated_at is not None + and current_updated_at is not None + and current_updated_at != self._original_updated_at + ): + raise ValidationError("A newer version of this form exists. Please try again.") + self.sync_organization_type() self.sync_yes_no_form_fields() @@ -788,7 +802,7 @@ def save(self, *args, **kwargs): self.last_status_update = timezone.now().date() super().save(*args, **kwargs) - + self._original_updated_at = self.updated_at # Handle custom status emails. # An email is sent out when a, for example, action_needed_reason is changed or added. statuses_that_send_custom_emails = [self.DomainRequestStatus.ACTION_NEEDED, self.DomainRequestStatus.REJECTED] diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html index f8c9f77d8..cb68b5c06 100644 --- a/src/registrar/templates/django/admin/domain_change_form.html +++ b/src/registrar/templates/django/admin/domain_change_form.html @@ -44,7 +44,6 @@ {% endif %} - {% for fieldset in adminform %} {% include "django/admin/includes/domain_fieldset.html" with state_help_message=state_help_message %} {% endfor %} diff --git a/src/registrar/templates/django/admin/domain_request_change_form.html b/src/registrar/templates/django/admin/domain_request_change_form.html index 46965f236..69f6f94fd 100644 --- a/src/registrar/templates/django/admin/domain_request_change_form.html +++ b/src/registrar/templates/django/admin/domain_request_change_form.html @@ -10,6 +10,7 @@ {% endblock content %} {% block field_sets %} + {# Create an invisible tag so that we can use a click event to toggle the modal. #} {# Store the current object id so we can access it easier #} diff --git a/src/registrar/templates/domain_request_form.html b/src/registrar/templates/domain_request_form.html index 57ad6ef7f..5926ea831 100644 --- a/src/registrar/templates/domain_request_form.html +++ b/src/registrar/templates/domain_request_form.html @@ -85,6 +85,7 @@

{{form_titles|get_item:steps.current}}

{% block form_fields %}{% endblock %} {% block form_buttons %} +
{% if steps.prev %}