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

Issue #2975 - domain and domain info portfolio fields -[HOTGOV] #3044

Merged
merged 37 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
9f9147e
Initial draft
CocoByte Nov 1, 2024
7292926
WIP on nl/2975-domain-and-domain-info-portfolio-fields
CocoByte Nov 6, 2024
0ac4e57
cleanup stray readonly field
CocoByte Nov 6, 2024
dcc0a51
Merge remote-tracking branch 'origin/main' into nl/2975-domain-and-do…
CocoByte Nov 6, 2024
b0e1b5a
updates to DomainAdmin
CocoByte Nov 6, 2024
ae0eb45
revert read-only field
CocoByte Nov 7, 2024
c481c3c
Merge remote-tracking branch 'origin/main' into nl/2975-domain-and-do…
CocoByte Nov 7, 2024
32232c5
export updates
CocoByte Nov 13, 2024
ba06660
Remove logs
CocoByte Nov 13, 2024
503d214
Cleanup
CocoByte Nov 25, 2024
76ac7a0
Added missing converted fields to domain request
CocoByte Nov 25, 2024
88554e0
Updates to analytics exports
CocoByte Nov 25, 2024
ca8cfda
unit test adjustments
CocoByte Nov 25, 2024
34ba850
linted
CocoByte Nov 25, 2024
e75c027
Fixed annotations and sort
CocoByte Nov 25, 2024
0446aab
unit test fixes
CocoByte Nov 25, 2024
3156315
cleanup
CocoByte Nov 25, 2024
c0a0ac3
linted
CocoByte Nov 25, 2024
36c19d6
fixes for unit test (and admin change form)
CocoByte Nov 26, 2024
bf34df5
resolved merge
CocoByte Nov 26, 2024
e7c22ce
linted
CocoByte Nov 26, 2024
d6f4b71
Merge remote-tracking branch 'origin/main' into nl/2975-domain-and-do…
CocoByte Nov 29, 2024
6ca5356
Re-added missing function (merge error)
CocoByte Nov 29, 2024
a6308ed
A few updates based on feedback
CocoByte Dec 2, 2024
9a8ac32
Remove converted values from exports + fixes
CocoByte Dec 3, 2024
fb2fad6
Efficiency updates and sorting fixes
CocoByte Dec 4, 2024
88e27c8
Merge remote-tracking branch 'origin/main' into nl/2975-domain-and-do…
CocoByte Dec 4, 2024
3b9e9e3
remove dead code (missed in another merge)
CocoByte Dec 4, 2024
87a9655
Cleanup and unit test fixes
CocoByte Dec 5, 2024
d5f7993
cleanup
CocoByte Dec 5, 2024
958d6dc
linted
CocoByte Dec 5, 2024
0a36b10
more unit test adjustments
CocoByte Dec 5, 2024
162ba73
Merge remote-tracking branch 'origin/main' into nl/2975-domain-and-do…
CocoByte Dec 5, 2024
a81cf60
Merge remote-tracking branch 'origin/main' into nl/2975-domain-and-do…
CocoByte Dec 5, 2024
77d4f0a
unit test fix
CocoByte Dec 5, 2024
4e1797a
remove maxDiff = None
CocoByte Dec 5, 2024
4be2aa1
Merge branch 'main' into nl/2975-domain-and-domain-info-portfolio-fields
CocoByte Dec 6, 2024
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
308 changes: 257 additions & 51 deletions src/registrar/admin.py

Large diffs are not rendered by default.

