From a6de5293c2fdb51971e87f6ac7bc89b36061dbaf Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 23 Jan 2025 15:15:24 -0700 Subject: [PATCH 01/15] Core logic --- src/registrar/forms/domain_request_wizard.py | 20 ++++++++++++++++++-- src/registrar/utility/waffle.py | 16 ++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 7c9dcb180..c1ea2dda1 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -2,7 +2,8 @@ import logging from api.views import DOMAIN_API_MESSAGES from phonenumber_field.formfields import PhoneNumberField # type: ignore - +from registrar.models.portfolio import Portfolio +from registrar.utility.waffle import flag_is_active_anywhere from django import forms from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe @@ -321,7 +322,7 @@ class OrganizationContactForm(RegistrarForm): # if it has been filled in when required. # uncomment to see if modelChoiceField can be an arg later required=False, - queryset=FederalAgency.objects.exclude(agency__in=excluded_agencies), + queryset=FederalAgency.objects.none(), widget=ComboboxWidget, ) organization_name = forms.CharField( @@ -363,6 +364,21 @@ class OrganizationContactForm(RegistrarForm): label="Urbanization (required for Puerto Rico only)", ) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + # Set the queryset for federal agency. + # If the organization_requests flag is active, we hide data that exists in portfolios. + # NOTE: This function assumes that the federal_agency field was first set to None if a portfolio exists. + federal_agency_queryset = FederalAgency.objects.exclude(agency__in=self.excluded_agencies) + if flag_is_active_anywhere("organization_feature") and flag_is_active_anywhere("organization_requests"): + # Exclude both predefined agencies and those matching portfolio names in one query + federal_agency_queryset = federal_agency_queryset.exclude( + agency__in=Portfolio.objects.values_list("organization_name", flat=True) + ) + + self.fields["federal_agency"].queryset = federal_agency_queryset + def clean_federal_agency(self): """Require something to be selected when this is a federal agency.""" federal_agency = self.cleaned_data.get("federal_agency", None) diff --git a/src/registrar/utility/waffle.py b/src/registrar/utility/waffle.py index a78799e4c..f97a18874 100644 --- a/src/registrar/utility/waffle.py +++ b/src/registrar/utility/waffle.py @@ -1,5 +1,6 @@ from django.http import HttpRequest from waffle.decorators import flag_is_active +from waffle.models import get_waffle_flag_model def flag_is_active_for_user(user, flag_name): @@ -10,3 +11,18 @@ def flag_is_active_for_user(user, flag_name): request = HttpRequest() request.user = user return flag_is_active(request, flag_name) + + +def flag_is_active_anywhere(flag_name): + """Checks if the given flag name is active for anyone, anywhere. + More specifically, it checks on flag.everyone or flag.users.exists(). + Does not check self.superuser, self.staff or self.group. + + This function effectively behaves like a switch: + 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 From a4f1abc0011556e8557a17909edeb58e1e3cd693 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 24 Jan 2025 08:12:10 -0700 Subject: [PATCH 02/15] add check --- src/registrar/models/domain_request.py | 26 ++++++++++++++++++++++++-- src/registrar/views/domain_request.py | 10 +--------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 3d3aac769..49fddbe18 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -14,8 +14,7 @@ from auditlog.models import LogEntry from django.core.exceptions import ValidationError -from registrar.utility.waffle import flag_is_active_for_user - +from registrar.utility.waffle import flag_is_active_for_user, flag_is_active_anywhere from .utility.time_stamped_model import TimeStampedModel from ..utility.email import send_templated_email, EmailSendingError from itertools import chain @@ -1289,6 +1288,29 @@ def unlock_requesting_entity(self) -> bool: return True return False + def unlock_organization_contact(self) -> bool: + """Unlocks the organization_contact step.""" + + # NOTE: This check may struggle with a high number of portfolios, consider something else then. + if flag_is_active_anywhere("organization_feature") and flag_is_active_anywhere("organization_requests"): + # Check if the current federal agency is an outlawed one + Portfolio = apps.get_model("registrar.Portfolio") + return FederalAgency.objects.exclude( + agency__in=Portfolio.objects.values_list("organization_name", flat=True), + ).filter(agency=self.federal_agency).exists() + + # NOTE: Shouldn't this be an AND on all required fields? + return ( + self.domain_request.federal_agency is not None + or self.domain_request.organization_name is not None + or self.domain_request.address_line1 is not None + or self.domain_request.city is not None + or self.domain_request.state_territory is not None + or self.domain_request.zipcode is not None + or self.domain_request.urbanization is not None + ) + + # ## Form policies ## # # # These methods control what questions need to be answered by applicants diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index 9754b0d0c..b530dc7ae 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -107,15 +107,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): Step.TRIBAL_GOVERNMENT: lambda self: self.domain_request.tribe_name is not None, Step.ORGANIZATION_FEDERAL: lambda self: self.domain_request.federal_type is not None, Step.ORGANIZATION_ELECTION: lambda self: self.domain_request.is_election_board is not None, - Step.ORGANIZATION_CONTACT: lambda self: ( - self.domain_request.federal_agency is not None - or self.domain_request.organization_name is not None - or self.domain_request.address_line1 is not None - or self.domain_request.city is not None - or self.domain_request.state_territory is not None - or self.domain_request.zipcode is not None - or self.domain_request.urbanization is not None - ), + Step.ORGANIZATION_CONTACT: lambda self: self.from_model("unlock_organization_contact", False), Step.ABOUT_YOUR_ORGANIZATION: lambda self: self.domain_request.about_your_organization is not None, Step.SENIOR_OFFICIAL: lambda self: self.domain_request.senior_official is not None, Step.CURRENT_SITES: lambda self: ( From 297029a447504749af194fab2065240634f21a6a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 27 Jan 2025 11:14:37 -0700 Subject: [PATCH 03/15] Add UI logic --- src/registrar/models/domain.py | 1 + src/registrar/models/domain_request.py | 151 +----------------- .../includes/request_review_steps.html | 2 +- src/registrar/views/domain_request.py | 9 +- 4 files changed, 12 insertions(+), 151 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 @@ def available(cls, domain: str) -> bool: 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 49fddbe18..1456742dd 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1290,8 +1290,6 @@ def unlock_requesting_entity(self) -> bool: def unlock_organization_contact(self) -> bool: """Unlocks the organization_contact step.""" - - # NOTE: This check may struggle with a high number of portfolios, consider something else then. if flag_is_active_anywhere("organization_feature") and flag_is_active_anywhere("organization_requests"): # Check if the current federal agency is an outlawed one Portfolio = apps.get_model("registrar.Portfolio") @@ -1301,16 +1299,15 @@ def unlock_organization_contact(self) -> bool: # NOTE: Shouldn't this be an AND on all required fields? return ( - self.domain_request.federal_agency is not None - or self.domain_request.organization_name is not None - or self.domain_request.address_line1 is not None - or self.domain_request.city is not None - or self.domain_request.state_territory is not None - or self.domain_request.zipcode is not None - or self.domain_request.urbanization is not None + self.federal_agency is not None + or self.organization_name is not None + or self.address_line1 is not None + or self.city is not None + or self.state_territory is not None + or self.zipcode is not None + or self.urbanization is not None ) - # ## Form policies ## # # # These methods control what questions need to be answered by applicants @@ -1408,140 +1405,6 @@ def get_formatted_cisa_rep_name(self): names = [n for n in [self.cisa_representative_first_name, self.cisa_representative_last_name] if n] return " ".join(names) if names else "Unknown" - def _is_federal_complete(self): - # Federal -> "Federal government branch" page can't be empty + Federal Agency selection can't be None - return not (self.federal_type is None or self.federal_agency is None) - - def _is_interstate_complete(self): - # Interstate -> "About your organization" page can't be empty - return self.about_your_organization is not None - - def _is_state_or_territory_complete(self): - # State -> ""Election office" page can't be empty - return self.is_election_board is not None - - def _is_tribal_complete(self): - # Tribal -> "Tribal name" and "Election office" page can't be empty - return self.tribe_name is not None and self.is_election_board is not None - - def _is_county_complete(self): - # County -> "Election office" page can't be empty - return self.is_election_board is not None - - def _is_city_complete(self): - # City -> "Election office" page can't be empty - return self.is_election_board is not None - - def _is_special_district_complete(self): - # Special District -> "Election office" and "About your organization" page can't be empty - return self.is_election_board is not None and self.about_your_organization is not None - - # Do we still want to test this after creator is autogenerated? Currently it went back to being selectable - def _is_creator_complete(self): - return self.creator is not None - - def _is_organization_name_and_address_complete(self): - return not ( - self.organization_name is None - and self.address_line1 is None - and self.city is None - and self.state_territory is None - and self.zipcode is None - ) - - def _is_senior_official_complete(self): - return self.senior_official is not None - - def _is_requested_domain_complete(self): - return self.requested_domain is not None - - def _is_purpose_complete(self): - return self.purpose is not None - - def _has_other_contacts_and_filled(self): - # Other Contacts Radio button is Yes and if all required fields are filled - return ( - self.has_other_contacts() - and self.other_contacts.filter( - first_name__isnull=False, - last_name__isnull=False, - title__isnull=False, - email__isnull=False, - phone__isnull=False, - ).exists() - ) - - def _has_no_other_contacts_gives_rationale(self): - # Other Contacts Radio button is No and a rationale is provided - return self.has_other_contacts() is False and self.no_other_contacts_rationale is not None - - def _is_other_contacts_complete(self): - if self._has_other_contacts_and_filled() or self._has_no_other_contacts_gives_rationale(): - return True - return False - - def _cisa_rep_check(self): - # Either does not have a CISA rep, OR has a CISA rep + both first name - # and last name are NOT empty and are NOT an empty string - to_return = ( - self.has_cisa_representative is True - and self.cisa_representative_first_name is not None - and self.cisa_representative_first_name != "" - and self.cisa_representative_last_name is not None - and self.cisa_representative_last_name != "" - ) or self.has_cisa_representative is False - - return to_return - - def _anything_else_radio_button_and_text_field_check(self): - # Anything else boolean is True + filled text field and it's not an empty string OR the boolean is No - return ( - self.has_anything_else_text is True and self.anything_else is not None and self.anything_else != "" - ) or self.has_anything_else_text is False - - def _is_additional_details_complete(self): - return self._cisa_rep_check() and self._anything_else_radio_button_and_text_field_check() - - def _is_policy_acknowledgement_complete(self): - return self.is_policy_acknowledged is not None - - def _is_general_form_complete(self, request): - return ( - self._is_creator_complete() - and self._is_organization_name_and_address_complete() - and self._is_senior_official_complete() - and self._is_requested_domain_complete() - and self._is_purpose_complete() - and self._is_other_contacts_complete() - and self._is_additional_details_complete() - and self._is_policy_acknowledgement_complete() - ) - - def _form_complete(self, request): - match self.generic_org_type: - case DomainRequest.OrganizationChoices.FEDERAL: - is_complete = self._is_federal_complete() - case DomainRequest.OrganizationChoices.INTERSTATE: - is_complete = self._is_interstate_complete() - case DomainRequest.OrganizationChoices.STATE_OR_TERRITORY: - is_complete = self._is_state_or_territory_complete() - case DomainRequest.OrganizationChoices.TRIBAL: - is_complete = self._is_tribal_complete() - case DomainRequest.OrganizationChoices.COUNTY: - is_complete = self._is_county_complete() - case DomainRequest.OrganizationChoices.CITY: - is_complete = self._is_city_complete() - case DomainRequest.OrganizationChoices.SPECIAL_DISTRICT: - is_complete = self._is_special_district_complete() - case DomainRequest.OrganizationChoices.SCHOOL_DISTRICT: - is_complete = True - case _: - # NOTE: Shouldn't happen, this is only if somehow they didn't choose an org type - is_complete = False - if not is_complete or not self._is_general_form_complete(request): - return False - return True - """The following converted_ property methods get field data from this domain request's portfolio, if there is an associated portfolio. If not, they return data from the domain request model.""" diff --git a/src/registrar/templates/includes/request_review_steps.html b/src/registrar/templates/includes/request_review_steps.html index 6151d01a8..96ba7f151 100644 --- a/src/registrar/templates/includes/request_review_steps.html +++ b/src/registrar/templates/includes/request_review_steps.html @@ -41,7 +41,7 @@ {% endif %} {% if step == Step.ORGANIZATION_CONTACT %} - {% if domain_request.organization_name %} + {% if domain_request.unlock_organization_contact %} {% with title=form_titles|get_item:step value=domain_request %} {% include "includes/summary_item.html" with title=title value=value heading_level=heading_level editable=is_editable edit_link=domain_request_url address='true' %} {% endwith %} diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index b530dc7ae..f200cc3ae 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -434,12 +434,8 @@ def get_context_data(self): requested_domain_name = self.domain_request.requested_domain.name context = {} - - # Note: we will want to consolidate the non_org_steps_complete check into the same check that - # org_steps_complete is using at some point. - non_org_steps_complete = DomainRequest._form_complete(self.domain_request, self.request) org_steps_complete = len(self.db_check_for_unlocking_steps()) == len(self.steps) - if (not self.is_portfolio and non_org_steps_complete) or (self.is_portfolio and org_steps_complete): + if org_steps_complete: context = { "form_titles": self.titles, "steps": self.steps, @@ -774,7 +770,8 @@ class Review(DomainRequestWizard): forms = [] # type: ignore def get_context_data(self): - if DomainRequest._form_complete(self.domain_request, self.request) is False: + form_complete = len(self.db_check_for_unlocking_steps()) == len(self.steps) + if form_complete is False: logger.warning("User arrived at review page with an incomplete form.") context = super().get_context_data() context["Step"] = self.get_step_enum().__members__ From 1f9efb7fc0baebd0c1c0d34bf4f835ae4e992a1e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 27 Jan 2025 12:56:08 -0700 Subject: [PATCH 04/15] Add unit test for waffle utility --- src/registrar/forms/domain_request_wizard.py | 2 +- src/registrar/tests/test_utilities.py | 40 +++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 5668b3757..d27ccde89 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -322,6 +322,7 @@ class OrganizationContactForm(RegistrarForm): # if it has been filled in when required. # uncomment to see if modelChoiceField can be an arg later required=False, + # We populate this queryset in init. We want to exclude agencies with a portfolio. queryset=FederalAgency.objects.none(), widget=ComboboxWidget, ) @@ -369,7 +370,6 @@ def __init__(self, *args, **kwargs): # Set the queryset for federal agency. # If the organization_requests flag is active, we hide data that exists in portfolios. - # NOTE: This function assumes that the federal_agency field was first set to None if a portfolio exists. federal_agency_queryset = FederalAgency.objects.exclude(agency__in=self.excluded_agencies) if flag_is_active_anywhere("organization_feature") and flag_is_active_anywhere("organization_requests"): # Exclude both predefined agencies and those matching portfolio names in one query diff --git a/src/registrar/tests/test_utilities.py b/src/registrar/tests/test_utilities.py index 5a2234d66..60b74cd60 100644 --- a/src/registrar/tests/test_utilities.py +++ b/src/registrar/tests/test_utilities.py @@ -1,7 +1,8 @@ from django.test import TestCase from registrar.models import User from waffle.testutils import override_flag -from registrar.utility.waffle import flag_is_active_for_user +from waffle.models import get_waffle_flag_model +from registrar.utility.waffle import flag_is_active_for_user, flag_is_active_anywhere class FlagIsActiveForUserTest(TestCase): @@ -21,3 +22,40 @@ def test_flag_inactive_for_user(self): # Test that the flag is inactive for the user is_active = flag_is_active_for_user(self.user, "test_flag") self.assertFalse(is_active) + + +class TestFlagIsActiveAnywhere(TestCase): + def setUp(self): + self.user = User.objects.create_user(username="testuser") + self.flag_name = "test_flag" + + @override_flag("test_flag", active=True) + def test_flag_active_for_everyone(self): + """Test when flag is active for everyone""" + is_active = flag_is_active_anywhere("test_flag") + self.assertTrue(is_active) + + @override_flag("test_flag", active=False) + def test_flag_inactive_for_everyone(self): + """Test when flag is inactive for everyone""" + is_active = flag_is_active_anywhere("test_flag") + self.assertFalse(is_active) + + def test_flag_active_for_some_users(self): + """Test when flag is active for specific users""" + flag, _ = get_waffle_flag_model().objects.get_or_create(name="test_flag") + flag.everyone = None + flag.save() + flag.users.add(self.user) + + is_active = flag_is_active_anywhere("test_flag") + self.assertTrue(is_active) + + def test_flag_inactive_with_no_users(self): + """Test when flag has no users and everyone is None""" + flag, _ = get_waffle_flag_model().objects.get_or_create(name="test_flag") + flag.everyone = None + flag.save() + + is_active = flag_is_active_anywhere("test_flag") + self.assertFalse(is_active) From bdb3875f1210671b9379ecccfdf0249b645ab382 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 27 Jan 2025 14:07:16 -0700 Subject: [PATCH 05/15] Add tests for unlock_organization_contact --- src/registrar/tests/test_views_request.py | 41 +++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index 81beba604..8e4871f79 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -3221,6 +3221,47 @@ def test_wizard_steps_portfolio(self): federal_agency.delete() domain_request.delete() + @override_flag("organization_feature", active=True) + @override_flag("organization_requests", active=True) + @less_console_noise_decorator + def test_unlock_organization_contact_flags_enabled(self): + """Tests unlock_organization_contact when agency exists in a portfolio""" + # Create a federal agency + federal_agency = FederalAgency.objects.create(agency="Portfolio Agency") + + # Create a portfolio with matching organization name + Portfolio.objects.create( + creator=self.user, + organization_name=federal_agency.agency + ) + + # Create domain request with the portfolio agency + domain_request = completed_domain_request( + federal_agency=federal_agency, + user=self.user + ) + self.assertFalse(domain_request.unlock_organization_contact()) + + @override_flag("organization_feature", active=False) + @override_flag("organization_requests", active=False) + @less_console_noise_decorator + def test_unlock_organization_contact_flags_disabled(self): + """Tests unlock_organization_contact when organization flags are disabled""" + # Create a federal agency + federal_agency = FederalAgency.objects.create(agency="Portfolio Agency") + + # Create a portfolio with matching organization name + Portfolio.objects.create( + creator=self.user, + organization_name=federal_agency.agency + ) + + domain_request = completed_domain_request( + federal_agency=federal_agency, + user=self.user + ) + self.assertTrue(domain_request.unlock_organization_contact()) + class TestPortfolioDomainRequestViewonly(TestWithUser, WebTest): From ad79557a55598cc4f67c33d11b01acf76a7024be Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 28 Jan 2025 09:58:41 -0700 Subject: [PATCH 06/15] lint and fix test --- src/registrar/forms/domain_request_wizard.py | 4 +-- src/registrar/models/domain.py | 1 - src/registrar/models/domain_request.py | 10 ++++--- src/registrar/tests/test_utilities.py | 10 +++---- src/registrar/tests/test_views_request.py | 28 ++++++-------------- 5 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index d27ccde89..0cf82558b 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -367,7 +367,7 @@ class OrganizationContactForm(RegistrarForm): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - + # Set the queryset for federal agency. # If the organization_requests flag is active, we hide data that exists in portfolios. federal_agency_queryset = FederalAgency.objects.exclude(agency__in=self.excluded_agencies) @@ -376,7 +376,7 @@ def __init__(self, *args, **kwargs): federal_agency_queryset = federal_agency_queryset.exclude( agency__in=Portfolio.objects.values_list("organization_name", flat=True) ) - + self.fields["federal_agency"].queryset = federal_agency_queryset def clean_federal_agency(self): diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c869dfa67..cb481db7a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -244,7 +244,6 @@ def available(cls, domain: str) -> bool: 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 1456742dd..2d0dd50e5 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1293,9 +1293,13 @@ def unlock_organization_contact(self) -> bool: if flag_is_active_anywhere("organization_feature") and flag_is_active_anywhere("organization_requests"): # Check if the current federal agency is an outlawed one Portfolio = apps.get_model("registrar.Portfolio") - return FederalAgency.objects.exclude( - agency__in=Portfolio.objects.values_list("organization_name", flat=True), - ).filter(agency=self.federal_agency).exists() + return ( + FederalAgency.objects.exclude( + agency__in=Portfolio.objects.values_list("organization_name", flat=True), + ) + .filter(agency=self.federal_agency) + .exists() + ) # NOTE: Shouldn't this be an AND on all required fields? return ( diff --git a/src/registrar/tests/test_utilities.py b/src/registrar/tests/test_utilities.py index 60b74cd60..d882fdedd 100644 --- a/src/registrar/tests/test_utilities.py +++ b/src/registrar/tests/test_utilities.py @@ -28,29 +28,29 @@ class TestFlagIsActiveAnywhere(TestCase): def setUp(self): self.user = User.objects.create_user(username="testuser") self.flag_name = "test_flag" - + @override_flag("test_flag", active=True) def test_flag_active_for_everyone(self): """Test when flag is active for everyone""" is_active = flag_is_active_anywhere("test_flag") self.assertTrue(is_active) - + @override_flag("test_flag", active=False) def test_flag_inactive_for_everyone(self): """Test when flag is inactive for everyone""" is_active = flag_is_active_anywhere("test_flag") self.assertFalse(is_active) - + def test_flag_active_for_some_users(self): """Test when flag is active for specific users""" flag, _ = get_waffle_flag_model().objects.get_or_create(name="test_flag") flag.everyone = None flag.save() flag.users.add(self.user) - + is_active = flag_is_active_anywhere("test_flag") self.assertTrue(is_active) - + def test_flag_inactive_with_no_users(self): """Test when flag has no users and everyone is None""" flag, _ = get_waffle_flag_model().objects.get_or_create(name="test_flag") diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index 8e4871f79..c559a73c7 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -3228,18 +3228,12 @@ def test_unlock_organization_contact_flags_enabled(self): """Tests unlock_organization_contact when agency exists in a portfolio""" # Create a federal agency federal_agency = FederalAgency.objects.create(agency="Portfolio Agency") - + # Create a portfolio with matching organization name - Portfolio.objects.create( - creator=self.user, - organization_name=federal_agency.agency - ) - + Portfolio.objects.create(creator=self.user, organization_name=federal_agency.agency) + # Create domain request with the portfolio agency - domain_request = completed_domain_request( - federal_agency=federal_agency, - user=self.user - ) + domain_request = completed_domain_request(federal_agency=federal_agency, user=self.user) self.assertFalse(domain_request.unlock_organization_contact()) @override_flag("organization_feature", active=False) @@ -3247,19 +3241,13 @@ def test_unlock_organization_contact_flags_enabled(self): @less_console_noise_decorator def test_unlock_organization_contact_flags_disabled(self): """Tests unlock_organization_contact when organization flags are disabled""" - # Create a federal agency + # Create a federal agency federal_agency = FederalAgency.objects.create(agency="Portfolio Agency") - + # Create a portfolio with matching organization name - Portfolio.objects.create( - creator=self.user, - organization_name=federal_agency.agency - ) + Portfolio.objects.create(creator=self.user, organization_name=federal_agency.agency) - domain_request = completed_domain_request( - federal_agency=federal_agency, - user=self.user - ) + domain_request = completed_domain_request(federal_agency=federal_agency, user=self.user) self.assertTrue(domain_request.unlock_organization_contact()) 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 07/15] 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 @@ def available(cls, domain: str) -> bool: 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 @@ def unlock_organization_contact(self) -> bool: .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 @@ def unlock_organization_contact(self) -> bool: 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 @@

