mirror of
https://github.com/cisagov/manage.get.gov.git
synced 2025-05-19 02:49:21 +02:00
Add reasons to auditlog
This commit is contained in:
parent
e31c0c9e51
commit
8980d98be9
4 changed files with 141 additions and 93 deletions
|
@ -1812,8 +1812,70 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin):
|
||||||
return response
|
return response
|
||||||
|
|
||||||
def change_view(self, request, object_id, form_url="", extra_context=None):
|
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)
|
obj = self.get_object(request, object_id)
|
||||||
self.display_restricted_warning(request, obj)
|
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)
|
return super().change_view(request, object_id, form_url, extra_context)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -17,8 +17,6 @@ from .utility.time_stamped_model import TimeStampedModel
|
||||||
from ..utility.email import send_templated_email, EmailSendingError
|
from ..utility.email import send_templated_email, EmailSendingError
|
||||||
from itertools import chain
|
from itertools import chain
|
||||||
|
|
||||||
from auditlog.models import AuditlogHistoryField # type: ignore
|
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
@ -35,11 +33,7 @@ class DomainRequest(TimeStampedModel):
|
||||||
]
|
]
|
||||||
|
|
||||||
# https://django-auditlog.readthedocs.io/en/latest/usage.html#object-history
|
# https://django-auditlog.readthedocs.io/en/latest/usage.html#object-history
|
||||||
# If we note any performace degradation due to this addition,
|
# history = AuditlogHistoryField()
|
||||||
# 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()
|
|
||||||
|
|
||||||
# Constants for choice fields
|
# Constants for choice fields
|
||||||
class DomainRequestStatus(models.TextChoices):
|
class DomainRequestStatus(models.TextChoices):
|
||||||
|
|
|
@ -68,7 +68,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
|
||||||
{% endblock field_readonly %}
|
{% endblock field_readonly %}
|
||||||
|
|
||||||
{% block after_help_text %}
|
{% block after_help_text %}
|
||||||
{% if field.field.name == "status" and original_object.history.count > 0 %}
|
{% if field.field.name == "status" and filtered_entries %}
|
||||||
<div class="flex-container" id="dja-status-changelog">
|
<div class="flex-container" id="dja-status-changelog">
|
||||||
<label aria-label="Status changelog"></label>
|
<label aria-label="Status changelog"></label>
|
||||||
<div>
|
<div>
|
||||||
|
@ -82,25 +82,26 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
|
||||||
</tr>
|
</tr>
|
||||||
</thead>
|
</thead>
|
||||||
<tbody>
|
<tbody>
|
||||||
{% for log_entry in original_object.history.all %}
|
{% for entry in filtered_entries %}
|
||||||
{% for key, value in log_entry.changes_display_dict.items %}
|
|
||||||
{% if key == "status" %}
|
|
||||||
<tr>
|
<tr>
|
||||||
<td>{{ value.1|default:"None" }}
|
|
||||||
{% for rk, rv in log_entry.changes_display_dict.items %}
|
|
||||||
{% if rk == "rejection reason" %}
|
|
||||||
- {{ rv.1|default:"None" }}
|
|
||||||
{% endif %}
|
|
||||||
{% endfor %}
|
|
||||||
</td>
|
|
||||||
<td>
|
<td>
|
||||||
{{ log_entry.actor|default:"None" }}
|
{% if entry.status %}
|
||||||
|
{{ entry.status|default:"None" }}
|
||||||
</td>
|
{% else %}
|
||||||
<td>{{ log_entry.timestamp|date:"Y-m-d H:i:s" }}</td>
|
Error
|
||||||
</tr>
|
|
||||||
{% endif %}
|
{% endif %}
|
||||||
{% endfor %}
|
|
||||||
|
{% if entry.rejection_reason %}
|
||||||
|
- {{ entry.rejection_reason|default:"None" }}
|
||||||
|
{% endif %}
|
||||||
|
|
||||||
|
{% if entry.action_needed_reason %}
|
||||||
|
- {{ entry.action_needed_reason|default:"None" }}
|
||||||
|
{% endif %}
|
||||||
|
</td>
|
||||||
|
<td>{{ entry.actor|default:"None" }}</td>
|
||||||
|
<td>{{ entry.timestamp|date:"Y-m-d H:i:s" }}</td>
|
||||||
|
</tr>
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
</tbody>
|
</tbody>
|
||||||
</table>
|
</table>
|
||||||
|
|
|
@ -1055,6 +1055,18 @@ class TestDomainRequestAdmin(MockEppLib):
|
||||||
accurately and in chronological order.
|
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"<td> {status} </td>"), 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"<td> {status} </td>", 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
|
# Create a fake domain request and domain
|
||||||
domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.STARTED)
|
domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.STARTED)
|
||||||
|
|
||||||
|
@ -1069,48 +1081,23 @@ class TestDomainRequestAdmin(MockEppLib):
|
||||||
self.assertEqual(response.status_code, 200)
|
self.assertEqual(response.status_code, 200)
|
||||||
self.assertContains(response, domain_request.requested_domain.name)
|
self.assertContains(response, domain_request.requested_domain.name)
|
||||||
|
|
||||||
# Table will contain one row for Started
|
|
||||||
self.assertContains(response, "<td>Started</td>", count=1)
|
|
||||||
self.assertNotContains(response, "<td>Submitted</td>")
|
|
||||||
|
|
||||||
domain_request.submit()
|
domain_request.submit()
|
||||||
domain_request.save()
|
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, "<td>Started</td>", count=1)
|
|
||||||
self.assertContains(response, "<td>Submitted</td>", count=1)
|
|
||||||
|
|
||||||
domain_request.in_review()
|
domain_request.in_review()
|
||||||
domain_request.save()
|
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, "<td>Started</td>", count=1)
|
|
||||||
self.assertContains(response, "<td>Submitted</td>", count=1)
|
|
||||||
self.assertContains(response, "<td>In review</td>", count=1)
|
|
||||||
|
|
||||||
domain_request.action_needed()
|
domain_request.action_needed()
|
||||||
|
domain_request.action_needed_reason = DomainRequest.ActionNeededReasons.ALREADY_HAS_DOMAINS
|
||||||
domain_request.save()
|
domain_request.save()
|
||||||
|
|
||||||
response = self.client.get(
|
# Let's just change the action needed reason
|
||||||
"/admin/registrar/domainrequest/{}/change/".format(domain_request.pk),
|
domain_request.action_needed_reason = DomainRequest.ActionNeededReasons.ELIGIBILITY_UNCLEAR
|
||||||
follow=True,
|
domain_request.save()
|
||||||
)
|
|
||||||
|
|
||||||
# Table will contain and extra row for Action needed
|
domain_request.reject()
|
||||||
self.assertContains(response, "<td>Started</td>", count=1)
|
domain_request.rejection_reason = DomainRequest.RejectionReasons.DOMAIN_PURPOSE
|
||||||
self.assertContains(response, "<td>Submitted</td>", count=1)
|
domain_request.save()
|
||||||
self.assertContains(response, "<td>In review</td>", count=1)
|
|
||||||
self.assertContains(response, "<td>Action needed</td>", count=1)
|
|
||||||
|
|
||||||
domain_request.in_review()
|
domain_request.in_review()
|
||||||
domain_request.save()
|
domain_request.save()
|
||||||
|
@ -1120,24 +1107,28 @@ class TestDomainRequestAdmin(MockEppLib):
|
||||||
follow=True,
|
follow=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Normalize the HTML response content
|
||||||
|
normalized_content = " ".join(response.content.decode("utf-8").split())
|
||||||
|
|
||||||
# Define the expected sequence of status changes
|
# Define the expected sequence of status changes
|
||||||
expected_status_changes = [
|
expected_status_changes = [
|
||||||
"<td>In review</td>",
|
"In review",
|
||||||
"<td>Action needed</td>",
|
"Rejected - Purpose requirements not met",
|
||||||
"<td>In review</td>",
|
"Action needed - Unclear organization eligibility",
|
||||||
"<td>Submitted</td>",
|
"Action needed - Already has domains",
|
||||||
"<td>Started</td>",
|
"In review",
|
||||||
|
"Submitted",
|
||||||
|
"Started",
|
||||||
]
|
]
|
||||||
|
|
||||||
# Test for the order of status changes
|
assert_status_order(normalized_content, expected_status_changes)
|
||||||
for status_change in expected_status_changes:
|
|
||||||
self.assertContains(response, status_change, html=True)
|
|
||||||
|
|
||||||
# Table now contains 2 rows for Approved
|
assert_status_count(normalized_content, "Started", 1)
|
||||||
self.assertContains(response, "<td>Started</td>", count=1)
|
assert_status_count(normalized_content, "Submitted", 1)
|
||||||
self.assertContains(response, "<td>Submitted</td>", count=1)
|
assert_status_count(normalized_content, "In review", 2)
|
||||||
self.assertContains(response, "<td>In review</td>", count=2)
|
assert_status_count(normalized_content, "Action needed - Already has domains", 1)
|
||||||
self.assertContains(response, "<td>Action needed</td>", count=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):
|
def test_collaspe_toggle_button_markup(self):
|
||||||
"""
|
"""
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue