From 913f918787631cd8ee2d02898f47cb7852421ef1 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 4 Jan 2024 20:14:37 -0500 Subject: [PATCH] linter --- src/registrar/forms/application_wizard.py | 67 ++++++++++++---------- src/registrar/models/domain_application.py | 2 +- src/registrar/tests/test_models.py | 28 +++------ src/registrar/tests/test_views.py | 66 ++++++++++----------- src/registrar/views/application.py | 2 +- 5 files changed, 81 insertions(+), 84 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index c8531939f..f1262f3e8 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -7,6 +7,7 @@ from phonenumber_field.formfields import PhoneNumberField # type: ignore from django import forms from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe +from django.db.models.fields.related import ForeignObjectRel from api.views import DOMAIN_API_MESSAGES @@ -94,13 +95,13 @@ class RegistrarFormSet(forms.BaseFormSet): Hint: Subclass should call `self._to_database(...)`. """ raise NotImplementedError - + def test_if_more_than_one_join(self, db_obj, rel, related_name): - """Helper for finding whether an object is joined more than once.""" + """Helper for finding whether an object is joined more than once.""" threshold = 0 if rel == related_name: threshold = 1 - + return getattr(db_obj, rel) is not None and getattr(db_obj, rel).count() > threshold def _to_database( @@ -123,8 +124,15 @@ class RegistrarFormSet(forms.BaseFormSet): obj.save() query = getattr(obj, join).order_by("created_at").all() # order matters - related_name = obj._meta.get_field(join).related_query_name() - + + related_name = "" + field = obj._meta.get_field(join) + + if isinstance(field, ForeignObjectRel) and callable(field.related_query_name): + related_name = field.related_query_name() + elif hasattr(field, "related_query_name") and callable(field.related_query_name): + related_name = field.related_query_name() + # the use of `zip` pairs the forms in the formset with the # related objects gotten from the database -- there should always be # at least as many forms as database entries: extra forms means new @@ -135,7 +143,7 @@ class RegistrarFormSet(forms.BaseFormSet): # matching database object exists, update it if db_obj is not None and cleaned: - if should_delete(cleaned): + if should_delete(cleaned): if any(self.test_if_more_than_one_join(db_obj, rel, related_name) for rel in reverse_joins): # Remove the specific relationship without deleting the object getattr(db_obj, related_name).remove(self.application) @@ -148,7 +156,7 @@ class RegistrarFormSet(forms.BaseFormSet): db_obj.save() # no matching database object, create it - elif db_obj is None and cleaned and not cleaned.get('delete', False): + elif db_obj is None and cleaned and not cleaned.get("delete", False): kwargs = pre_create(db_obj, cleaned) getattr(obj, join).create(**kwargs) @@ -517,7 +525,7 @@ class PurposeForm(RegistrarForm): ], error_messages={"required": "Describe how you'll use the .gov domain you’re requesting."}, ) - + class YourContactForm(RegistrarForm): def to_database(self, obj): @@ -576,16 +584,13 @@ class OtherContactsYesNoForm(RegistrarForm): # No pre-selection for new applications default_value = None - self.fields['has_other_contacts'] = forms.TypedChoiceField( - coerce=lambda x: x.lower() == 'true' if x is not None else None, - choices=( - (True, "Yes, I can name other employees."), - (False, "No (We'll ask you to explain why).") - ), + self.fields["has_other_contacts"] = forms.TypedChoiceField( + coerce=lambda x: x.lower() == "true" if x is not None else None, + choices=((True, "Yes, I can name other employees."), (False, "No (We'll ask you to explain why).")), initial=default_value, - widget=forms.RadioSelect - ) - + widget=forms.RadioSelect, + ) + class OtherContactsForm(RegistrarForm): first_name = forms.CharField( @@ -644,14 +649,22 @@ class OtherContactsForm(RegistrarForm): for field in self.fields: if field in self.errors: del self.errors[field] - return {'delete': True} + return {"delete": True} return self.cleaned_data - + class BaseOtherContactsFormSet(RegistrarFormSet): JOIN = "other_contacts" - REVERSE_JOINS = ["user", "authorizing_official", "submitted_applications", "contact_applications", "information_authorizing_official", "submitted_applications_information", "contact_applications_information"] + REVERSE_JOINS = [ + "user", + "authorizing_official", + "submitted_applications", + "contact_applications", + "information_authorizing_official", + "submitted_applications_information", + "contact_applications_information", + ] def __init__(self, *args, **kwargs): self.formset_data_marked_for_deletion = False @@ -662,12 +675,12 @@ class BaseOtherContactsFormSet(RegistrarFormSet): # in the formset plus those that have data already. for index in range(max(self.initial_form_count(), 1)): self.forms[index].use_required_attribute = True - + def pre_update(self, db_obj, cleaned): """Code to run before an item in the formset is saved.""" for key, value in cleaned.items(): setattr(db_obj, key, value) - + def should_delete(self, cleaned): empty = (isinstance(v, str) and (v.strip() == "" or v is None) for v in cleaned.values()) return all(empty) or self.formset_data_marked_for_deletion @@ -720,11 +733,7 @@ class NoOtherContactsForm(RegistrarForm): message="Response must be less than 1000 characters.", ) ], - error_messages={ - "required": ( - "Rationale for no other employees is required." - ) - }, + error_messages={"required": ("Rationale for no other employees is required.")}, ) def __init__(self, *args, **kwargs): @@ -736,7 +745,7 @@ class NoOtherContactsForm(RegistrarForm): This changes behavior of validity checks and to_database methods.""" self.form_data_marked_for_deletion = True - + def clean(self): """ This method overrides the default behavior for forms. @@ -758,7 +767,7 @@ class NoOtherContactsForm(RegistrarForm): del self.errors[field] return self.cleaned_data - + def to_database(self, obj): """ This method overrides the behavior of RegistrarForm. diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index ec2628bd3..2910b6b4f 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -838,7 +838,7 @@ class DomainApplication(TimeStampedModel): def has_other_contacts(self) -> bool: """Does this application have other contacts listed?""" return self.other_contacts.exists() - + def is_federal(self) -> Union[bool, None]: """Is this application for a federal agency? diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 792d9599a..22202ce5f 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -441,40 +441,28 @@ class TestDomainApplication(TestCase): # Now, when you call is_active on Domain, it will return True with self.assertRaises(TransitionNotAllowed): self.approved_application.reject_with_prejudice() - + def test_has_rationale_returns_true(self): """has_rationale() returns true when an application has no_other_contacts_rationale""" self.started_application.no_other_contacts_rationale = "You talkin' to me?" self.started_application.save() - self.assertEquals( - self.started_application.has_rationale(), - True - ) - + self.assertEquals(self.started_application.has_rationale(), True) + def test_has_rationale_returns_false(self): """has_rationale() returns false when an application has no no_other_contacts_rationale""" - self.assertEquals( - self.started_application.has_rationale(), - False - ) - + self.assertEquals(self.started_application.has_rationale(), False) + def test_has_other_contacts_returns_true(self): """has_other_contacts() returns true when an application has other_contacts""" # completed_application has other contacts by default - self.assertEquals( - self.started_application.has_other_contacts(), - True - ) - + self.assertEquals(self.started_application.has_other_contacts(), True) + def test_has_other_contacts_returns_false(self): """has_other_contacts() returns false when an application has no other_contacts""" application = completed_application( status=DomainApplication.ApplicationStatus.STARTED, name="no-others.gov", has_other_contacts=False ) - self.assertEquals( - application.has_other_contacts(), - False - ) + self.assertEquals(application.has_other_contacts(), False) class TestPermissions(TestCase): diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 028f4c310..0feeb5fdc 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -373,15 +373,15 @@ class DomainApplicationTests(TestWithUser, WebTest): # This page has 3 forms in 1. # Let's set the yes/no radios to enable the other contacts fieldsets other_contacts_form = other_contacts_page.forms[0] - + other_contacts_form["other_contacts-has_other_contacts"] = "True" - + other_contacts_form["other_contacts-0-first_name"] = "Testy2" other_contacts_form["other_contacts-0-last_name"] = "Tester2" other_contacts_form["other_contacts-0-title"] = "Another Tester" other_contacts_form["other_contacts-0-email"] = "testy2@town.com" other_contacts_form["other_contacts-0-phone"] = "(201) 555 5557" - + # test next button self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) other_contacts_result = other_contacts_form.submit() @@ -715,14 +715,14 @@ class DomainApplicationTests(TestWithUser, WebTest): contact_page = type_result.follow() self.assertContains(contact_page, self.TITLES[Step.ABOUT_YOUR_ORGANIZATION]) - + def test_yes_no_form_inits_blank_for_new_application(self): """On the Other Contacts page, the yes/no form gets initialized with nothing selected for new applications""" other_contacts_page = self.app.get(reverse("application:other_contacts")) other_contacts_form = other_contacts_page.forms[0] self.assertEquals(other_contacts_form["other_contacts-has_other_contacts"].value, None) - + def test_yes_no_form_inits_yes_for_application_with_other_contacts(self): """On the Other Contacts page, the yes/no form gets initialized with YES selected if the application has other contacts""" @@ -742,7 +742,7 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] self.assertEquals(other_contacts_form["other_contacts-has_other_contacts"].value, "True") - + def test_yes_no_form_inits_no_for_application_with_no_other_contacts_rationale(self): """On the Other Contacts page, the yes/no form gets initialized with NO selected if the application has no other contacts""" @@ -764,7 +764,7 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] self.assertEquals(other_contacts_form["other_contacts-has_other_contacts"].value, "False") - + def test_submitting_other_contacts_deletes_no_other_contacts_rationale(self): """When a user submits the Other Contacts form with other contacts selected, the application's no other contacts rationale gets deleted""" @@ -786,19 +786,19 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] self.assertEquals(other_contacts_form["other_contacts-has_other_contacts"].value, "False") - + other_contacts_form["other_contacts-has_other_contacts"] = "True" - + other_contacts_form["other_contacts-0-first_name"] = "Testy" other_contacts_form["other_contacts-0-middle_name"] = "" other_contacts_form["other_contacts-0-last_name"] = "McTesterson" other_contacts_form["other_contacts-0-title"] = "Lord" other_contacts_form["other_contacts-0-email"] = "testy@abc.org" other_contacts_form["other_contacts-0-phone"] = "(201) 555-0123" - + # Submit the now empty form other_contacts_form.submit() - + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) # Verify that the no_other_contacts_rationale we saved earlier has been removed from the database @@ -807,12 +807,12 @@ class DomainApplicationTests(TestWithUser, WebTest): application.other_contacts.count(), 1, ) - + self.assertEquals( application.no_other_contacts_rationale, None, ) - + def test_submitting_no_other_contacts_rationale_deletes_other_contacts(self): """When a user submits the Other Contacts form with no other contacts selected, the application's other contacts get deleted for other contacts that exist and are not joined to other objects @@ -833,14 +833,14 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] self.assertEquals(other_contacts_form["other_contacts-has_other_contacts"].value, "True") - + other_contacts_form["other_contacts-has_other_contacts"] = "False" - + other_contacts_form["other_contacts-no_other_contacts_rationale"] = "Hello again!" - + # Submit the now empty form other_contacts_form.submit() - + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) # Verify that the no_other_contacts_rationale we saved earlier has been removed from the database @@ -849,12 +849,12 @@ class DomainApplicationTests(TestWithUser, WebTest): application.other_contacts.count(), 0, ) - + self.assertEquals( application.no_other_contacts_rationale, "Hello again!", ) - + def test_submitting_no_other_contacts_rationale_removes_reference_other_contacts_when_joined(self): """When a user submits the Other Contacts form with no other contacts selected, the application's other contacts references get removed for other contacts that exist and are joined to other objects""" @@ -898,11 +898,11 @@ class DomainApplicationTests(TestWithUser, WebTest): status="started", ) application.other_contacts.add(other) - + # Now let's join the other contact to another object domain_info = DomainInformation.objects.create(creator=self.user) domain_info.other_contacts.set([other]) - + # prime the form by visiting /edit self.app.get(reverse("edit-application", kwargs={"id": application.pk})) # django-webtest does not handle cookie-based sessions well because it keeps @@ -917,14 +917,14 @@ class DomainApplicationTests(TestWithUser, WebTest): other_contacts_form = other_contacts_page.forms[0] self.assertEquals(other_contacts_form["other_contacts-has_other_contacts"].value, "True") - + other_contacts_form["other_contacts-has_other_contacts"] = "False" - + other_contacts_form["other_contacts-no_other_contacts_rationale"] = "Hello again!" - + # Submit the now empty form other_contacts_form.submit() - + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) # Verify that the no_other_contacts_rationale we saved earlier is no longer associated with the application @@ -933,7 +933,7 @@ class DomainApplicationTests(TestWithUser, WebTest): application.other_contacts.count(), 0, ) - + # Verify that the 'other' contact object still exists domain_info = DomainInformation.objects.get() self.assertEqual( @@ -944,27 +944,27 @@ class DomainApplicationTests(TestWithUser, WebTest): domain_info.other_contacts.all()[0].first_name, "Testy2", ) - + self.assertEquals( application.no_other_contacts_rationale, "Hello again!", - ) - + ) + def test_if_yes_no_form_is_no_then_no_other_contacts_required(self): """Applicants with no other contacts have to give a reason.""" other_contacts_page = self.app.get(reverse("application:other_contacts")) other_contacts_form = other_contacts_page.forms[0] other_contacts_form["other_contacts-has_other_contacts"] = "False" response = other_contacts_page.forms[0].submit() - + # The textarea for no other contacts returns this error message # Assert that it is returned, ie the no other contacts form is required self.assertContains(response, "Rationale for no other employees is required.") - + # The first name field for other contacts returns this error message # Assert that it is not returned, ie the contacts form is not required self.assertNotContains(response, "Enter the first name / given name of this contact.") - + def test_if_yes_no_form_is_yes_then_other_contacts_required(self): """Applicants with other contacts do not have to give a reason.""" other_contacts_page = self.app.get(reverse("application:other_contacts")) @@ -975,7 +975,7 @@ class DomainApplicationTests(TestWithUser, WebTest): # The textarea for no other contacts returns this error message # Assert that it is not returned, ie the no other contacts form is not required self.assertNotContains(response, "Rationale for no other employees is required.") - + # The first name field for other contacts returns this error message # Assert that it is returned, ie the contacts form is required self.assertContains(response, "Enter the first name / given name of this contact.") diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index bfff02fff..927342fc5 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -494,7 +494,7 @@ class OtherContacts(ApplicationWizard): # test first for yes_no_form validity if other_contacts_yes_no_form.is_valid(): # test for has_contacts - if other_contacts_yes_no_form.cleaned_data.get('has_other_contacts'): + if other_contacts_yes_no_form.cleaned_data.get("has_other_contacts"): # mark the no_other_contacts_form for deletion no_other_contacts_form.mark_form_for_deletion() # test that the other_contacts_forms and no_other_contacts_forms are valid