Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#3317: Don't allow non-org model domain requests for federal agencies in a portfolio [ZA] #3408

Merged
merged 23 commits into from
Feb 4, 2025
Merged
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a6de529
Core logic
zandercymatics Jan 23, 2025
a4f1abc
add check
zandercymatics Jan 24, 2025
0725447
Merge branch 'main' into za/3317-hide-org-federal-agencies
zandercymatics Jan 27, 2025
297029a
Add UI logic
zandercymatics Jan 27, 2025
1f9efb7
Add unit test for waffle utility
zandercymatics Jan 27, 2025
bdb3875
Add tests for unlock_organization_contact
zandercymatics Jan 27, 2025
8504eb1
Merge branch 'main' into za/3317-hide-org-federal-agencies
zandercymatics Jan 28, 2025
ad79557
lint and fix test
zandercymatics Jan 28, 2025
0478d2b
Fix unit tests
zandercymatics Jan 28, 2025
4dba0a3
remove test code and lint
zandercymatics Jan 28, 2025
b5935a3
lint
zandercymatics Jan 29, 2025
cc0c530
Update test_forms.py
zandercymatics Jan 29, 2025
59d1301
Update test_views_request.py
zandercymatics Jan 30, 2025
714a723
lint
zandercymatics Jan 30, 2025
b24ab81
Merge branch 'main' into za/3317-hide-org-federal-agencies
zandercymatics Jan 30, 2025
f1005f5
Merge branch 'main' into za/3317-hide-org-federal-agencies
zandercymatics Jan 31, 2025
e270b8d
Merge branch 'main' into za/3317-hide-org-federal-agencies
zandercymatics Feb 3, 2025
8419d22
Merge branch 'main' into za/3317-hide-org-federal-agencies
zandercymatics Feb 3, 2025
d14a236
fix bug
zandercymatics Feb 3, 2025
367b2b8
Requested changes
zandercymatics Feb 4, 2025
05bf5d0
Merge branch 'main' into za/3317-hide-org-federal-agencies
zandercymatics Feb 4, 2025
9bf152a
unit test
zandercymatics Feb 4, 2025
0d1af72
Merge branch 'main' into za/3317-hide-org-federal-agencies
zandercymatics Feb 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
lint and fix test
zandercymatics committed Jan 28, 2025

Verified

This commit was signed with the committer’s verified signature.
JayKickliter Jay Kickliter
commit ad79557a55598cc4f67c33d11b01acf76a7024be
4 changes: 2 additions & 2 deletions src/registrar/forms/domain_request_wizard.py
Original file line number Diff line number Diff line change
@@ -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)
Copy link
Contributor

@allly-b allly-b Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zandercymatics why not just check the federal agency value against the federal agency value on the portfolio? This avoids the risk of the org name naming convention changing in the future and this breaking silently

Copy link
Contributor Author

@zandercymatics zandercymatics Feb 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abroddrick Good point! I'll implement this

)

self.fields["federal_agency"].queryset = federal_agency_queryset

def clean_federal_agency(self):
1 change: 0 additions & 1 deletion src/registrar/models/domain.py
Original file line number Diff line number Diff line change
@@ -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
10 changes: 7 additions & 3 deletions src/registrar/models/domain_request.py
Original file line number Diff line number Diff line change
@@ -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 (
10 changes: 5 additions & 5 deletions src/registrar/tests/test_utilities.py
Original file line number Diff line number Diff line change
@@ -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")
28 changes: 8 additions & 20 deletions src/registrar/tests/test_views_request.py
Original file line number Diff line number Diff line change
@@ -3228,38 +3228,26 @@ 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)
@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
# 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())