5 changes: 0 additions & 5 deletions src/registrar/config/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,6 @@
ExportDataTypeRequests.as_view(),
name="export_data_type_requests",
),
path(
"reports/export_data_type_requests/",
ExportDataTypeRequests.as_view(),
name="export_data_type_requests",
),
CocoByte marked this conversation as resolved.
Show resolved Hide resolved
path(
"domain-request/<int:id>/edit/",
views.DomainRequestWizard.as_view(),
Expand Down
44 changes: 29 additions & 15 deletions src/registrar/models/domain_information.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,13 +426,14 @@ def get_state_display_of_domain(self):
else:
return None

# ----- Portfolio Properties -----

@property
def converted_organization_name(self):
if self.portfolio:
return self.portfolio.organization_name
return self.organization_name

# ----- Portfolio Properties -----
@property
def converted_generic_org_type(self):
if self.portfolio:
Expand All @@ -442,8 +443,8 @@ def converted_generic_org_type(self):
@property
def converted_federal_agency(self):
if self.portfolio:
return self.portfolio.federal_agency
return self.federal_agency
return self.portfolio.federal_agency.agency
Comment on lines 445 to +446
Copy link
Contributor

Choose a reason for hiding this comment

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

(nitpick) shouldn't we check on self.portfolio and self.portfolio.federal_agency?

return self.federal_agency.agency

@property
def converted_federal_type(self):
Expand All @@ -454,20 +455,20 @@ def converted_federal_type(self):
@property
def converted_senior_official(self):
if self.portfolio:
return self.portfolio.senior_official
return self.senior_official
return self.portfolio.display_senior_official
return self.display_senior_official

@property
def converted_address_line1(self):
if self.portfolio:
return self.portfolio.address_line1
return self.address_line1
return self.portfolio.display_address_line1
return self.display_address_line1

@property
def converted_address_line2(self):
if self.portfolio:
return self.portfolio.address_line2
return self.address_line2
return self.portfolio.display_address_line2
return self.display_address_line2

@property
def converted_city(self):
Expand All @@ -478,17 +479,30 @@ def converted_city(self):
@property
def converted_state_territory(self):
if self.portfolio:
return self.portfolio.state_territory
return self.state_territory
return self.portfolio.get_state_territory_display()
return self.get_state_territory_display()

@property
def converted_zipcode(self):
if self.portfolio:
return self.portfolio.zipcode
return self.zipcode
return self.portfolio.display_zipcode
return self.display_zipcode

@property
def converted_urbanization(self):
if self.portfolio:
return self.portfolio.urbanization
return self.urbanization
return self.portfolio.display_urbanization
return self.display_urbanization

# ----- Portfolio Properties (display values)-----
@property
def converted_generic_org_type_display(self):
if self.portfolio:
return self.portfolio.get_organization_type_display()
return self.get_generic_org_type_display()

@property
def converted_federal_type_display(self):
if self.portfolio:
return self.portfolio.federal_agency.get_federal_type_display()
return self.get_federal_type_display()
37 changes: 37 additions & 0 deletions src/registrar/models/domain_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -1437,6 +1437,18 @@ def converted_federal_type(self):
return self.portfolio.federal_type
return self.federal_type

@property
def converted_address_line1(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

(nitpick / optional) These would read better if they followed the portfolio_address_line1 convention, but not a change you need to make as I know it was introduced prior

if self.portfolio:
return self.portfolio.address_line1
return self.address_line1

@property
def converted_address_line2(self):
if self.portfolio:
return self.portfolio.address_line2
return self.address_line2

@property
def converted_city(self):
if self.portfolio:
Expand All @@ -1449,8 +1461,33 @@ def converted_state_territory(self):
return self.portfolio.state_territory
return self.state_territory

@property
def converted_urbanization(self):
if self.portfolio:
return self.portfolio.urbanization
return self.urbanization

@property
def converted_zipcode(self):
if self.portfolio:
return self.portfolio.zipcode
return self.zipcode

@property
def converted_senior_official(self):
if self.portfolio:
return self.portfolio.senior_official
return self.senior_official

# ----- Portfolio Properties (display values)-----
@property
def converted_generic_org_type_display(self):
if self.portfolio:
return self.portfolio.get_organization_type_display()
return self.get_generic_org_type_display()

@property
def converted_federal_type_display(self):
if self.portfolio:
return self.portfolio.federal_agency.get_federal_type_display()
return self.get_federal_type_display()
5 changes: 4 additions & 1 deletion src/registrar/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,9 +563,12 @@ def sharedSetUp(cls):

cls.federal_agency_1, _ = FederalAgency.objects.get_or_create(agency="World War I Centennial Commission")
cls.federal_agency_2, _ = FederalAgency.objects.get_or_create(agency="Armed Forces Retirement Home")
cls.federal_agency_3, _ = FederalAgency.objects.get_or_create(
agency="Portfolio 1 Federal Agency", federal_type="executive"
)

cls.portfolio_1, _ = Portfolio.objects.get_or_create(
creator=cls.custom_superuser, federal_agency=cls.federal_agency_1
creator=cls.custom_superuser, federal_agency=cls.federal_agency_3, organization_type="federal"
)

current_date = get_time_aware_date(datetime(2024, 4, 2))
Expand Down
4 changes: 2 additions & 2 deletions src/registrar/tests/test_admin_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -728,9 +728,9 @@ def test_short_org_name_in_domains_list(self):
response = self.client.get("/admin/registrar/domain/")
# There are 4 template references to Federal (4) plus four references in the table
# for our actual domain_request
self.assertContains(response, "Federal", count=56)
self.assertContains(response, "Federal", count=57)
# This may be a bit more robust
self.assertContains(response, '<td class="field-generic_org_type">Federal</td>', count=1)
self.assertContains(response, '<td class="field-converted_generic_org_type">Federal</td>', count=1)
# Now let's make sure the long description does not exist
self.assertNotContains(response, "Federal: an agency of the U.S. government")

Expand Down
6 changes: 2 additions & 4 deletions src/registrar/tests/test_admin_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -576,9 +576,9 @@ def test_short_org_name_in_domain_requests_list(self):
response = self.client.get("/admin/registrar/domainrequest/?generic_org_type__exact=federal")
# There are 2 template references to Federal (4) and two in the results data
# of the request
self.assertContains(response, "Federal", count=51)
self.assertContains(response, "Federal", count=55)
# This may be a bit more robust
self.assertContains(response, '<td class="field-converted_generic_org_type">federal</td>', count=1)
self.assertContains(response, '<td class="field-converted_generic_org_type">Federal</td>', count=1)
# Now let's make sure the long description does not exist
self.assertNotContains(response, "Federal: an agency of the U.S. government")

Expand Down Expand Up @@ -1693,7 +1693,6 @@ def test_readonly_when_restricted_creator(self):
"notes",
"alternative_domains",
]
self.maxDiff = None
self.assertEqual(readonly_fields, expected_fields)

def test_readonly_fields_for_analyst(self):
Expand All @@ -1702,7 +1701,6 @@ def test_readonly_fields_for_analyst(self):
request.user = self.staffuser

readonly_fields = self.admin.get_readonly_fields(request)
self.maxDiff = None
expected_fields = [
"portfolio_senior_official",
"portfolio_organization_type",
Expand Down
1 change: 0 additions & 1 deletion src/registrar/tests/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ def test_groups_created(self):

# Get the codenames of actual permissions associated with the group
actual_permissions = [p.codename for p in cisa_analysts_group.permissions.all()]
self.maxDiff = None

# Assert that the actual permissions match the expected permissions
self.assertListEqual(actual_permissions, expected_permissions)
Loading
Loading