diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 19d876728..3562383bd 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1812,8 +1812,70 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return response def change_view(self, request, object_id, form_url="", extra_context=None): + """Display restricted warning, + Setup the auditlog trail and pass it in extra context.""" obj = self.get_object(request, object_id) self.display_restricted_warning(request, obj) + + # Retrieve and order audit log entries by timestamp in ascending order + audit_log_entries = LogEntry.objects.filter(object_id=object_id).order_by("timestamp") + + # Initialize variables for tracking status changes and filtered entries + filtered_entries = [] + + # Process each log entry to filter based on the change criteria + for log_entry in audit_log_entries: + changes = log_entry.changes + status_changed = "status" in changes + rejection_reason_changed = "rejection_reason" in changes + action_needed_reason_changed = "action_needed_reason" in changes + + # Check if the log entry meets the filtering criteria + if status_changed or (not status_changed and (rejection_reason_changed or action_needed_reason_changed)): + entry = {} + + # Handle status change + if status_changed: + entry["status"] = DomainRequest.DomainRequestStatus(changes["status"][1]).label + # last_status = entry["status"] + + # Handle rejection reason change + if rejection_reason_changed: + rejection_reason = changes["rejection_reason"][1] + entry["rejection_reason"] = ( + "" if rejection_reason == "None" else DomainRequest.RejectionReasons(rejection_reason).label + ) + # Handle case where rejection reason changed but not status + if not status_changed: + entry["status"] = DomainRequest.DomainRequestStatus.REJECTED.label + + # Handle action needed reason change + if action_needed_reason_changed: + action_needed_reason = changes["action_needed_reason"][1] + entry["action_needed_reason"] = ( + "" + if action_needed_reason == "None" + else DomainRequest.ActionNeededReasons(action_needed_reason).label + ) + # Handle case where action needed reason changed but not status + if not status_changed: + entry["status"] = DomainRequest.DomainRequestStatus.ACTION_NEEDED.label + + # Add actor and timestamp information + entry["actor"] = log_entry.actor + entry["timestamp"] = log_entry.timestamp + + # Append the filtered entry to the list + filtered_entries.append(entry) + + # Reverse the filtered entries list to get newest to oldest order + filtered_entries.reverse() + + # Initialize extra_context and add filtered entries + if extra_context is None: + extra_context = {} + extra_context["filtered_entries"] = filtered_entries + return super().change_view(request, object_id, form_url, extra_context) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 8dfa419c9..a403f3256 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -17,8 +17,6 @@ from .utility.time_stamped_model import TimeStampedModel from ..utility.email import send_templated_email, EmailSendingError from itertools import chain -from auditlog.models import AuditlogHistoryField # type: ignore - logger = logging.getLogger(__name__) @@ -35,11 +33,7 @@ class DomainRequest(TimeStampedModel): ] # https://django-auditlog.readthedocs.io/en/latest/usage.html#object-history - # If we note any performace degradation due to this addition, - # we can query the auditlogs table in admin.py and add the results to - # extra_context in the change_view method for DomainRequestAdmin. - # This is the more straightforward way so trying it first. - history = AuditlogHistoryField() + # history = AuditlogHistoryField() # Constants for choice fields class DomainRequestStatus(models.TextChoices): 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 791753042..46c4f56ea 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -68,51 +68,52 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endblock field_readonly %} {% block after_help_text %} - {% if field.field.name == "status" and original_object.history.count > 0 %} -
- -
-
- - + {% if field.field.name == "status" and filtered_entries %} +
+ +
+
+
+ + + + + + + + + {% for entry in filtered_entries %} - - - - - - - {% for log_entry in original_object.history.all %} - {% for key, value in log_entry.changes_display_dict.items %} - {% if key == "status" %} - - - - - + -
StatusUserChanged at
StatusUserChanged at
{{ value.1|default:"None" }} - {% for rk, rv in log_entry.changes_display_dict.items %} - {% if rk == "rejection reason" %} - - {{ rv.1|default:"None" }} - {% endif %} - {% endfor %} - - {{ log_entry.actor|default:"None" }} - - {{ log_entry.timestamp|date:"Y-m-d H:i:s" }}
+ {% if entry.status %} + {{ entry.status|default:"None" }} + {% else %} + Error {% endif %} - {% endfor %} - {% endfor %} -
-
- + + {% if entry.rejection_reason %} + - {{ entry.rejection_reason|default:"None" }} + {% endif %} + + {% if entry.action_needed_reason %} + - {{ entry.action_needed_reason|default:"None" }} + {% endif %} + + {{ entry.actor|default:"None" }} + {{ entry.timestamp|date:"Y-m-d H:i:s" }} + + {% endfor %} + +
+
+ {% elif field.field.name == "creator" %}
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index ff19d06be..0252ad371 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1055,6 +1055,18 @@ class TestDomainRequestAdmin(MockEppLib): accurately and in chronological order. """ + def assert_status_count(normalized_content, status, count): + """Helper function to assert the count of a status in the HTML content.""" + self.assertEqual(normalized_content.count(f" {status} "), count) + + def assert_status_order(normalized_content, statuses): + """Helper function to assert the order of statuses in the HTML content.""" + start_index = 0 + for status in statuses: + index = normalized_content.find(f" {status} ", start_index) + self.assertNotEqual(index, -1, f"Status '{status}' not found in the expected order.") + start_index = index + len(status) + # Create a fake domain request and domain domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.STARTED) @@ -1069,48 +1081,23 @@ class TestDomainRequestAdmin(MockEppLib): self.assertEqual(response.status_code, 200) self.assertContains(response, domain_request.requested_domain.name) - # Table will contain one row for Started - self.assertContains(response, "Started", count=1) - self.assertNotContains(response, "Submitted") - domain_request.submit() domain_request.save() - response = self.client.get( - "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), - follow=True, - ) - - # Table will contain and extra row for Submitted - self.assertContains(response, "Started", count=1) - self.assertContains(response, "Submitted", count=1) - domain_request.in_review() domain_request.save() - response = self.client.get( - "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), - follow=True, - ) - - # Table will contain and extra row for In review - self.assertContains(response, "Started", count=1) - self.assertContains(response, "Submitted", count=1) - self.assertContains(response, "In review", count=1) - domain_request.action_needed() + domain_request.action_needed_reason = DomainRequest.ActionNeededReasons.ALREADY_HAS_DOMAINS domain_request.save() - response = self.client.get( - "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), - follow=True, - ) + # Let's just change the action needed reason + domain_request.action_needed_reason = DomainRequest.ActionNeededReasons.ELIGIBILITY_UNCLEAR + domain_request.save() - # Table will contain and extra row for Action needed - self.assertContains(response, "Started", count=1) - self.assertContains(response, "Submitted", count=1) - self.assertContains(response, "In review", count=1) - self.assertContains(response, "Action needed", count=1) + domain_request.reject() + domain_request.rejection_reason = DomainRequest.RejectionReasons.DOMAIN_PURPOSE + domain_request.save() domain_request.in_review() domain_request.save() @@ -1120,24 +1107,28 @@ class TestDomainRequestAdmin(MockEppLib): follow=True, ) + # Normalize the HTML response content + normalized_content = " ".join(response.content.decode("utf-8").split()) + # Define the expected sequence of status changes expected_status_changes = [ - "In review", - "Action needed", - "In review", - "Submitted", - "Started", + "In review", + "Rejected - Purpose requirements not met", + "Action needed - Unclear organization eligibility", + "Action needed - Already has domains", + "In review", + "Submitted", + "Started", ] - # Test for the order of status changes - for status_change in expected_status_changes: - self.assertContains(response, status_change, html=True) + assert_status_order(normalized_content, expected_status_changes) - # Table now contains 2 rows for Approved - self.assertContains(response, "Started", count=1) - self.assertContains(response, "Submitted", count=1) - self.assertContains(response, "In review", count=2) - self.assertContains(response, "Action needed", count=1) + assert_status_count(normalized_content, "Started", 1) + assert_status_count(normalized_content, "Submitted", 1) + assert_status_count(normalized_content, "In review", 2) + assert_status_count(normalized_content, "Action needed - Already has domains", 1) + assert_status_count(normalized_content, "Action needed - Unclear organization eligibility", 1) + assert_status_count(normalized_content, "Rejected - Purpose requirements not met", 1) def test_collaspe_toggle_button_markup(self): """