diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ccdb7be3e..eeb9a9375 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4,43 +4,12 @@ from django.contrib.auth.admin import UserAdmin as BaseUserAdmin from django.contrib.contenttypes.models import ContentType from django.http.response import HttpResponseRedirect from django.urls import reverse -# from django.forms.widgets import CheckboxSelectMultiple -# from django import forms + from . import models logger = logging.getLogger(__name__) -# class TextDisplayWidget(forms.Widget): -# def render(self, name, value, attrs=None, renderer=None): -# if value: -# choices_dict = dict(self.choices) -# selected_values = set(value) -# display_values = [choices_dict[val] for val in selected_values] -# return ', '.join(display_values) -# return '' - - -# class DomainApplicationForm(forms.ModelForm): -# class Meta: -# model = models.DomainApplication -# fields = '__all__' - -# def __init__(self, *args, **kwargs): -# super().__init__(*args, **kwargs) - -# # Replace the current_websites field with text if a condition is met -# if self.instance and self.instance.creator.status == 'ineligible': -# self.fields['current_websites'].widget = TextDisplayWidget() -# self.fields['current_websites'].widget.choices = self.fields['current_websites'].choices - -# self.fields['other_contacts'].widget = TextDisplayWidget() -# self.fields['other_contacts'].widget.choices = self.fields['other_contacts'].choices - -# self.fields['alternative_domains'].widget = TextDisplayWidget() -# self.fields['alternative_domains'].widget.choices = self.fields['alternative_domains'].choices - - class AuditedAdmin(admin.ModelAdmin): """Custom admin to make auditing easier.""" @@ -130,14 +99,35 @@ class MyUserAdmin(BaseUserAdmin): """Custom user admin class to use our inlines.""" inlines = [UserContactInline] - - list_display = ("email", "first_name", "last_name", "is_staff", "is_superuser", "status") - + + list_display = ( + "email", + "first_name", + "last_name", + "is_staff", + "is_superuser", + "status", + ) + fieldsets = ( - (None, {'fields': ('username', 'password', 'status')}), # Add the 'status' field here - ('Personal Info', {'fields': ('first_name', 'last_name', 'email')}), - ('Permissions', {'fields': ('is_active', 'is_staff', 'is_superuser', 'groups', 'user_permissions')}), - ('Important dates', {'fields': ('last_login', 'date_joined')}), + ( + None, + {"fields": ("username", "password", "status")}, + ), # Add the 'status' field here + ("Personal Info", {"fields": ("first_name", "last_name", "email")}), + ( + "Permissions", + { + "fields": ( + "is_active", + "is_staff", + "is_superuser", + "groups", + "user_permissions", + ) + }, + ), + ("Important dates", {"fields": ("last_login", "date_joined")}), ) def get_list_display(self, request): @@ -213,10 +203,10 @@ class ContactAdmin(ListHeaderAdmin): class DomainApplicationAdmin(ListHeaderAdmin): """Customize the applications listing view.""" - + # Set multi-selects 'read-only' (hide selects and show data) # based on user perms and application creator's status - #form = DomainApplicationForm + # form = DomainApplicationForm # Columns list_display = [ @@ -314,62 +304,85 @@ class DomainApplicationAdmin(ListHeaderAdmin): # Get the original application from the database original_obj = models.DomainApplication.objects.get(pk=obj.pk) + # if obj.status != original_obj.status: + # if obj.status == models.DomainApplication.STARTED: + # # No conditions + # pass + # elif obj.status == models.DomainApplication.SUBMITTED: + # # This is an fsm in model which will throw an error if the + # # transition condition is violated, so we roll back the + # # status to what it was before the admin user changed it and + # # let the fsm method set it. Same comment applies to + # # transition method calls below. + # obj.status = original_obj.status + # obj.submit() + # elif obj.status == models.DomainApplication.IN_REVIEW: + # obj.status = original_obj.status + # obj.in_review() + # elif obj.status == models.DomainApplication.ACTION_NEEDED: + # obj.status = original_obj.status + # obj.action_needed() + # elif obj.status == models.DomainApplication.APPROVED: + # obj.status = original_obj.status + # obj.approve() + # elif obj.status == models.DomainApplication.WITHDRAWN: + # obj.status = original_obj.status + # obj.withdraw() + # elif obj.status == models.DomainApplication.REJECTED: + # obj.status = original_obj.status + # obj.reject() + # elif obj.status == models.DomainApplication.INELIGIBLE: + # obj.status = original_obj.status + # obj.reject_with_prejudice() + # else: + # logger.warning("Unknown status selected in django admin") + if obj.status != original_obj.status: - if obj.status == models.DomainApplication.STARTED: - # No conditions - pass - elif obj.status == models.DomainApplication.SUBMITTED: - # This is an fsm in model which will throw an error if the - # transition condition is violated, so we roll back the - # status to what it was before the admin user changed it and - # let the fsm method set it. Same comment applies to - # transition method calls below. - obj.status = original_obj.status - obj.submit() - elif obj.status == models.DomainApplication.IN_REVIEW: - obj.status = original_obj.status - obj.in_review() - elif obj.status == models.DomainApplication.ACTION_NEEDED: - obj.status = original_obj.status - obj.action_needed() - elif obj.status == models.DomainApplication.APPROVED: - obj.status = original_obj.status - obj.approve() - elif obj.status == models.DomainApplication.WITHDRAWN: - obj.status = original_obj.status - obj.withdraw() - elif obj.status == models.DomainApplication.REJECTED: - obj.status = original_obj.status - obj.reject() - elif obj.status == models.DomainApplication.INELIGIBLE: - obj.status = original_obj.status - obj.reject_with_prejudice() - else: + status_method_mapping = { + models.DomainApplication.STARTED: None, + models.DomainApplication.SUBMITTED: obj.submit, + models.DomainApplication.IN_REVIEW: obj.in_review, + models.DomainApplication.ACTION_NEEDED: obj.action_needed, + models.DomainApplication.APPROVED: obj.approve, + models.DomainApplication.WITHDRAWN: obj.withdraw, + models.DomainApplication.REJECTED: obj.reject, + models.DomainApplication.INELIGIBLE: obj.reject_with_prejudice, + } + selected_method = status_method_mapping.get(obj.status) + if selected_method is None: logger.warning("Unknown status selected in django admin") + else: + obj.status = original_obj.status + selected_method() super().save_model(request, obj, form, change) - else: - messages.error(request, "This action is not permitted for applications with an ineligible creator.") + else: + messages.error( + request, + "This action is not permitted for applications " + + "with an ineligible creator.", + ) def get_readonly_fields(self, request, obj=None): - - """Set the read-only state on form elements. + """Set the read-only state on form elements. We have 2 conditions that determine which fields are read-only: admin user permissions and the application creator's status, so we'll use the baseline readonly_fields and extend it as needed. """ - + readonly_fields = list(self.readonly_fields) - + # Check if the creator is ineligible if obj and obj.creator.status == "ineligible": - # For fields like CharField, IntegerField, etc., the widget used is straightforward, - # and the readonly_fields list can control their behavior + # For fields like CharField, IntegerField, etc., the widget used is + # straightforward and the readonly_fields list can control their behavior readonly_fields.extend([field.name for field in self.model._meta.fields]) # Add the multi-select fields to readonly_fields: # Complex fields like ManyToManyField require special handling - readonly_fields.extend(["current_websites", "other_contacts", "alternative_domains"]) - + readonly_fields.extend( + ["current_websites", "other_contacts", "alternative_domains"] + ) + if request.user.is_superuser: return readonly_fields else: diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index e020676af..7d017ab38 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -35,7 +35,7 @@ class DomainApplication(TimeStampedModel): (APPROVED, APPROVED), (WITHDRAWN, WITHDRAWN), (REJECTED, REJECTED), - (INELIGIBLE, INELIGIBLE) + (INELIGIBLE, INELIGIBLE), ] class StateTerritoryChoices(models.TextChoices): @@ -556,7 +556,9 @@ class DomainApplication(TimeStampedModel): ) @transition( - field="status", source=[SUBMITTED, IN_REVIEW, REJECTED, INELIGIBLE], target=APPROVED + field="status", + source=[SUBMITTED, IN_REVIEW, REJECTED, INELIGIBLE], + target=APPROVED, ) def approve(self): """Approve an application that has been submitted. @@ -604,7 +606,7 @@ class DomainApplication(TimeStampedModel): "emails/status_change_rejected.txt", "emails/status_change_rejected_subject.txt", ) - + @transition(field="status", source=[IN_REVIEW, APPROVED], target=INELIGIBLE) def reject_with_prejudice(self): """The applicant is a bad actor, reject with prejudice. @@ -613,11 +615,9 @@ class DomainApplication(TimeStampedModel): any existing domains/applications and from submitting new aplications. We do this by setting an ineligible status on the user, which the permissions classes test against""" - + self.creator.block_user() - - # ## Form policies ### # # These methods control what questions need to be answered by applicants diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index b6402a2bf..9b3ba9162 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -16,19 +16,17 @@ class User(AbstractUser): A custom user model that performs identically to the default user model but can be customized later. """ - + # #### Constants for choice fields #### - INELIGIBLE = 'ineligible' - STATUS_CHOICES = ( - (INELIGIBLE, INELIGIBLE), - ) - + INELIGIBLE = "ineligible" + STATUS_CHOICES = ((INELIGIBLE, INELIGIBLE),) + status = models.CharField( max_length=10, choices=STATUS_CHOICES, default=None, # Set the default value to None - null=True, # Allow the field to be null - blank=True, # Allow the field to be blank + null=True, # Allow the field to be null + blank=True, # Allow the field to be blank ) domains = models.ManyToManyField( @@ -52,15 +50,15 @@ class User(AbstractUser): return self.email else: return self.username - + def block_user(self): self.status = "ineligible" self.save() - + def unblock_user(self): self.status = None self.save() - + def is_blocked(self): if self.status == "ineligible": return True diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 6b6be1bd5..7da38cb8a 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -15,7 +15,9 @@ class TestDomainApplicationAdmin(TestCase): def setUp(self): self.site = AdminSite() self.factory = RequestFactory() - self.admin = DomainApplicationAdmin(model=DomainApplication, admin_site=self.site) + self.admin = DomainApplicationAdmin( + model=DomainApplication, admin_site=self.site + ) self.superuser = create_superuser() self.staffuser = create_user() @@ -260,7 +262,7 @@ class TestDomainApplicationAdmin(TestCase): # Perform assertions on the mock call itself mock_client_instance.send_email.assert_called_once() - + def test_save_model_sets_ineligible_status_on_user(self): # make sure there is no user with this email EMAIL = "mayor@igorville.gov" @@ -281,60 +283,110 @@ class TestDomainApplicationAdmin(TestCase): self.admin.save_model(request, application, form=None, change=True) # Test that approved domain exists and equals requested domain - self.assertEqual( - application.creator.status, "ineligible" - ) - + self.assertEqual(application.creator.status, "ineligible") + def test_readonly_when_ineligible_creator(self): application = completed_application(status=DomainApplication.IN_REVIEW) - application.creator.status = 'ineligible' + application.creator.status = "ineligible" application.creator.save() - - request = self.factory.get('/') + + request = self.factory.get("/") request.user = self.superuser - + readonly_fields = self.admin.get_readonly_fields(request, application) - - expected_fields = ['id', 'created_at', 'updated_at', 'status', 'creator', 'investigator', 'organization_type', 'federally_recognized_tribe', 'state_recognized_tribe', 'tribe_name', 'federal_agency', 'federal_type', 'is_election_board', 'organization_name', 'address_line1', 'address_line2', 'city', 'state_territory', 'zipcode', 'urbanization', 'type_of_work', 'more_organization_information', 'authorizing_official', 'approved_domain', 'requested_domain', 'submitter', 'purpose', 'no_other_contacts_rationale', 'anything_else', 'is_policy_acknowledged', 'current_websites', 'other_contacts', 'alternative_domains'] - + + expected_fields = [ + "id", + "created_at", + "updated_at", + "status", + "creator", + "investigator", + "organization_type", + "federally_recognized_tribe", + "state_recognized_tribe", + "tribe_name", + "federal_agency", + "federal_type", + "is_election_board", + "organization_name", + "address_line1", + "address_line2", + "city", + "state_territory", + "zipcode", + "urbanization", + "type_of_work", + "more_organization_information", + "authorizing_official", + "approved_domain", + "requested_domain", + "submitter", + "purpose", + "no_other_contacts_rationale", + "anything_else", + "is_policy_acknowledged", + "current_websites", + "other_contacts", + "alternative_domains", + ] + self.assertEqual(readonly_fields, expected_fields) - + def test_readonly_fields_for_analyst(self): - request = self.factory.get('/') # Use the correct method and path + request = self.factory.get("/") # Use the correct method and path request.user = self.staffuser - + readonly_fields = self.admin.get_readonly_fields(request) - - expected_fields = ['creator', 'type_of_work', 'more_organization_information', 'address_line1', 'address_line2', 'zipcode', 'requested_domain', 'alternative_domains', 'purpose', 'submitter', 'no_other_contacts_rationale', 'anything_else', 'is_policy_acknowledged'] - + + expected_fields = [ + "creator", + "type_of_work", + "more_organization_information", + "address_line1", + "address_line2", + "zipcode", + "requested_domain", + "alternative_domains", + "purpose", + "submitter", + "no_other_contacts_rationale", + "anything_else", + "is_policy_acknowledged", + ] + self.assertEqual(readonly_fields, expected_fields) - + def test_readonly_fields_for_superuser(self): - request = self.factory.get('/') # Use the correct method and path + request = self.factory.get("/") # Use the correct method and path request.user = self.superuser - + readonly_fields = self.admin.get_readonly_fields(request) - + expected_fields = [] - + self.assertEqual(readonly_fields, expected_fields) - + def test_saving_when_ineligible_creator(self): # Create an instance of the model application = completed_application(status=DomainApplication.IN_REVIEW) - application.creator.status = 'ineligible' + application.creator.status = "ineligible" application.creator.save() - + # Create a request object with a superuser - request = self.factory.get('/') + request = self.factory.get("/") request.user = self.superuser - - with patch('django.contrib.messages.error') as mock_error: + + with patch("django.contrib.messages.error") as mock_error: # Simulate saving the model self.admin.save_model(request, application, None, False) - + # Assert that the error message was called with the correct argument - mock_error.assert_called_once_with(request, "This action is not permitted for applications with an ineligible creator.") + mock_error.assert_called_once_with( + request, + "This action is not permitted for applications with " + + "an ineligible creator.", + ) # Assert that the status has not changed self.assertEqual(application.status, DomainApplication.IN_REVIEW) @@ -418,7 +470,7 @@ class ListHeaderAdminTest(TestCase): DomainInformation.objects.all().delete() DomainApplication.objects.all().delete() User.objects.all().delete() - + class MyUserAdminTest(TestCase): def setUp(self): diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 8274908fa..ca1191061 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -170,7 +170,7 @@ class TestDomainApplication(TestCase): with self.assertRaises(TransitionNotAllowed): application.submit() - + def test_transition_not_allowed_ineligible_submitted(self): """Create an application with status ineligible and call submit against transition rules""" @@ -233,7 +233,7 @@ class TestDomainApplication(TestCase): with self.assertRaises(TransitionNotAllowed): application.in_review() - + def test_transition_not_allowed_ineligible_in_review(self): """Create an application with status ineligible and call in_review against transition rules""" @@ -287,7 +287,7 @@ class TestDomainApplication(TestCase): with self.assertRaises(TransitionNotAllowed): application.action_needed() - + def test_transition_not_allowed_ineligible_action_needed(self): """Create an application with status ineligible and call action_needed against transition rules""" @@ -377,7 +377,7 @@ class TestDomainApplication(TestCase): with self.assertRaises(TransitionNotAllowed): application.withdraw() - + def test_transition_not_allowed_ineligible_withdrawn(self): """Create an application with status ineligible and call withdraw against transition rules""" @@ -431,7 +431,7 @@ class TestDomainApplication(TestCase): with self.assertRaises(TransitionNotAllowed): application.reject() - + def test_transition_not_allowed_ineligible_rejected(self): """Create an application with status ineligible and call reject against transition rules""" @@ -440,7 +440,7 @@ class TestDomainApplication(TestCase): with self.assertRaises(TransitionNotAllowed): application.reject_with_prejudice() - + def test_transition_not_allowed_started_ineligible(self): """Create an application with status started and call reject against transition rules""" @@ -485,7 +485,7 @@ class TestDomainApplication(TestCase): with self.assertRaises(TransitionNotAllowed): application.reject_with_prejudice() - + def test_transition_not_allowed_ineligible_ineligible(self): """Create an application with status ineligible and call reject against transition rules""" diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index eccf769ea..0d1239cf3 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -100,14 +100,14 @@ class LoggedInTests(TestWithUser): response, "What kind of U.S.-based government organization do you represent?", ) - + def test_domain_application_form_with_ineligible_user(self): """Application form not accessible for an ineligible user. This test should be solid enough since all application wizard views share the same permissions class""" self.user.status = "ineligible" self.user.save() - + with less_console_noise(): response = self.client.get("/register/", follow=True) print(response.status_code) @@ -1434,7 +1434,7 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest): self.assertContains( success_page, "The security email for this domain have been updated" ) - + def test_domain_overview_blocked_for_ineligible_user(self): """We could easily duplicate this test for all domain management views, but a single url test should be solid enough since all domain @@ -1443,10 +1443,8 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest): self.user.save() home_page = self.app.get("/") self.assertContains(home_page, "igorville.gov") - with less_console_noise(): - response = self.client.get( - reverse("domain", kwargs={"pk": self.domain.id}) - ) + with less_console_noise(): + response = self.client.get(reverse("domain", kwargs={"pk": self.domain.id})) self.assertEqual(response.status_code, 403) @@ -1472,13 +1470,13 @@ class TestApplicationStatus(TestWithUser, WebTest): self.assertContains(detail_page, "testy@town.com") self.assertContains(detail_page, "Admin Tester") self.assertContains(detail_page, "Status:") - + def test_application_status_with_ineligible_user(self): """Checking application status page whith a blocked user. The user should still have access to view.""" self.user.status = "ineligible" self.user.save() - + application = completed_application( status=DomainApplication.SUBMITTED, user=self.user ) diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index fc9443847..23d7348e9 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -59,7 +59,7 @@ class ApplicationWizard(ApplicationWizardPermissionView, TemplateView): Any method not marked as internal can be overridden in a subclass, although not without consulting the base implementation, first. """ - + template_name = "" # uniquely namespace the wizard in urls.py diff --git a/src/registrar/views/utility/__init__.py b/src/registrar/views/utility/__init__.py index b782f59fd..71d3edb91 100644 --- a/src/registrar/views/utility/__init__.py +++ b/src/registrar/views/utility/__init__.py @@ -5,5 +5,5 @@ from .permission_views import ( DomainPermissionView, DomainApplicationPermissionView, DomainInvitationPermissionDeleteView, - ApplicationWizardPermissionView + ApplicationWizardPermissionView, ) diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 715514bd8..e85348d6c 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -69,7 +69,7 @@ class DomainApplicationPermission(PermissionsLoginMixin): return False return True - + class ApplicationWizardPermission(PermissionsLoginMixin): @@ -80,7 +80,7 @@ class ApplicationWizardPermission(PermissionsLoginMixin): The user is in self.request.user """ - + # The user has an ineligible flag if self.request.user.is_blocked(): return False diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index d7b846377..0ef4ff4e5 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -10,7 +10,7 @@ from .mixins import ( DomainPermission, DomainApplicationPermission, DomainInvitationPermission, - ApplicationWizardPermission + ApplicationWizardPermission, ) @@ -52,9 +52,11 @@ class DomainApplicationPermissionView(DomainApplicationPermission, DetailView, a @abc.abstractmethod def template_name(self): raise NotImplementedError - - -class ApplicationWizardPermissionView(ApplicationWizardPermission, TemplateView, abc.ABC): + + +class ApplicationWizardPermissionView( + ApplicationWizardPermission, TemplateView, abc.ABC +): """Abstract base view for the application form that enforces permissions