From eb061be71bf34ec3f75fabe282089e5c80d80457 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 21 Aug 2024 12:25:27 -0400 Subject: [PATCH 01/69] Django admin portfolio members section --- src/registrar/admin.py | 110 ++++++++++++++++-- src/registrar/models/user.py | 43 +++++++ .../admin/includes/detail_table_fieldset.html | 11 ++ .../django/admin/portfolio_change_form.html | 3 +- src/registrar/tests/test_admin.py | 80 +++++++++++++ src/registrar/tests/test_models.py | 55 +++++++++ 6 files changed, 288 insertions(+), 14 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 423c0a01b..12c5d8cf8 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2867,11 +2867,7 @@ class PortfolioAdmin(ListHeaderAdmin): fieldsets = [ # created_on is the created_at field, and portfolio_type is f"{organization_type} - {federal_type}" (None, {"fields": ["portfolio_type", "organization_name", "creator", "created_on", "notes"]}), - # TODO - uncomment in #2521 - # ("Portfolio members", { - # "classes": ("collapse", "closed"), - # "fields": ["administrators", "members"]} - # ), + ("Portfolio members", {"fields": ["display_admins", "display_members"]}), ("Portfolio domains", {"classes": ("collapse", "closed"), "fields": ["domains", "domain_requests"]}), ("Type of organization", {"fields": ["organization_type", "federal_type"]}), ( @@ -2925,8 +2921,94 @@ class PortfolioAdmin(ListHeaderAdmin): "domain_requests", "suborganizations", "portfolio_type", + # Django admin doesn't allow methods to be directly listed in fieldsets. We can + # display the custom methods display_admins amd display_members in the admin form if + # they are readonly. + "display_admins", + "display_members", ] + def display_admins(self, obj): + """Get joined users who are Admin, unpack and return an HTML block. + + 'DJA readonly can't handle querysets, so we need to unpack and return html here. + Alternatively, we could return querysets in context but that would limit where this + data would display in a custom change form without extensive template customization. + + Will be used in the field_readonly block""" + admins = [user for user in obj.user.all() if "Admin" in user.portfolio_role_summary] + if not admins: + return format_html("

No admins found.

") + + admin_details = "" + for portfolio_admin in admins: + change_url = reverse("admin:registrar_user_change", args=[portfolio_admin.pk]) + admin_details += "
" + admin_details += f'{portfolio_admin}
' + admin_details += f"{portfolio_admin.title}
" + admin_details += f"{portfolio_admin.email}" + admin_details += "
" + admin_details += f"" + admin_details += ( + "" + admin_details += "

" + admin_details += f"{portfolio_admin.phone}" + admin_details += "
" + return format_html(admin_details) + + display_admins.short_description = "Administrators" # type: ignore + + def display_members(self, obj): + """Get joined users who have roles/perms that are not Admin, unpack and return an HTML block. + + DJA readonly can't handle querysets, so we need to unpack and return html here. + Alternatively, we could return querysets in context but that would limit where this + data would display in a custom change form without extensive template customization. + + Will be used in the after_help_text block.""" + members = [user for user in obj.user.all() if "Admin" not in user.portfolio_role_summary] + if not members: + return format_html("

No members found.

") + + member_details = ( + "" + + "" + ) + for member in members: + full_name = ( + f"{member.first_name} {member.middle_name} {member.last_name}" + if member.middle_name + else f"{member.first_name} {member.last_name}" + ) + member_details += "" + member_details += f"" + member_details += f"" + member_details += f"" + member_details += f"" + member_details += "" + member_details += "
NameTitleEmailPhoneRoles
{full_name}{member.title}{member.email}{member.phone}" + for role in member.portfolio_role_summary: + member_details += f"{role} " + member_details += "
" + return format_html(member_details) + + display_members.short_description = "Members" # type: ignore + + def display_members_summary(self, obj): + """Will be passed as context and used in the field_readonly block.""" + members = [user for user in obj.user.all() if "Admin" not in user.portfolio_role_summary] + if not members: + return {} + + return self.get_field_links_as_list(members, "user", separator=", ") + def federal_type(self, obj: models.Portfolio): """Returns the federal_type field""" return BranchChoices.get_branch_label(obj.federal_type) if obj.federal_type else "-" @@ -2977,7 +3059,7 @@ class PortfolioAdmin(ListHeaderAdmin): ] def get_field_links_as_list( - self, queryset, model_name, attribute_name=None, link_info_attribute=None, seperator=None + self, queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None ): """ Generate HTML links for items in a queryset, using a specified attribute for link text. @@ -3009,14 +3091,14 @@ class PortfolioAdmin(ListHeaderAdmin): if link_info_attribute: link += f" ({self.value_of_attribute(item, link_info_attribute)})" - if seperator: + if separator: links.append(link) else: links.append(f"
  • {link}
    • ") - # If no seperator is specified, just return an unordered list. - if seperator: - return format_html(seperator.join(links)) if links else "-" + # If no separator is specified, just return an unordered list. + if separator: + return format_html(separator.join(links)) if links else "-" else: links = "".join(links) return format_html(f'') if links else "-" @@ -3059,8 +3141,12 @@ class PortfolioAdmin(ListHeaderAdmin): return readonly_fields def change_view(self, request, object_id, form_url="", extra_context=None): - """Add related suborganizations and domain groups""" - extra_context = {"skip_additional_contact_info": True} + """Add related suborganizations and domain groups. + Add the summary for the portfolio members field (list of members that link to change_forms).""" + obj = self.get_object(request, object_id) + extra_context = extra_context or {} + extra_context["skip_additional_contact_info"] = True + extra_context["display_members_summary"] = self.display_members_summary(obj) return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 81d3b9b61..a6026b63a 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -293,6 +293,49 @@ class User(AbstractUser): def has_edit_suborganization(self): return self._has_portfolio_permission(UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) + def has_edit_requests(self): + return self._has_portfolio_permission(UserPortfolioPermissionChoices.EDIT_REQUESTS) + + @property + def portfolio_role_summary(self): + """Returns a list of roles based on the user's permissions.""" + roles = [] + + # Define the conditions and their corresponding roles + conditions_roles = [ + (self.has_edit_suborganization(), ["Admin"]), + ( + self.has_view_all_domains_permission() + and self.has_domain_requests_portfolio_permission() + and self.has_edit_requests(), + ["View-only admin", "Domain requestor"], + ), + ( + self.has_view_all_domains_permission() and self.has_domain_requests_portfolio_permission(), + ["View-only admin"], + ), + ( + self.has_base_portfolio_permission() + and self.has_edit_requests() + and self.has_domains_portfolio_permission(), + ["Member", "Domain requestor", "Domain manager"], + ), + (self.has_base_portfolio_permission() and self.has_edit_requests(), ["Member", "Domain requestor"]), + ( + self.has_base_portfolio_permission() and self.has_domains_portfolio_permission(), + ["Member", "Domain manager"], + ), + (self.has_base_portfolio_permission(), ["Member"]), + ] + + # Evaluate conditions and add roles + for condition, role_list in conditions_roles: + if condition: + roles.extend(role_list) + break + + return roles + @classmethod def needs_identity_verification(cls, email, uuid): """A method used by our oidc classes to test whether a user needs email/uuid verification diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 683f33117..5da6c2ca2 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -137,6 +137,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endfor %} {% endwith %} + {% elif field.field.name == "display_admins" %} +
    {{ field.contents|safe }}
    + {% elif field.field.name == "display_members" %} +
    {{ display_members_summary }}
    {% else %}
    {{ field.contents }}
    {% endif %} @@ -240,6 +244,13 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endif %} {% endwith %} + {% elif field.field.name == "display_members" %} +
    + Details +
    + {{ field.contents|safe }} +
    +
    {% elif field.field.name == "state_territory" %}
    diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 9d59aae42..a2d70611c 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -14,8 +14,7 @@ This is a placeholder for now. Disclaimer: - When extending the fieldset view - *make a new one* that extends from detail_table_fieldset. - For instance, "portfolio_fieldset.html". + When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset. detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences. {% endcomment %} {% include "django/admin/includes/detail_table_fieldset.html" with original_object=original %} diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 827742ef1..7ef1a4a97 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -45,6 +45,7 @@ from registrar.models import ( from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial from registrar.models.user_domain_role import UserDomainRole +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.models.verified_by_staff import VerifiedByStaff from .common import ( MockDbForSharedTests, @@ -2066,6 +2067,7 @@ class TestPortfolioAdmin(TestCase): DomainRequest.objects.all().delete() Domain.objects.all().delete() Portfolio.objects.all().delete() + User.objects.all().delete() @less_console_noise_decorator def test_created_on_display(self): @@ -2121,3 +2123,81 @@ class TestPortfolioAdmin(TestCase): self.assertIn("request1.gov", domain_requests) self.assertIn("request2.gov", domain_requests) self.assertIn('
    @@ -25,7 +25,7 @@ Template for an input field with a clipboard
    {% endif %} \ No newline at end of file diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 02cd6e0f6..93e611c1a 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -45,6 +45,7 @@ from registrar.models import ( from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial from registrar.models.user_domain_role import UserDomainRole +from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.models.verified_by_staff import VerifiedByStaff from .common import ( @@ -2129,8 +2130,10 @@ class TestPortfolioAdmin(TestCase): last_name="Meoward", title="Captain", email="meaoward@gov.gov", - portfolio=self.portfolio, - portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_1, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) admin_user_2 = User.objects.create( @@ -2139,8 +2142,10 @@ class TestPortfolioAdmin(TestCase): last_name="Poopy", title="Major", email="poopy@gov.gov", - portfolio=self.portfolio, - portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_2, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) admin_user_3 = User.objects.create( @@ -2149,8 +2154,10 @@ class TestPortfolioAdmin(TestCase): last_name="Max", title="Road warrior", email="madmax@gov.gov", - portfolio=self.portfolio, - portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_3, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] ) admin_user_4 = User.objects.create( @@ -2159,8 +2166,12 @@ class TestPortfolioAdmin(TestCase): last_name="Smith", title="Program", email="thematrix@gov.gov", + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_4, portfolio=self.portfolio, - portfolio_additional_permissions=[ + additional_permissions=[ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_REQUESTS, ], From ef2adaaf288e7a386ce67e2cbec0b5632ed8fcad Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 30 Aug 2024 17:25:56 -0400 Subject: [PATCH 52/69] No additional members found --- .../templates/django/admin/includes/detail_table_fieldset.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index da2753b4c..7eea7b72b 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -144,7 +144,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% if display_members_summary %} {{ display_members_summary }} {% else %} -

    No members found.

    +

    No additional members found.

    {% endif %} {% else %} From ca1c9bbb0424313425848f5e004b64968994112b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 30 Aug 2024 17:35:57 -0400 Subject: [PATCH 53/69] fix model tests? --- src/registrar/tests/test_models.py | 39 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 19f9d46c3..3510ff65e 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1308,6 +1308,7 @@ class TestUser(TestCase): self.domain, _ = Domain.objects.get_or_create(name="igorville.gov") self.user, _ = User.objects.get_or_create(email=self.email) self.factory = RequestFactory() + self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.user) def tearDown(self): super().tearDown() @@ -1325,57 +1326,57 @@ class TestUser(TestCase): @patch.object(User, "has_edit_suborganization", return_value=True) def test_portfolio_role_summary_admin(self, mock_edit_suborganization): # Test if the user is recognized as an Admin - self.assertEqual(self.user.portfolio_role_summary, ["Admin"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Admin"]) @patch.multiple( User, - has_view_all_domains_permission=lambda self: True, - has_domain_requests_portfolio_permission=lambda self: True, - has_edit_requests=lambda self: True, + has_view_all_domains_permission=lambda self, portfolio: True, + has_domain_requests_portfolio_permission=lambda self, portfolio: True, + has_edit_requests=lambda self, portfolio: True, ) def test_portfolio_role_summary_view_only_admin_and_domain_requestor(self): # Test if the user has both 'View-only admin' and 'Domain requestor' roles - self.assertEqual(self.user.portfolio_role_summary, ["View-only admin", "Domain requestor"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["View-only admin", "Domain requestor"]) @patch.multiple( User, - has_view_all_domains_permission=lambda self: True, - has_domain_requests_portfolio_permission=lambda self: True, + has_view_all_domains_permission=lambda self, portfolio: True, + has_domain_requests_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_view_only_admin(self): # Test if the user is recognized as a View-only admin - self.assertEqual(self.user.portfolio_role_summary, ["View-only admin"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["View-only admin"]) @patch.multiple( User, - has_base_portfolio_permission=lambda self: True, - has_edit_requests=lambda self: True, - has_domains_portfolio_permission=lambda self: True, + has_base_portfolio_permission=lambda self, portfolio: True, + has_edit_requests=lambda self, portfolio: True, + has_domains_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_member_domain_requestor_domain_manager(self): # Test if the user has 'Member', 'Domain requestor', and 'Domain manager' roles - self.assertEqual(self.user.portfolio_role_summary, ["Domain requestor", "Domain manager"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor", "Domain manager"]) - @patch.multiple(User, has_base_portfolio_permission=lambda self: True, has_edit_requests=lambda self: True) + @patch.multiple(User, has_base_portfolio_permission=lambda self, portfolio: True, has_edit_requests=lambda self, portfolio: True) def test_portfolio_role_summary_member_domain_requestor(self): # Test if the user has 'Member' and 'Domain requestor' roles - self.assertEqual(self.user.portfolio_role_summary, ["Domain requestor"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor"]) @patch.multiple( - User, has_base_portfolio_permission=lambda self: True, has_domains_portfolio_permission=lambda self: True + User, has_base_portfolio_permission=lambda self, portfolio: True, has_domains_portfolio_permission=lambda self, portfolio: True ) def test_portfolio_role_summary_member_domain_manager(self): # Test if the user has 'Member' and 'Domain manager' roles - self.assertEqual(self.user.portfolio_role_summary, ["Domain manager"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain manager"]) - @patch.multiple(User, has_base_portfolio_permission=lambda self: True) + @patch.multiple(User, has_base_portfolio_permission=lambda self, portfolio: True) def test_portfolio_role_summary_member(self): # Test if the user is recognized as a Member - self.assertEqual(self.user.portfolio_role_summary, ["Member"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Member"]) def test_portfolio_role_summary_empty(self): # Test if the user has no roles - self.assertEqual(self.user.portfolio_role_summary, []) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), []) @less_console_noise_decorator def test_check_transition_domains_without_domains_on_login(self): From e20bfa55678ee12c7ba897ecf52f4c3cad0ad84b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 30 Aug 2024 17:44:09 -0400 Subject: [PATCH 54/69] lint --- src/registrar/admin.py | 6 +----- src/registrar/tests/test_models.py | 8 ++++++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a47617c28..c8be04715 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3085,11 +3085,7 @@ class PortfolioAdmin(ListHeaderAdmin): + "PhoneRoles" ) for member in members: - full_name = ( - f"{member.first_name} {member.middle_name} {member.last_name}" - if member.middle_name - else f"{member.first_name} {member.last_name}" - ) + full_name = member.get_formatted_name() member_details += "" member_details += f"{full_name}" member_details += f"{member.title}" diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 3510ff65e..7983fed36 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1357,13 +1357,17 @@ class TestUser(TestCase): # Test if the user has 'Member', 'Domain requestor', and 'Domain manager' roles self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor", "Domain manager"]) - @patch.multiple(User, has_base_portfolio_permission=lambda self, portfolio: True, has_edit_requests=lambda self, portfolio: True) + @patch.multiple( + User, has_base_portfolio_permission=lambda self, portfolio: True, has_edit_requests=lambda self, portfolio: True + ) def test_portfolio_role_summary_member_domain_requestor(self): # Test if the user has 'Member' and 'Domain requestor' roles self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor"]) @patch.multiple( - User, has_base_portfolio_permission=lambda self, portfolio: True, has_domains_portfolio_permission=lambda self, portfolio: True + User, + has_base_portfolio_permission=lambda self, portfolio: True, + has_domains_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_member_domain_manager(self): # Test if the user has 'Member' and 'Domain manager' roles From 79fbc5c54026c7516e39347a7d7be50d0f9a6263 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 30 Aug 2024 17:46:05 -0400 Subject: [PATCH 55/69] cleanup --- src/registrar/admin.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c8be04715..fd32238f7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2943,11 +2943,6 @@ class PortfolioAdmin(ListHeaderAdmin): # created_on is the created_at field, and portfolio_type is f"{organization_type} - {federal_type}" (None, {"fields": ["portfolio_type", "organization_name", "creator", "created_on", "notes"]}), ("Portfolio members", {"fields": ["display_admins", "display_members"]}), - # TODO - uncomment in #2521 - # ("Portfolio members", { - # "classes": ("collapse", "closed"), - # "fields": ["administrators", "members"]} - # ), ("Portfolio domains", {"fields": ["domains", "domain_requests"]}), ("Type of organization", {"fields": ["organization_type", "federal_type"]}), ( @@ -2995,15 +2990,14 @@ class PortfolioAdmin(ListHeaderAdmin): readonly_fields = [ # This is the created_at field "created_on", - # Custom fields such as these must be defined as readonly. + # Django admin doesn't allow methods to be directly listed in fieldsets. We can + # display the custom methods display_admins amd display_members in the admin form if + # they are readonly. "federal_type", "domains", "domain_requests", "suborganizations", "portfolio_type", - # Django admin doesn't allow methods to be directly listed in fieldsets. We can - # display the custom methods display_admins amd display_members in the admin form if - # they are readonly. "display_admins", "display_members", "creator", From dd1b92340401a462d22dbd9df2cdf383e3bd3029 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 4 Sep 2024 13:41:26 -0400 Subject: [PATCH 56/69] wip --- src/registrar/context_processors.py | 6 ++ src/registrar/models/user.py | 56 ++++++++++--------- .../models/user_portfolio_permission.py | 6 +- .../models/utility/portfolio_helper.py | 4 +- .../templates/includes/header_extended.html | 6 +- 5 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index ea04dca80..beae164c3 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -69,6 +69,8 @@ def portfolio_permissions(request): "has_domain_requests_portfolio_permission": request.user.has_domain_requests_portfolio_permission( portfolio ), + "has_view_members_portfolio_permission": request.user.has_view_members_portfolio_permission(portfolio), + "has_edit_members_portfolio_permission": request.user.has_edit_members_portfolio_permission(portfolio), "has_view_suborganization": request.user.has_view_suborganization(portfolio), "has_edit_suborganization": request.user.has_edit_suborganization(portfolio), "portfolio": portfolio, @@ -78,6 +80,8 @@ def portfolio_permissions(request): "has_base_portfolio_permission": False, "has_domains_portfolio_permission": False, "has_domain_requests_portfolio_permission": False, + "has_view_members_portfolio_permission": False, + "has_edit_members_portfolio_permission": False, "has_view_suborganization": False, "has_edit_suborganization": False, "portfolio": None, @@ -90,6 +94,8 @@ def portfolio_permissions(request): "has_base_portfolio_permission": False, "has_domains_portfolio_permission": False, "has_domain_requests_portfolio_permission": False, + "has_view_members_portfolio_permission": False, + "has_edit_members_portfolio_permission": False, "has_view_suborganization": False, "has_edit_suborganization": False, "portfolio": None, diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index a7ea1e14a..d68104566 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -64,32 +64,6 @@ class User(AbstractUser): # after they login. FIXTURE_USER = "fixture_user", "Created by fixtures" - PORTFOLIO_ROLE_PERMISSIONS = { - UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, - UserPortfolioPermissionChoices.EDIT_MEMBER, - UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.EDIT_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.EDIT_PORTFOLIO, - # Domain: field specific permissions - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, - UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, - ], - UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY: [ - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, - UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - # Domain: field specific permissions - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, - ], - UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - ], - } - # #### Constants for choice fields #### RESTRICTED = "restricted" STATUS_CHOICES = ((RESTRICTED, RESTRICTED),) @@ -224,10 +198,40 @@ class User(AbstractUser): ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) def has_domain_requests_portfolio_permission(self, portfolio): + ## BEGIN + ## Note code below is to add organization_request feature + request = HttpRequest() + request.user = self + has_organization_requests_flag = flag_is_active(request, "organization_requests") + if not has_organization_requests_flag: + return False + ## END return self._has_portfolio_permission( portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) + def has_view_members_portfolio_permission(self, portfolio): + ## BEGIN + ## Note code below is to add organization_request feature + request = HttpRequest() + request.user = self + has_organization_members_flag = flag_is_active(request, "organization_members") + if not has_organization_members_flag: + return False + ## END + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MEMBERS) + + def has_edit_members_portfolio_permission(self, portfolio): + ## BEGIN + ## Note code below is to add organization_request feature + request = HttpRequest() + request.user = self + has_organization_members_flag = flag_is_active(request, "organization_members") + if not has_organization_members_flag: + return False + ## END + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_MEMBERS) + def has_view_all_domains_permission(self, portfolio): """Determines if the current user can view all available domains in a given portfolio""" return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index bf1c3e566..0c2487df3 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -16,8 +16,8 @@ class UserPortfolioPermission(TimeStampedModel): PORTFOLIO_ROLE_PERMISSIONS = { UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, - UserPortfolioPermissionChoices.EDIT_MEMBER, + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.EDIT_REQUESTS, UserPortfolioPermissionChoices.VIEW_PORTFOLIO, @@ -28,7 +28,7 @@ class UserPortfolioPermission(TimeStampedModel): ], UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY: [ UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, + UserPortfolioPermissionChoices.VIEW_MEMBERS, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.VIEW_PORTFOLIO, # Domain: field specific permissions diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 86aaa5e16..7afd32603 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -17,8 +17,8 @@ class UserPortfolioPermissionChoices(models.TextChoices): VIEW_ALL_DOMAINS = "view_all_domains", "View all domains and domain reports" VIEW_MANAGED_DOMAINS = "view_managed_domains", "View managed domains" - VIEW_MEMBER = "view_member", "View members" - EDIT_MEMBER = "edit_member", "Create and edit members" + VIEW_MEMBERS = "view_members", "View members" + EDIT_MEMBERS = "edit_members", "Create and edit members" VIEW_ALL_REQUESTS = "view_all_requests", "View all requests" VIEW_CREATED_REQUESTS = "view_created_requests", "View created requests" diff --git a/src/registrar/templates/includes/header_extended.html b/src/registrar/templates/includes/header_extended.html index 54ceb3a67..3bda2c326 100644 --- a/src/registrar/templates/includes/header_extended.html +++ b/src/registrar/templates/includes/header_extended.html @@ -23,11 +23,11 @@ Domains     -
  • + {% if has_domain_requests_portfolio_permission %}
  • @@ -37,11 +37,13 @@
    • {% endif %} + {% if has_view_members_portfolio_permission %}
  • Members
    • + {% endif %}
  • {% url 'organization' as url %} From cba2b27d79122951ee67c680a62021a72beebe48 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 4 Sep 2024 13:44:45 -0400 Subject: [PATCH 57/69] escape dynamic values --- src/registrar/admin.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 4b36c9589..e3bd5c9f7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3068,11 +3068,11 @@ class PortfolioAdmin(ListHeaderAdmin): for portfolio_admin in admins: change_url = reverse("admin:registrar_user_change", args=[portfolio_admin.pk]) admin_details += "
    " - admin_details += f'{portfolio_admin}
    ' - admin_details += f"{portfolio_admin.title}
    " - admin_details += f"{portfolio_admin.email}" + admin_details += f'{escape(portfolio_admin)}
    ' + admin_details += f"{escape(portfolio_admin.title)}
    " + admin_details += f"{escape(portfolio_admin.email)}" admin_details += "
    " - admin_details += f"" + admin_details += f"" admin_details += ( "" admin_details += "

    " - admin_details += f"{portfolio_admin.phone}" + admin_details += f"{escape(portfolio_admin.phone)}" admin_details += "
    " return format_html(admin_details) @@ -3108,13 +3108,13 @@ class PortfolioAdmin(ListHeaderAdmin): for member in members: full_name = member.get_formatted_name() member_details += "" - member_details += f"{full_name}" - member_details += f"{member.title}" - member_details += f"{member.email}" - member_details += f"{member.phone}" + member_details += f"{escape(full_name)}" + member_details += f"{escape(member.title)}" + member_details += f"{escape(member.email)}" + member_details += f"{escape(member.phone)}" member_details += "" for role in member.portfolio_role_summary(obj): - member_details += f"{role} " + member_details += f"{escape(role)} " member_details += "" member_details += "" return format_html(member_details) From 39e6cb89330ed2a3e43c7c4ed9cdf797ef9179bd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 4 Sep 2024 11:50:44 -0600 Subject: [PATCH 58/69] Add members --- src/registrar/views/utility/__init__.py | 1 + src/registrar/views/utility/mixins.py | 17 +++++++++++++++++ src/registrar/views/utility/permission_views.py | 9 +++++++++ 3 files changed, 27 insertions(+) diff --git a/src/registrar/views/utility/__init__.py b/src/registrar/views/utility/__init__.py index 7219f4358..7e4e19085 100644 --- a/src/registrar/views/utility/__init__.py +++ b/src/registrar/views/utility/__init__.py @@ -7,5 +7,6 @@ from .permission_views import ( DomainRequestPermissionWithdrawView, DomainInvitationPermissionDeleteView, DomainRequestWizardPermissionView, + PortfolioMembersPermission, ) from .api_views import get_senior_official_from_federal_agency_json diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 6f0745f41..190d80981 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -454,3 +454,20 @@ class PortfolioDomainRequestsPermission(PortfolioBasePermission): return False return super().has_permission() + + +class PortfolioMembersPermission(PortfolioBasePermission): + """Permission mixin that allows access to portfolio members pages if user + has access, otherwise 403""" + + def has_permission(self): + """Check if this user has access to members for this portfolio. + + The user is in self.request.user and the portfolio can be looked + up from the portfolio's primary key in self.kwargs["pk"]""" + + portfolio = self.request.session.get("portfolio") + if not self.request.user.has_view_members(portfolio): + return False + + return super().has_permission() diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index 0ff7d1676..734a6dbfe 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -18,6 +18,7 @@ from .mixins import ( UserDeleteDomainRolePermission, UserProfilePermission, PortfolioBasePermission, + PortfolioMembersPermission, ) import logging @@ -229,3 +230,11 @@ class PortfolioDomainRequestsPermissionView(PortfolioDomainRequestsPermission, P This abstract view cannot be instantiated. Actual views must specify `template_name`. """ + + +class PortfolioMembersPermissionView(PortfolioMembersPermission, PortfolioBasePermissionView, abc.ABC): + """Abstract base view for portfolio domain request views that enforces permissions. + + This abstract view cannot be instantiated. Actual views must specify + `template_name`. + """ \ No newline at end of file From dbe438b6dcf4c0208a71da57e2060e85b6c2072e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 4 Sep 2024 11:52:02 -0600 Subject: [PATCH 59/69] Update context_processors.py --- src/registrar/context_processors.py | 35 +++++++++++------------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index beae164c3..2ac22b2e0 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -60,6 +60,17 @@ def add_has_profile_feature_flag_to_context(request): def portfolio_permissions(request): """Make portfolio permissions for the request user available in global context""" + context = { + "has_base_portfolio_permission": False, + "has_domains_portfolio_permission": False, + "has_domain_requests_portfolio_permission": False, + "has_view_members_portfolio_permission": False, + "has_edit_members_portfolio_permission": False, + "has_view_suborganization": False, + "has_edit_suborganization": False, + "portfolio": None, + "has_organization_feature_flag": False, + } try: portfolio = request.session.get("portfolio") if portfolio: @@ -76,28 +87,8 @@ def portfolio_permissions(request): "portfolio": portfolio, "has_organization_feature_flag": True, } - return { - "has_base_portfolio_permission": False, - "has_domains_portfolio_permission": False, - "has_domain_requests_portfolio_permission": False, - "has_view_members_portfolio_permission": False, - "has_edit_members_portfolio_permission": False, - "has_view_suborganization": False, - "has_edit_suborganization": False, - "portfolio": None, - "has_organization_feature_flag": False, - } + return context except AttributeError: # Handles cases where request.user might not exist - return { - "has_base_portfolio_permission": False, - "has_domains_portfolio_permission": False, - "has_domain_requests_portfolio_permission": False, - "has_view_members_portfolio_permission": False, - "has_edit_members_portfolio_permission": False, - "has_view_suborganization": False, - "has_edit_suborganization": False, - "portfolio": None, - "has_organization_feature_flag": False, - } + return context From d802b51dc47d5abeb854dd111bb70af3e28798b2 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 4 Sep 2024 14:38:58 -0400 Subject: [PATCH 60/69] unit tests and lint --- src/registrar/models/user.py | 20 ++-- src/registrar/tests/test_views_portfolio.py | 98 +++++++++++++++++++ .../views/utility/permission_views.py | 2 +- 3 files changed, 109 insertions(+), 11 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index d68104566..4b4659094 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -6,7 +6,7 @@ from django.db.models import Q from django.http import HttpRequest from registrar.models import DomainInformation, UserDomainRole -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices from .domain_invitation import DomainInvitation from .portfolio_invitation import PortfolioInvitation @@ -198,38 +198,38 @@ class User(AbstractUser): ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) def has_domain_requests_portfolio_permission(self, portfolio): - ## BEGIN - ## Note code below is to add organization_request feature + # BEGIN + # Note code below is to add organization_request feature request = HttpRequest() request.user = self has_organization_requests_flag = flag_is_active(request, "organization_requests") if not has_organization_requests_flag: return False - ## END + # END return self._has_portfolio_permission( portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) def has_view_members_portfolio_permission(self, portfolio): - ## BEGIN - ## Note code below is to add organization_request feature + # BEGIN + # Note code below is to add organization_request feature request = HttpRequest() request.user = self has_organization_members_flag = flag_is_active(request, "organization_members") if not has_organization_members_flag: return False - ## END + # END return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MEMBERS) def has_edit_members_portfolio_permission(self, portfolio): - ## BEGIN - ## Note code below is to add organization_request feature + # BEGIN + # Note code below is to add organization_request feature request = HttpRequest() request.user = self has_organization_members_flag = flag_is_active(request, "organization_members") if not has_organization_members_flag: return False - ## END + # END return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_MEMBERS) def has_view_all_domains_permission(self, portfolio): diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index c5d1a9830..807c66cf7 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -230,6 +230,7 @@ class TestPortfolio(WebTest): self.assertContains(response, 'for="id_city"') @less_console_noise_decorator + @override_flag("organization_requests", active=True) def test_accessible_pages_when_user_does_not_have_permission(self): """Tests which pages are accessible when user does not have portfolio permissions""" self.app.set_user(self.user.username) @@ -280,6 +281,7 @@ class TestPortfolio(WebTest): self.assertEquals(domain_request_page.status_code, 403) @less_console_noise_decorator + @override_flag("organization_requests", active=True) def test_accessible_pages_when_user_does_not_have_role(self): """Test that admin / memmber roles are associated with the right access""" self.app.set_user(self.user.username) @@ -532,3 +534,99 @@ class TestPortfolio(WebTest): self.assertEqual(response.status_code, 200) self.assertContains(response, "Domain name") permission.delete() + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_requests", active=False) + def test_organization_requests_waffle_flag_off_hides_nav_link_and_restricts_permission(self): + """Setting the organization_requests waffle off hides the nav link and restricts access to the requests page""" + self.app.set_user(self.user.username) + + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS, + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.EDIT_REQUESTS, + ], + ) + + home = self.app.get(reverse("home")).follow() + + self.assertContains(home, "Hotel California") + self.assertNotContains(home, "Domain requests") + + domain_requests = self.app.get(reverse("domain-requests"), expect_errors=True) + self.assertEqual(domain_requests.status_code, 403) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_requests", active=True) + def test_organization_requests_waffle_flag_on_shows_nav_link_and_allows_permission(self): + """Setting the organization_requests waffle on shows the nav link and allows access to the requests page""" + self.app.set_user(self.user.username) + + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS, + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.EDIT_REQUESTS, + ], + ) + + home = self.app.get(reverse("home")).follow() + + self.assertContains(home, "Hotel California") + self.assertContains(home, "Domain requests") + + domain_requests = self.app.get(reverse("domain-requests")) + self.assertEqual(domain_requests.status_code, 200) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=False) + def test_organization_members_waffle_flag_off_hides_nav_link(self): + """Setting the organization_members waffle off hides the nav link""" + self.app.set_user(self.user.username) + + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS, + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.EDIT_REQUESTS, + ], + ) + + home = self.app.get(reverse("home")).follow() + + self.assertContains(home, "Hotel California") + self.assertNotContains(home, "Members") + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_organization_members_waffle_flag_on_shows_nav_link(self): + """Setting the organization_members waffle on shows the nav link""" + self.app.set_user(self.user.username) + + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_MEMBERS, + ], + ) + + home = self.app.get(reverse("home")).follow() + + self.assertContains(home, "Hotel California") + self.assertContains(home, "Members") diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index 734a6dbfe..e7031cf0d 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -237,4 +237,4 @@ class PortfolioMembersPermissionView(PortfolioMembersPermission, PortfolioBasePe This abstract view cannot be instantiated. Actual views must specify `template_name`. - """ \ No newline at end of file + """ From 684cf181c9852672a14eae7210eadc2051747ec9 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 4 Sep 2024 13:52:30 -0600 Subject: [PATCH 61/69] fixed sort button issue (thank you Rachid and Zander for the peer programming) --- src/registrar/templates/includes/domains_table.html | 2 +- src/registrar/views/domains_json.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index 48de2d98c..4112906eb 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -158,7 +158,7 @@ Expires Status {% if portfolio and has_view_suborganization %} - Suborganization + Suborganization {% endif %} Date: Wed, 4 Sep 2024 14:23:34 -0600 Subject: [PATCH 62/69] (missed a file in last push) --- src/registrar/assets/js/get-gov.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 70659b009..3913a3360 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1220,7 +1220,7 @@ document.addEventListener('DOMContentLoaded', function() { const expirationDateFormatted = expirationDate ? expirationDate.toLocaleDateString('en-US', options) : ''; const expirationDateSortValue = expirationDate ? expirationDate.getTime() : ''; const actionUrl = domain.action_url; - const suborganization = domain.suborganization ? domain.suborganization : ''; + const suborganization = domain.domain_info__sub_organization ? domain.domain_info__sub_organization : ''; const row = document.createElement('tr'); From e5f2b83f6a34b200dcb9c6f6545bf620be31f6e9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 4 Sep 2024 15:34:54 -0600 Subject: [PATCH 63/69] Fix failing tests --- ...rtfolio_additional_permissions_and_more.py | 66 +++++++++++++++++++ src/registrar/tests/test_models.py | 6 +- 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 src/registrar/migrations/0123_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py diff --git a/src/registrar/migrations/0123_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py b/src/registrar/migrations/0123_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py new file mode 100644 index 000000000..c14a70ab0 --- /dev/null +++ b/src/registrar/migrations/0123_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py @@ -0,0 +1,66 @@ +# Generated by Django 4.2.10 on 2024-09-04 21:29 + +import django.contrib.postgres.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0122_create_groups_v16"), + ] + + operations = [ + migrations.AlterField( + model_name="portfolioinvitation", + name="portfolio_additional_permissions", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("view_all_domains", "View all domains and domain reports"), + ("view_managed_domains", "View managed domains"), + ("view_members", "View members"), + ("edit_members", "Create and edit members"), + ("view_all_requests", "View all requests"), + ("view_created_requests", "View created requests"), + ("edit_requests", "Create and edit requests"), + ("view_portfolio", "View organization"), + ("edit_portfolio", "Edit organization"), + ("view_suborganization", "View suborganization"), + ("edit_suborganization", "Edit suborganization"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more additional permissions.", + null=True, + size=None, + ), + ), + migrations.AlterField( + model_name="userportfoliopermission", + name="additional_permissions", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("view_all_domains", "View all domains and domain reports"), + ("view_managed_domains", "View managed domains"), + ("view_members", "View members"), + ("edit_members", "Create and edit members"), + ("view_all_requests", "View all requests"), + ("view_created_requests", "View created requests"), + ("edit_requests", "Create and edit requests"), + ("view_portfolio", "View organization"), + ("edit_portfolio", "Edit organization"), + ("view_suborganization", "View suborganization"), + ("edit_suborganization", "Edit suborganization"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more additional permissions.", + null=True, + size=None, + ), + ), + ] diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 3a1b21ee2..d0a3e0462 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1474,6 +1474,7 @@ class TestUser(TestCase): self.assertFalse(self.user.has_contact_info()) @less_console_noise_decorator + @override_flag("organization_requests", active=True) def test_has_portfolio_permission(self): """ 0. Returns False when user does not have a permission @@ -1495,7 +1496,10 @@ class TestUser(TestCase): portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( portfolio=portfolio, user=self.user, - additional_permissions=[UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS + ], ) user_can_view_all_domains = self.user.has_domains_portfolio_permission(portfolio) From a6b9d79fad37af6173f89942df95dd48d2df09ba Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 5 Sep 2024 12:07:57 -0400 Subject: [PATCH 64/69] cleanup comments --- src/registrar/assets/js/get-gov.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 7b676eeb3..e6b5a039f 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -2044,17 +2044,9 @@ document.addEventListener('DOMContentLoaded', function() { // Use a MutationObserver to watch for changes in the dropdown list const dropdownList = comboBox.querySelector(`#${input.id}--list`); - // TODO: customize blank value - // const isSuborgComboBox = comboBox.querySelector('#id_sub_organization--list'); const observer = new MutationObserver(function(mutations) { mutations.forEach(function(mutation) { if (mutation.type === "childList") { - // TODO: customize blank value - // if (isSuborgComboBox) { - // addBlankOption(clearInputButton, dropdownList, initialValue, 'No suborganization'); - // } else { - // addBlankOption(clearInputButton, dropdownList, initialValue); - // } addBlankOption(clearInputButton, dropdownList, initialValue); } }); @@ -2110,8 +2102,6 @@ document.addEventListener('DOMContentLoaded', function() { } } - // TODO: customize blank value - // function addBlankOption(clearInputButton, dropdownList, initialValue, customBlank) { function addBlankOption(clearInputButton, dropdownList, initialValue) { if (dropdownList && !dropdownList.querySelector('[data-value=""]') && !isTyping) { const blankOption = document.createElement("li"); @@ -2121,8 +2111,6 @@ document.addEventListener('DOMContentLoaded', function() { if (!initialValue){ blankOption.classList.add("usa-combo-box__list-option--selected") } - // TODO: customize blank value - // customBlank ? blankOption.textContent = customBlank : blankOption.textContent = "---------"; blankOption.textContent = "⎯"; dropdownList.insertBefore(blankOption, dropdownList.firstChild); From 49110461ef542f3a230f28cb3e11f32597f73cdc Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 5 Sep 2024 12:10:49 -0400 Subject: [PATCH 65/69] lint --- src/registrar/tests/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index d0a3e0462..827f349b9 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1498,7 +1498,7 @@ class TestUser(TestCase): user=self.user, additional_permissions=[ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, ], ) From a2291d75ed149b6836cd1335da7fe4c3bdc3336a Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 6 Sep 2024 10:27:52 -0500 Subject: [PATCH 66/69] doc fixes --- docs/operations/data_migration.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 90080520b..4301ca878 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -834,10 +834,10 @@ Example: `cf ssh getgov-za` ```/tmp/lifecycle/shell``` #### Step 4: Running the script -```./manage.py populate_domain_request_dates``` +```./manage.py update_first_ready``` ### Running locally -```docker-compose exec app ./manage.py populate_domain_request_dates``` +```docker-compose exec app ./manage.py update_first_ready``` ## Populate Domain Request Dates This section outlines how to run the populate_domain_request_dates script From db593fcb6e7a9022ed73802b529f2a8050fcee5a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 6 Sep 2024 13:57:28 -0400 Subject: [PATCH 67/69] merge cleanup --- src/registrar/tests/test_admin.py | 175 +++++++++++++++--------------- 1 file changed, 88 insertions(+), 87 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 93942e438..25d7e5fd2 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2122,6 +2122,94 @@ class TestPortfolioAdmin(TestCase): domain_requests = self.admin.domain_requests(self.portfolio) self.assertIn("2 domain requests", domain_requests) + @less_console_noise_decorator + def test_portfolio_members_display(self): + """Tests the custom portfolio members field, admin and member sections""" + admin_user_1 = User.objects.create( + username="testuser1", + first_name="Gerald", + last_name="Meoward", + title="Captain", + email="meaoward@gov.gov", + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_1, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + admin_user_2 = User.objects.create( + username="testuser2", + first_name="Arnold", + last_name="Poopy", + title="Major", + email="poopy@gov.gov", + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_2, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + admin_user_3 = User.objects.create( + username="testuser3", + first_name="Mad", + last_name="Max", + title="Road warrior", + email="madmax@gov.gov", + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_3, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + ) + + admin_user_4 = User.objects.create( + username="testuser4", + first_name="Agent", + last_name="Smith", + title="Program", + email="thematrix@gov.gov", + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_4, + portfolio=self.portfolio, + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.EDIT_REQUESTS, + ], + ) + + display_admins = self.admin.display_admins(self.portfolio) + + self.assertIn( + f'Gerald Meoward meaoward@gov.gov', + display_admins, + ) + self.assertIn("Captain", display_admins) + self.assertIn( + f'Arnold Poopy poopy@gov.gov', display_admins + ) + self.assertIn("Major", display_admins) + + display_members_summary = self.admin.display_members_summary(self.portfolio) + + self.assertIn( + f'Mad Max madmax@gov.gov', + display_members_summary, + ) + self.assertIn( + f'Agent Smith thematrix@gov.gov', + display_members_summary, + ) + + display_members = self.admin.display_members(self.portfolio) + + self.assertIn("Mad Max", display_members) + self.assertIn("Member", display_members) + self.assertIn("Road warrior", display_members) + self.assertIn("Agent Smith", display_members) + self.assertIn("Domain requestor", display_members) + self.assertIn("Program", display_members) + class TestTransferUser(WebTest): """User transfer custom admin page""" @@ -2340,90 +2428,3 @@ class TestTransferUser(WebTest): """Assert modal on page""" user_transfer_page = self.app.get(reverse("transfer_user", args=[self.user1.pk])) self.assertContains(user_transfer_page, "This action cannot be undone.") - @less_console_noise_decorator - def test_portfolio_members_display(self): - """Tests the custom portfolio members field, admin and member sections""" - admin_user_1 = User.objects.create( - username="testuser1", - first_name="Gerald", - last_name="Meoward", - title="Captain", - email="meaoward@gov.gov", - ) - - UserPortfolioPermission.objects.all().create( - user=admin_user_1, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - - admin_user_2 = User.objects.create( - username="testuser2", - first_name="Arnold", - last_name="Poopy", - title="Major", - email="poopy@gov.gov", - ) - - UserPortfolioPermission.objects.all().create( - user=admin_user_2, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - - admin_user_3 = User.objects.create( - username="testuser3", - first_name="Mad", - last_name="Max", - title="Road warrior", - email="madmax@gov.gov", - ) - - UserPortfolioPermission.objects.all().create( - user=admin_user_3, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] - ) - - admin_user_4 = User.objects.create( - username="testuser4", - first_name="Agent", - last_name="Smith", - title="Program", - email="thematrix@gov.gov", - ) - - UserPortfolioPermission.objects.all().create( - user=admin_user_4, - portfolio=self.portfolio, - additional_permissions=[ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.EDIT_REQUESTS, - ], - ) - - display_admins = self.admin.display_admins(self.portfolio) - - self.assertIn( - f'Gerald Meoward meaoward@gov.gov', - display_admins, - ) - self.assertIn("Captain", display_admins) - self.assertIn( - f'Arnold Poopy poopy@gov.gov', display_admins - ) - self.assertIn("Major", display_admins) - - display_members_summary = self.admin.display_members_summary(self.portfolio) - - self.assertIn( - f'Mad Max madmax@gov.gov', - display_members_summary, - ) - self.assertIn( - f'Agent Smith thematrix@gov.gov', - display_members_summary, - ) - - display_members = self.admin.display_members(self.portfolio) - - self.assertIn("Mad Max", display_members) - self.assertIn("Member", display_members) - self.assertIn("Road warrior", display_members) - self.assertIn("Agent Smith", display_members) - self.assertIn("Domain requestor", display_members) - self.assertIn("Program", display_members) From ffaf3b5a375364fc6e78977bc8600d5b00747ede Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 9 Sep 2024 10:00:34 -0500 Subject: [PATCH 68/69] fix linter errors --- src/registrar/management/commands/update_first_ready.py | 2 +- src/registrar/management/commands/utility/terminal_helper.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/management/commands/update_first_ready.py b/src/registrar/management/commands/update_first_ready.py index f1ebdd555..22320a3d3 100644 --- a/src/registrar/management/commands/update_first_ready.py +++ b/src/registrar/management/commands/update_first_ready.py @@ -27,7 +27,7 @@ class Command(BaseCommand, PopulateScriptTemplate): # check if a transition domain object for this domain name exists, # or if so whether its first_ready value matches its created_at date - def custom_filter(self, records: BaseManager[Domain]) -> BaseManager[Domain]: + def custom_filter(self, records): to_include_pks = [] for record in records: if ( diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index e69d54c07..fa7cde683 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -78,7 +78,7 @@ class PopulateScriptTemplate(ABC): run_summary_header = None @abstractmethod - def update_record(self, record: Model): + def update_record(self, record): """Defines how we update each field. raises: @@ -86,7 +86,7 @@ class PopulateScriptTemplate(ABC): """ raise NotImplementedError - def mass_update_records(self, object_class: Model, filter_conditions, fields_to_update, debug=True, verbose=False): + def mass_update_records(self, object_class, filter_conditions, fields_to_update, debug=True, verbose=False): """Loops through each valid "object_class" object - specified by filter_conditions - and updates fields defined by fields_to_update using update_record. From 872f943220e43274a02cd202801de5b0bb5c825e Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 9 Sep 2024 10:10:04 -0500 Subject: [PATCH 69/69] unused import --- src/registrar/management/commands/update_first_ready.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/management/commands/update_first_ready.py b/src/registrar/management/commands/update_first_ready.py index 22320a3d3..0a4ea10a7 100644 --- a/src/registrar/management/commands/update_first_ready.py +++ b/src/registrar/management/commands/update_first_ready.py @@ -1,6 +1,5 @@ import logging from django.core.management import BaseCommand -from django.db.models.manager import BaseManager from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors from registrar.models import Domain, TransitionDomain