From 0478d2bee6078859d5b96e1076de0608fe2459f4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 28 Jan 2025 14:36:40 -0700 Subject: [PATCH] Fix unit tests --- src/registrar/models/domain.py | 1 + src/registrar/models/domain_request.py | 13 ++- .../includes/request_review_steps.html | 2 +- src/registrar/tests/test_models.py | 91 +++++++++++-------- src/registrar/utility/waffle.py | 11 ++- src/registrar/views/domain_request.py | 32 +++++-- 6 files changed, 97 insertions(+), 53 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index cb481db7a..c869dfa67 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -244,6 +244,7 @@ class Domain(TimeStampedModel, DomainHelper): is called in the validate function on the request/domain page throws- RegistryError or InvalidDomainError""" + return True if not cls.string_could_be_domain(domain): logger.warning("Not a valid domain: %s" % str(domain)) # throw invalid domain error so that it can be caught in diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 2d0dd50e5..c56d42c1e 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1300,8 +1300,6 @@ class DomainRequest(TimeStampedModel): .filter(agency=self.federal_agency) .exists() ) - - # NOTE: Shouldn't this be an AND on all required fields? return ( self.federal_agency is not None or self.organization_name is not None @@ -1312,6 +1310,17 @@ class DomainRequest(TimeStampedModel): or self.urbanization is not None ) + def unlock_other_contacts(self) -> bool: + """Unlocks the other contacts step""" + other_contacts_filled_out = self.other_contacts.filter( + first_name__isnull=False, + last_name__isnull=False, + title__isnull=False, + email__isnull=False, + phone__isnull=False, + ).exists() + return (self.has_other_contacts() and other_contacts_filled_out) or self.no_other_contacts_rationale is not None + # ## Form policies ## # # # These methods control what questions need to be answered by applicants diff --git a/src/registrar/templates/includes/request_review_steps.html b/src/registrar/templates/includes/request_review_steps.html index 96ba7f151..f1b13f890 100644 --- a/src/registrar/templates/includes/request_review_steps.html +++ b/src/registrar/templates/includes/request_review_steps.html @@ -116,7 +116,7 @@ {% endif %} {% if step == Step.OTHER_CONTACTS %} - {% if domain_request.other_contacts.all %} + {% if domain_request.unlock_other_contacts %} {% with title=form_titles|get_item:step value=domain_request.other_contacts.all %} {% include "includes/summary_item.html" with title=title value=value heading_level=heading_level editable=is_editable edit_link=domain_request_url contact='true' list='true' %} {% endwith %} diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index d8db0f043..45f8a15f0 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1,9 +1,10 @@ from django.forms import ValidationError from django.test import TestCase from unittest.mock import patch - +from unittest.mock import Mock from django.test import RequestFactory - +from waffle.models import get_waffle_flag_model +from registrar.views.domain_request import DomainRequestWizard from registrar.models import ( Contact, DomainRequest, @@ -1692,11 +1693,20 @@ class TestDomainRequestIncomplete(TestCase): anything_else="Anything else", is_policy_acknowledged=True, creator=self.user, + city="fake", ) - self.domain_request.other_contacts.add(other) self.domain_request.current_websites.add(current) self.domain_request.alternative_domains.add(alt) + self.wizard = DomainRequestWizard() + self.wizard._domain_request = self.domain_request + self.wizard.request = Mock(user=self.user, session={}) + self.wizard.kwargs = {"id": self.domain_request.id} + + # We use both of these flags in the test. In the normal app these are generated normally. + # The alternative syntax is adding the decorator to each test. + get_waffle_flag_model().objects.get_or_create(name="organization_feature") + get_waffle_flag_model().objects.get_or_create(name="organization_requests") def tearDown(self): super().tearDown() @@ -1709,66 +1719,67 @@ class TestDomainRequestIncomplete(TestCase): super().tearDownClass() cls.user.delete() - @less_console_noise_decorator + # @less_console_noise_decorator def test_is_federal_complete(self): - self.assertTrue(self.domain_request._is_federal_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.federal_type = None self.domain_request.save() - self.assertFalse(self.domain_request._is_federal_complete()) + self.domain_request.refresh_from_db() + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_interstate_complete(self): self.domain_request.generic_org_type = DomainRequest.OrganizationChoices.INTERSTATE self.domain_request.about_your_organization = "Something something about your organization" self.domain_request.save() - self.assertTrue(self.domain_request._is_interstate_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.about_your_organization = None self.domain_request.save() - self.assertFalse(self.domain_request._is_interstate_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_state_or_territory_complete(self): self.domain_request.generic_org_type = DomainRequest.OrganizationChoices.STATE_OR_TERRITORY self.domain_request.is_election_board = True self.domain_request.save() - self.assertTrue(self.domain_request._is_state_or_territory_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_state_or_territory_complete()) + self.assertFalse(self.wizard.form_is_complete()) - @less_console_noise_decorator + # @less_console_noise_decorator def test_is_tribal_complete(self): self.domain_request.generic_org_type = DomainRequest.OrganizationChoices.TRIBAL self.domain_request.tribe_name = "Tribe Name" self.domain_request.is_election_board = False self.domain_request.save() - self.assertTrue(self.domain_request._is_tribal_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_tribal_complete()) + self.assertFalse(self.wizard.form_is_complete()) self.domain_request.tribe_name = None self.domain_request.save() - self.assertFalse(self.domain_request._is_tribal_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_county_complete(self): self.domain_request.generic_org_type = DomainRequest.OrganizationChoices.COUNTY self.domain_request.is_election_board = False self.domain_request.save() - self.assertTrue(self.domain_request._is_county_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_county_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_city_complete(self): self.domain_request.generic_org_type = DomainRequest.OrganizationChoices.CITY self.domain_request.is_election_board = False self.domain_request.save() - self.assertTrue(self.domain_request._is_city_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_city_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_special_district_complete(self): @@ -1776,55 +1787,55 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.about_your_organization = "Something something about your organization" self.domain_request.is_election_board = False self.domain_request.save() - self.assertTrue(self.domain_request._is_special_district_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_election_board = None self.domain_request.save() - self.assertFalse(self.domain_request._is_special_district_complete()) + self.assertFalse(self.wizard.form_is_complete()) self.domain_request.about_your_organization = None self.domain_request.save() - self.assertFalse(self.domain_request._is_special_district_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_organization_name_and_address_complete(self): - self.assertTrue(self.domain_request._is_organization_name_and_address_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.organization_name = None self.domain_request.address_line1 = None self.domain_request.save() - self.assertTrue(self.domain_request._is_organization_name_and_address_complete()) + self.assertTrue(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_senior_official_complete(self): - self.assertTrue(self.domain_request._is_senior_official_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.senior_official = None self.domain_request.save() - self.assertFalse(self.domain_request._is_senior_official_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_requested_domain_complete(self): - self.assertTrue(self.domain_request._is_requested_domain_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.requested_domain = None self.domain_request.save() - self.assertFalse(self.domain_request._is_requested_domain_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_purpose_complete(self): - self.assertTrue(self.domain_request._is_purpose_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.purpose = None self.domain_request.save() - self.assertFalse(self.domain_request._is_purpose_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_other_contacts_complete_missing_one_field(self): - self.assertTrue(self.domain_request._is_other_contacts_complete()) + self.assertTrue(self.wizard.form_is_complete()) contact = self.domain_request.other_contacts.first() contact.first_name = None contact.save() - self.assertFalse(self.domain_request._is_other_contacts_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_other_contacts_complete_all_none(self): self.domain_request.other_contacts.clear() - self.assertFalse(self.domain_request._is_other_contacts_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_other_contacts_False_and_has_rationale(self): @@ -1832,7 +1843,7 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.other_contacts.clear() self.domain_request.other_contacts.exists = False self.domain_request.no_other_contacts_rationale = "Some rationale" - self.assertTrue(self.domain_request._is_other_contacts_complete()) + self.assertTrue(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_other_contacts_False_and_NO_rationale(self): @@ -1840,7 +1851,7 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.other_contacts.clear() self.domain_request.other_contacts.exists = False self.domain_request.no_other_contacts_rationale = None - self.assertFalse(self.domain_request._is_other_contacts_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_is_additional_details_complete(self): @@ -2044,28 +2055,28 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.save() self.domain_request.refresh_from_db() self.assertEqual( - self.domain_request._is_additional_details_complete(), + self.wizard.form_is_complete(), case["expected"], msg=f"Failed for case: {case}", ) @less_console_noise_decorator def test_is_policy_acknowledgement_complete(self): - self.assertTrue(self.domain_request._is_policy_acknowledgement_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_policy_acknowledged = False - self.assertTrue(self.domain_request._is_policy_acknowledgement_complete()) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.is_policy_acknowledged = None - self.assertFalse(self.domain_request._is_policy_acknowledgement_complete()) + self.assertFalse(self.wizard.form_is_complete()) @less_console_noise_decorator def test_form_complete(self): request = self.factory.get("/") request.user = self.user - self.assertTrue(self.domain_request._form_complete(request)) + self.assertTrue(self.wizard.form_is_complete()) self.domain_request.generic_org_type = None self.domain_request.save() - self.assertFalse(self.domain_request._form_complete(request)) + self.assertFalse(self.wizard.form_is_complete()) class TestPortfolio(TestCase): diff --git a/src/registrar/utility/waffle.py b/src/registrar/utility/waffle.py index f97a18874..3071fbed9 100644 --- a/src/registrar/utility/waffle.py +++ b/src/registrar/utility/waffle.py @@ -22,7 +22,10 @@ def flag_is_active_anywhere(flag_name): If said flag is enabled for someone, somewhere - return true. Otherwise - return false. """ - flag = get_waffle_flag_model().get(flag_name) - if flag.everyone is None: - return flag.users.exists() - return flag.everyone + try: + flag = get_waffle_flag_model().get(flag_name) + if flag.everyone is None: + return flag.users.exists() + return flag.everyone + except get_waffle_flag_model().DoesNotExist: + return False diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index f200cc3ae..45d802764 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -115,9 +115,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): ), Step.DOTGOV_DOMAIN: lambda self: self.domain_request.requested_domain is not None, Step.PURPOSE: lambda self: self.domain_request.purpose is not None, - Step.OTHER_CONTACTS: lambda self: ( - self.domain_request.other_contacts.exists() or self.domain_request.no_other_contacts_rationale is not None - ), + Step.OTHER_CONTACTS: lambda self: self.from_model("unlock_other_contacts", False), Step.ADDITIONAL_DETAILS: lambda self: ( # Additional details is complete as long as "has anything else" and "has cisa rep" are not None ( @@ -425,16 +423,38 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): """Helper for get_context_data. Queries the DB for a domain request and returns a list of unlocked steps.""" return [key for key, is_unlocked_checker in self.unlocking_steps.items() if is_unlocked_checker(self)] + + def form_is_complete(self): + """ + Determines if all required steps in the domain request form are complete. + + This method: + 1. Gets a list of all steps that have been completed (unlocked_steps) + 2. Filters the full step list to only include steps that should be shown based on + the wizard conditions. For example, federal-specific steps are only required + if the organization type is federal. + 3. Compares the number of completed steps to required steps to determine if + the form is complete. + + Returns: + bool: True if all required steps are complete, False otherwise + """ + unlockable_steps = {step.value for step in self.db_check_for_unlocking_steps()} + required_steps = set(self.steps.all) + unlocked_steps = set() + for step in required_steps: + if step in unlockable_steps: + unlocked_steps.add(step) + return required_steps == unlocked_steps def get_context_data(self): """Define context for access on all wizard pages.""" - requested_domain_name = None if self.domain_request.requested_domain is not None: requested_domain_name = self.domain_request.requested_domain.name context = {} - org_steps_complete = len(self.db_check_for_unlocking_steps()) == len(self.steps) + org_steps_complete = self.form_is_complete() if org_steps_complete: context = { "form_titles": self.titles, @@ -770,7 +790,7 @@ class Review(DomainRequestWizard): forms = [] # type: ignore def get_context_data(self): - form_complete = len(self.db_check_for_unlocking_steps()) == len(self.steps) + form_complete = self.form_is_complete() if form_complete is False: logger.warning("User arrived at review page with an incomplete form.") context = super().get_context_data()