From a592b5ff718167812a84b8415ec3032752f85729 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 11 Sep 2023 14:49:52 -0400 Subject: [PATCH 1/6] added form for DomainApplicationAdmin which limits the list of statuses to those which are available to transition to --- src/registrar/admin.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d6109a0cc..82984b262 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,4 +1,6 @@ import logging +from django import forms +from django_fsm import get_available_FIELD_transitions from django.contrib import admin, messages from django.contrib.auth.admin import UserAdmin as BaseUserAdmin from django.contrib.contenttypes.models import ContentType @@ -224,6 +226,29 @@ class ContactAdmin(ListHeaderAdmin): search_help_text = "Search by firstname, lastname or email." +class DomainApplicationAdminForm(forms.ModelForm): + """Custom form to limit transitions to available transitions""" + class Meta: + model = models.DomainApplication + fields = '__all__' + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + instance = kwargs.get('instance') + if instance and instance.pk: + current_state = instance.status + transitions = get_available_FIELD_transitions( + instance, models.DomainApplication._meta.get_field('status')) + # first option in status transitions is current state + available_transitions = [(current_state, current_state)] + + for transition in transitions: + available_transitions.append((transition.target, transition.target)) + + self.fields['status'].widget.choices = available_transitions + + class DomainApplicationAdmin(ListHeaderAdmin): """Customize the applications listing view.""" @@ -255,6 +280,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): search_help_text = "Search by domain or submitter." # Detail view + form = DomainApplicationAdminForm fieldsets = [ (None, {"fields": ["status", "investigator", "creator"]}), ( From 924651b0881791f376eaefb978ed281ea81a70b7 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 11 Sep 2023 14:54:37 -0400 Subject: [PATCH 2/6] reformatting - linting --- src/registrar/admin.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 82984b262..65b5d123a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -228,25 +228,27 @@ class ContactAdmin(ListHeaderAdmin): class DomainApplicationAdminForm(forms.ModelForm): """Custom form to limit transitions to available transitions""" + class Meta: model = models.DomainApplication - fields = '__all__' + fields = "__all__" def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - instance = kwargs.get('instance') + instance = kwargs.get("instance") if instance and instance.pk: current_state = instance.status transitions = get_available_FIELD_transitions( - instance, models.DomainApplication._meta.get_field('status')) + instance, models.DomainApplication._meta.get_field("status") + ) # first option in status transitions is current state available_transitions = [(current_state, current_state)] for transition in transitions: available_transitions.append((transition.target, transition.target)) - self.fields['status'].widget.choices = available_transitions + self.fields["status"].widget.choices = available_transitions class DomainApplicationAdmin(ListHeaderAdmin): From 3752148630c549d6ae605f650dcc2ffa46333554 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 11 Sep 2023 15:46:03 -0400 Subject: [PATCH 3/6] added test for DomainApplicationAdminForm --- src/registrar/tests/test_admin.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 8cd1988f2..22fd018e8 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,10 +1,12 @@ from django.test import TestCase, RequestFactory, Client +from django_fsm import transition from django.contrib.admin.sites import AdminSite from django.urls import reverse from registrar.admin import ( DomainAdmin, DomainApplicationAdmin, + DomainApplicationAdminForm, ListHeaderAdmin, MyUserAdmin, AuditedAdmin, @@ -36,6 +38,20 @@ logger = logging.getLogger(__name__) +class TestDomainApplicationAdminForm(TestCase): + def setUp(self): + # Create a test application with an initial state of started + self.application = completed_application() + + def test_form_choices(self): + # Create a form instance with the test application + form = DomainApplicationAdminForm(instance=self.application) + + # Verify that the form choices match the available transitions for the initial state + expected_choices = [('started', 'started'), ('submitted', 'submitted')] + self.assertEqual(form.fields['status'].widget.choices, expected_choices) + + class TestDomainApplicationAdmin(TestCase): def setUp(self): self.site = AdminSite() From 1520e80adc524ca18caedee2934d4e69a55327de Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 11 Sep 2023 16:10:59 -0400 Subject: [PATCH 4/6] reformatted code to satisfy linter --- src/registrar/tests/test_admin.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 22fd018e8..1d7915bf3 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,5 +1,4 @@ from django.test import TestCase, RequestFactory, Client -from django_fsm import transition from django.contrib.admin.sites import AdminSite from django.urls import reverse @@ -47,9 +46,9 @@ def test_form_choices(self): # Create a form instance with the test application form = DomainApplicationAdminForm(instance=self.application) - # Verify that the form choices match the available transitions for the initial state - expected_choices = [('started', 'started'), ('submitted', 'submitted')] - self.assertEqual(form.fields['status'].widget.choices, expected_choices) + # Verify that the form choices match the available transitions for started + expected_choices = [("started", "started"), ("submitted", "submitted")] + self.assertEqual(form.fields["status"].widget.choices, expected_choices) class TestDomainApplicationAdmin(TestCase): From 31d04c026971fb64fbe29fb37b9d62155f0859e4 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 12 Sep 2023 14:47:22 -0400 Subject: [PATCH 5/6] added test for case where no status --- src/registrar/admin.py | 6 ++++-- src/registrar/tests/test_admin.py | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a373757cd..d8fc4a232 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -267,11 +267,13 @@ def __init__(self, *args, **kwargs): instance = kwargs.get("instance") if instance and instance.pk: current_state = instance.status + + # first option in status transitions is current state + available_transitions = [(current_state, current_state)] + transitions = get_available_FIELD_transitions( instance, models.DomainApplication._meta.get_field("status") ) - # first option in status transitions is current state - available_transitions = [(current_state, current_state)] for transition in transitions: available_transitions.append((transition.target, transition.target)) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f2225d877..78d358034 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -106,6 +106,20 @@ def test_form_choices(self): expected_choices = [("started", "started"), ("submitted", "submitted")] self.assertEqual(form.fields["status"].widget.choices, expected_choices) + def test_form_choices_when_no_instance(self): + # Create a form instance without an instance + form = DomainApplicationAdminForm() + + # Verify that the form choices show all choices when no instance is provided; + # this is necessary to show all choices when creating a new domain + # application in django admin; + # note that FSM ensures that no domain application exists with invalid status, + # so don't need to test for invalid status + self.assertEqual( + form.fields["status"].widget.choices, + DomainApplication._meta.get_field("status").choices, + ) + class TestDomainApplicationAdmin(TestCase): def setUp(self): From af5ca2afdf2d3fd9139fba9d195cf1a40c1d6e3f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 13 Sep 2023 08:22:10 -0400 Subject: [PATCH 6/6] updated code for readability; handled ineligible status; tested for restricted user --- src/registrar/admin.py | 14 +++++++++----- src/registrar/tests/test_admin.py | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d8fc4a232..7e1c59609 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -264,21 +264,25 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - instance = kwargs.get("instance") - if instance and instance.pk: - current_state = instance.status + application = kwargs.get("instance") + if application and application.pk: + current_state = application.status # first option in status transitions is current state available_transitions = [(current_state, current_state)] transitions = get_available_FIELD_transitions( - instance, models.DomainApplication._meta.get_field("status") + application, models.DomainApplication._meta.get_field("status") ) for transition in transitions: available_transitions.append((transition.target, transition.target)) - self.fields["status"].widget.choices = available_transitions + # only set the available transitions if the user is not restricted + # from editing the domain application; otherwise, the form will be + # readonly and the status field will not have a widget + if not application.creator.is_restricted(): + self.fields["status"].widget.choices = available_transitions class DomainApplicationAdmin(ListHeaderAdmin): diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 78d358034..f4e5ec862 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -120,6 +120,20 @@ def test_form_choices_when_no_instance(self): DomainApplication._meta.get_field("status").choices, ) + def test_form_choices_when_ineligible(self): + # Create a form instance with a domain application with ineligible status + ineligible_application = DomainApplication(status="ineligible") + + # Attempt to create a form with the ineligible application + # The form should not raise an error, but choices should be the + # full list of possible choices + form = DomainApplicationAdminForm(instance=ineligible_application) + + self.assertEqual( + form.fields["status"].widget.choices, + DomainApplication._meta.get_field("status").choices, + ) + class TestDomainApplicationAdmin(TestCase): def setUp(self):