Alternative domains

{% 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 @@ def setUp(self): 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 @@ def tearDownClass(cls): 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 @@ def test_is_special_district_complete(self): 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 @@ def test_is_other_contacts_False_and_has_rationale(self): 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 @@ def test_is_other_contacts_False_and_NO_rationale(self): 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 @@ def test_is_additional_details_complete(self): 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 @@ def db_check_for_unlocking_steps(self): """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() From 4dba0a31c35faf1eab2fdc19c34ca1b372728666 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 28 Jan 2025 15:15:56 -0700 Subject: [PATCH 08/15] remove test code and lint --- src/registrar/models/domain.py | 1 - src/registrar/views/domain_request.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c869dfa67..cb481db7a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -244,7 +244,6 @@ def available(cls, domain: str) -> bool: 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/views/domain_request.py b/src/registrar/views/domain_request.py index 45d802764..e1f94391e 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -423,11 +423,11 @@ def db_check_for_unlocking_steps(self): """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 From b5935a36d690bfda88f7ec277f6e2313b28ca309 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 29 Jan 2025 11:10:12 -0700 Subject: [PATCH 09/15] lint --- src/registrar/tests/test_models.py | 4 ++-- src/registrar/views/domain_request.py | 22 ++++++---------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 45f8a15f0..2a44f7765 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1719,7 +1719,7 @@ def tearDownClass(cls): super().tearDownClass() cls.user.delete() - # @less_console_noise_decorator + @less_console_noise_decorator def test_is_federal_complete(self): self.assertTrue(self.wizard.form_is_complete()) self.domain_request.federal_type = None @@ -1747,7 +1747,7 @@ def test_is_state_or_territory_complete(self): self.domain_request.save() 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" diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index e1f94391e..3248c1368 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -425,26 +425,16 @@ def db_check_for_unlocking_steps(self): 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. - + """Determines if all required steps in the domain request form are 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()} + # 1. Get all steps visibly present to the user (required steps) + # 2. Return every possible step that is "unlocked" (even hidden, conditional ones) + # 3. Narrows down the list to remove hidden conditional steps required_steps = set(self.steps.all) - unlocked_steps = set() - for step in required_steps: - if step in unlockable_steps: - unlocked_steps.add(step) + unlockable_steps = {step.value for step in self.db_check_for_unlocking_steps()} + unlocked_steps = {step for step in required_steps if step in unlockable_steps} return required_steps == unlocked_steps def get_context_data(self): From cc0c53006c6696a119d310bb76bbc762ba2b2e9f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 29 Jan 2025 15:43:53 -0700 Subject: [PATCH 10/15] Update test_forms.py --- src/registrar/tests/test_forms.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index a2960deac..0589d8dec 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -23,6 +23,7 @@ PortfolioMemberForm, PortfolioNewMemberForm, ) +from waffle.models import get_waffle_flag_model from registrar.models.portfolio import Portfolio from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user import User @@ -38,6 +39,10 @@ def setUp(self): self.API_BASE_PATH = "/api/v1/available/?domain=" self.user = get_user_model().objects.create(username="username") self.factory = RequestFactory() + # 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 test_org_contact_zip_invalid(self): form = OrganizationContactForm(data={"zipcode": "nah"}) From 59d1301fd72be651e49b0a39ad7d08b133783ee5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 30 Jan 2025 08:40:24 -0700 Subject: [PATCH 11/15] Update test_views_request.py --- src/registrar/tests/test_views_request.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index c559a73c7..9638ff669 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -3081,17 +3081,26 @@ def test_unlocked_steps_partial_domain_request(self): contact = Contact.objects.create( first_name="Henry", last_name="Mcfakerson", + title="test", + email="moar@igorville.gov", + phone="1234567890" ) # Create two non-orphaned contacts contact_2 = Contact.objects.create( first_name="Saturn", last_name="Mars", + title="test", + email="moar@igorville.gov", + phone="1234567890" ) # Attach a user object to a contact (should not be deleted) contact_user, _ = Contact.objects.get_or_create( first_name="Hank", last_name="McFakey", + title="test", + email="moar@igorville.gov", + phone="1234567890" ) site = DraftDomain.objects.create(name="igorville.gov") From 714a7232311be3659eb2c79819bf476c3386decc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 30 Jan 2025 08:58:37 -0700 Subject: [PATCH 12/15] lint --- src/registrar/tests/test_views_request.py | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index 9638ff669..d33d7e6ad 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -3079,28 +3079,16 @@ def test_unlocked_steps_partial_domain_request(self): # Create the site and contacts to delete (orphaned) contact = Contact.objects.create( - first_name="Henry", - last_name="Mcfakerson", - title="test", - email="moar@igorville.gov", - phone="1234567890" + first_name="Henry", last_name="Mcfakerson", title="test", email="moar@igorville.gov", phone="1234567890" ) # Create two non-orphaned contacts contact_2 = Contact.objects.create( - first_name="Saturn", - last_name="Mars", - title="test", - email="moar@igorville.gov", - phone="1234567890" + first_name="Saturn", last_name="Mars", title="test", email="moar@igorville.gov", phone="1234567890" ) # Attach a user object to a contact (should not be deleted) contact_user, _ = Contact.objects.get_or_create( - first_name="Hank", - last_name="McFakey", - title="test", - email="moar@igorville.gov", - phone="1234567890" + first_name="Hank", last_name="McFakey", title="test", email="moar@igorville.gov", phone="1234567890" ) site = DraftDomain.objects.create(name="igorville.gov") From d14a23621e7f76891bde5bdff5f8246dc342c288 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 3 Feb 2025 15:36:45 -0700 Subject: [PATCH 13/15] fix bug --- src/registrar/models/domain_request.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index f58c6c6a2..fdeebaad6 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1302,15 +1302,16 @@ def unlock_organization_contact(self) -> bool: """Unlocks the organization_contact step.""" if flag_is_active_anywhere("organization_feature") and flag_is_active_anywhere("organization_requests"): # Check if the current federal agency is an outlawed one - Portfolio = apps.get_model("registrar.Portfolio") - return ( - FederalAgency.objects.exclude( - agency__in=Portfolio.objects.values_list("organization_name", flat=True), + if self.organization_type == self.OrganizationChoices.FEDERAL and self.federal_agency: + Portfolio = apps.get_model("registrar.Portfolio") + return ( + FederalAgency.objects.exclude( + agency__in=Portfolio.objects.values_list("organization_name", flat=True), + ) + .filter(agency=self.federal_agency) + .exists() ) - .filter(agency=self.federal_agency) - .exists() - ) - return ( + return bool( self.federal_agency is not None or self.organization_name is not None or self.address_line1 is not None From 367b2b804316237b6d1dffa7d8263e92211a0e45 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 4 Feb 2025 09:58:17 -0700 Subject: [PATCH 14/15] Requested changes --- src/registrar/forms/domain_request_wizard.py | 8 ++++---- src/registrar/models/domain_request.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 0cf82558b..d7a02b124 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -322,7 +322,7 @@ class OrganizationContactForm(RegistrarForm): # if it has been filled in when required. # uncomment to see if modelChoiceField can be an arg later required=False, - # We populate this queryset in init. We want to exclude agencies with a portfolio. + # We populate this queryset in init. queryset=FederalAgency.objects.none(), widget=ComboboxWidget, ) @@ -369,12 +369,12 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # Set the queryset for federal agency. - # If the organization_requests flag is active, we hide data that exists in portfolios. + # If the organization_requests flag is active, We want to exclude agencies with a portfolio. federal_agency_queryset = FederalAgency.objects.exclude(agency__in=self.excluded_agencies) if flag_is_active_anywhere("organization_feature") and flag_is_active_anywhere("organization_requests"): - # Exclude both predefined agencies and those matching portfolio names in one query + # Exclude both predefined agencies and those matching portfolio records in one query federal_agency_queryset = federal_agency_queryset.exclude( - agency__in=Portfolio.objects.values_list("organization_name", flat=True) + id__in=Portfolio.objects.values_list("federal_agency__id", flat=True) ) self.fields["federal_agency"].queryset = federal_agency_queryset diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index fdeebaad6..3071d40d8 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1306,9 +1306,9 @@ def unlock_organization_contact(self) -> bool: Portfolio = apps.get_model("registrar.Portfolio") return ( FederalAgency.objects.exclude( - agency__in=Portfolio.objects.values_list("organization_name", flat=True), + id__in=Portfolio.objects.values_list("federal_agency__id", flat=True), ) - .filter(agency=self.federal_agency) + .filter(id=self.federal_agency.id) .exists() ) return bool( From 9bf152a8fc9abd5b1c6e07314daaa6bb08a06571 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 4 Feb 2025 13:18:24 -0700 Subject: [PATCH 15/15] unit test --- src/registrar/tests/test_views_request.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index d33d7e6ad..f6eba2a56 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -3227,7 +3227,9 @@ def test_unlock_organization_contact_flags_enabled(self): federal_agency = FederalAgency.objects.create(agency="Portfolio Agency") # Create a portfolio with matching organization name - Portfolio.objects.create(creator=self.user, organization_name=federal_agency.agency) + Portfolio.objects.create( + creator=self.user, organization_name=federal_agency.agency, federal_agency=federal_agency + ) # Create domain request with the portfolio agency domain_request = completed_domain_request(federal_agency=federal_agency, user=self.user)