From ac826c9e053304cb1e6cfd76830ab554bda75691 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 19 Feb 2025 12:20:12 -0500 Subject: [PATCH 01/18] temp file to remove --- src/temp-remove | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 src/temp-remove diff --git a/src/temp-remove b/src/temp-remove new file mode 100644 index 000000000..e69de29bb From 260d2e587f05050ea948a15264fd961ddb6c8b49 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 21 Feb 2025 20:01:58 -0500 Subject: [PATCH 02/18] wip --- src/registrar/admin.py | 110 ++++++++++++++++-- .../models/utility/portfolio_helper.py | 16 +-- 2 files changed, 107 insertions(+), 19 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 2d2b90a5f..dbb6c54f7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -193,19 +193,107 @@ class MyUserAdminForm(UserChangeForm): class UserPortfolioPermissionsForm(forms.ModelForm): + REQUEST_PERMISSIONS = [ + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.EDIT_REQUESTS, + ] + + DOMAIN_PERMISSIONS = [ + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, + UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + ] + + MEMBER_PERMISSIONS = [ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + ] + + user = forms.ModelChoiceField( + queryset=models.User.objects.all(), + label="User" + ) + + portfolio = forms.ModelChoiceField( + queryset=models.Portfolio.objects.all(), + label="Portfolio" + ) + + role = forms.ChoiceField( + choices=UserPortfolioRoleChoices.choices, + required=True, + widget=forms.Select(attrs={"class": "admin-dropdown"}), + label="Member access" + ) + + request_permissions = forms.ChoiceField( + choices=[(perm.value, perm.label) for perm in REQUEST_PERMISSIONS], + required=False, + widget=forms.Select(attrs={"class": "admin-dropdown"}), + label="Domain requests" + ) + + domain_permissions = forms.ChoiceField( + choices=[(perm.value, perm.label) for perm in DOMAIN_PERMISSIONS], + required=False, + widget=forms.Select(attrs={"class": "admin-dropdown"}), + label="Domains" + ) + + member_permissions = forms.ChoiceField( + choices=[(perm.value, perm.label) for perm in MEMBER_PERMISSIONS], + required=False, + widget=forms.Select(attrs={"class": "admin-dropdown"}), + label="Members" + ) + class Meta: model = models.UserPortfolioPermission - fields = "__all__" - widgets = { - "roles": FilteredSelectMultipleArrayWidget( - "roles", is_stacked=False, choices=UserPortfolioRoleChoices.choices - ), - "additional_permissions": FilteredSelectMultipleArrayWidget( - "additional_permissions", - is_stacked=False, - choices=UserPortfolioPermissionChoices.choices, - ), - } + fields = ["user", "portfolio", "role", "domain_permissions", "request_permissions", "member_permissions"] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + logger.debug("Initializing form") + + # Populate roles + if self.instance and self.instance.pk: + if self.instance.roles: + logger.debug(f"Setting role: {self.instance.roles[0]}") + self.fields["role"].initial = self.instance.roles[0] # Assuming single role per user + + if self.instance.additional_permissions: + logger.debug(f"Existing permissions: {self.instance.additional_permissions}") + for perm in self.instance.additional_permissions: + logger.debug(f"Processing permission: {perm}") + if perm in self.REQUEST_PERMISSIONS: + logger.debug("Assigning request permission") + self.fields["request_permissions"].initial = perm + elif perm in self.DOMAIN_PERMISSIONS: + logger.debug("Assigning domain permission") + self.fields["domain_permissions"].initial = perm + elif perm in self.MEMBER_PERMISSIONS: + logger.debug("Assigning member permission") + self.fields["member_permissions"].initial = perm + + def clean(self): + cleaned_data = super().clean() + + self.instance.roles = [cleaned_data.get("role")] if cleaned_data.get("role") else [] + logger.debug(f"Cleaned roles: {self.instance.roles}") + + if self.instance.roles == [UserPortfolioRoleChoices.ORGANIZATION_MEMBER]: + self.instance.additional_permissions = list( + filter(None, [ + cleaned_data.get("request_permissions"), + cleaned_data.get("domain_permissions"), + cleaned_data.get("member_permissions"), + ]) + ) + else: + self.instance.additional_permissions = [] + + logger.debug(f"Final saved permissions: {self.instance.additional_permissions}") + + return cleaned_data class PortfolioInvitationAdminForm(UserChangeForm): diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 03733237e..041f3b2f3 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -16,7 +16,7 @@ class UserPortfolioRoleChoices(models.TextChoices): """ ORGANIZATION_ADMIN = "organization_admin", "Admin" - ORGANIZATION_MEMBER = "organization_member", "Member" + ORGANIZATION_MEMBER = "organization_member", "Basic" @classmethod def get_user_portfolio_role_label(cls, user_portfolio_role): @@ -30,17 +30,17 @@ class UserPortfolioRoleChoices(models.TextChoices): 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_ALL_DOMAINS = "view_all_domains", "Viewer" + VIEW_MANAGED_DOMAINS = "view_managed_domains", "Viewer, limited (domains they manage)" VIEW_MEMBERS = "view_members", "View members" - EDIT_MEMBERS = "edit_members", "Create and edit members" + EDIT_MEMBERS = "edit_members", "Manager" - VIEW_ALL_REQUESTS = "view_all_requests", "View all requests" - EDIT_REQUESTS = "edit_requests", "Create and edit requests" + VIEW_ALL_REQUESTS = "view_all_requests", "Viewer" + EDIT_REQUESTS = "edit_requests", "Creator" - VIEW_PORTFOLIO = "view_portfolio", "View organization" - EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" + VIEW_PORTFOLIO = "view_portfolio", "Viewer" + EDIT_PORTFOLIO = "edit_portfolio", "Manager" @classmethod def get_user_portfolio_permission_label(cls, user_portfolio_permission): From 3434519a1616defe0bb2a57436ca3267fdda83bf Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 21 Feb 2025 22:57:12 -0500 Subject: [PATCH 03/18] modified userportfoliopermissions and portfolioinvitation admin forms --- src/registrar/admin.py | 146 +++++++++++------- .../assets/src/js/getgov-admin/main.js | 4 + .../portfolio-permissions-form.js | 67 ++++++++ .../commands/disclose_security_emails.py | 2 +- .../commands/master_domain_migrations.py | 8 +- .../populate_domain_updated_federal_agency.py | 2 +- src/registrar/views/user_profile.py | 3 +- 7 files changed, 167 insertions(+), 65 deletions(-) create mode 100644 src/registrar/assets/src/js/getgov-admin/portfolio-permissions-form.js diff --git a/src/registrar/admin.py b/src/registrar/admin.py index dbb6c54f7..93d0673f0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -192,126 +192,158 @@ class MyUserAdminForm(UserChangeForm): ) -class UserPortfolioPermissionsForm(forms.ModelForm): +class PortfolioPermissionsForm(forms.ModelForm): + """ + Form for managing portfolio permissions in Django admin. This form class is used + for both UserPortfolioPermission and PortfolioInvitation models. + + Allows selecting a portfolio, assigning a role, and managing specific permissions + related to requests, domains, and members. + """ + + # Define available permissions for requests, domains, and members REQUEST_PERMISSIONS = [ UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.EDIT_REQUESTS, ] - + DOMAIN_PERMISSIONS = [ UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, ] - + MEMBER_PERMISSIONS = [ UserPortfolioPermissionChoices.VIEW_MEMBERS, ] - user = forms.ModelChoiceField( - queryset=models.User.objects.all(), - label="User" - ) - - portfolio = forms.ModelChoiceField( - queryset=models.Portfolio.objects.all(), - label="Portfolio" - ) + # Dropdown to select a portfolio + portfolio = forms.ModelChoiceField(queryset=models.Portfolio.objects.all(), label="Portfolio") + # Dropdown for selecting the user role (e.g., Admin or Basic) role = forms.ChoiceField( choices=UserPortfolioRoleChoices.choices, required=True, widget=forms.Select(attrs={"class": "admin-dropdown"}), - label="Member access" + label="Member access", ) + # Dropdown for selecting request permissions, with a default "No access" option request_permissions = forms.ChoiceField( - choices=[(perm.value, perm.label) for perm in REQUEST_PERMISSIONS], + choices=[(None, "No access")] + [(perm.value, perm.label) for perm in REQUEST_PERMISSIONS], required=False, widget=forms.Select(attrs={"class": "admin-dropdown"}), - label="Domain requests" + label="Domain requests", ) + # Dropdown for selecting domain permissions domain_permissions = forms.ChoiceField( choices=[(perm.value, perm.label) for perm in DOMAIN_PERMISSIONS], required=False, widget=forms.Select(attrs={"class": "admin-dropdown"}), - label="Domains" + label="Domains", ) + # Dropdown for selecting member permissions, with a default "No access" option member_permissions = forms.ChoiceField( - choices=[(perm.value, perm.label) for perm in MEMBER_PERMISSIONS], + choices=[(None, "No access")] + [(perm.value, perm.label) for perm in MEMBER_PERMISSIONS], required=False, widget=forms.Select(attrs={"class": "admin-dropdown"}), - label="Members" + label="Members", ) - class Meta: - model = models.UserPortfolioPermission - fields = ["user", "portfolio", "role", "domain_permissions", "request_permissions", "member_permissions"] - def __init__(self, *args, **kwargs): + """ + Initialize the form and set default values based on the existing instance. + """ super().__init__(*args, **kwargs) - logger.debug("Initializing form") - - # Populate roles + # If an instance exists, populate the form fields with existing data if self.instance and self.instance.pk: + # Set the initial value for the role field if self.instance.roles: - logger.debug(f"Setting role: {self.instance.roles[0]}") - self.fields["role"].initial = self.instance.roles[0] # Assuming single role per user + self.fields["role"].initial = self.instance.roles[0] # Assuming a single role per user + # Set the initial values for permissions based on the instance data if self.instance.additional_permissions: - logger.debug(f"Existing permissions: {self.instance.additional_permissions}") for perm in self.instance.additional_permissions: - logger.debug(f"Processing permission: {perm}") if perm in self.REQUEST_PERMISSIONS: - logger.debug("Assigning request permission") self.fields["request_permissions"].initial = perm elif perm in self.DOMAIN_PERMISSIONS: - logger.debug("Assigning domain permission") self.fields["domain_permissions"].initial = perm elif perm in self.MEMBER_PERMISSIONS: - logger.debug("Assigning member permission") self.fields["member_permissions"].initial = perm def clean(self): + """ + Custom validation and processing of form data before saving. + """ cleaned_data = super().clean() - - self.instance.roles = [cleaned_data.get("role")] if cleaned_data.get("role") else [] - logger.debug(f"Cleaned roles: {self.instance.roles}") + # Store the selected role as a list (assuming single role assignment) + self.instance.roles = [cleaned_data.get("role")] if cleaned_data.get("role") else [] + + # If the selected role is "organization_member," store additional permissions if self.instance.roles == [UserPortfolioRoleChoices.ORGANIZATION_MEMBER]: self.instance.additional_permissions = list( - filter(None, [ - cleaned_data.get("request_permissions"), - cleaned_data.get("domain_permissions"), - cleaned_data.get("member_permissions"), - ]) + filter( + None, + [ + cleaned_data.get("request_permissions"), + cleaned_data.get("domain_permissions"), + cleaned_data.get("member_permissions"), + ], + ) ) else: + # If the user is an admin, clear any additional permissions self.instance.additional_permissions = [] - - logger.debug(f"Final saved permissions: {self.instance.additional_permissions}") return cleaned_data -class PortfolioInvitationAdminForm(UserChangeForm): - """This form utilizes the custom widget for its class's ManyToMany UIs.""" +class UserPortfolioPermissionsForm(PortfolioPermissionsForm): + """ + Form for managing user portfolio permissions in Django admin. + + Extends PortfolioPermissionsForm to include a user field, allowing administrators + to assign roles and permissions to specific users within a portfolio. + """ + + # Dropdown to select a user from the database + user = forms.ModelChoiceField(queryset=models.User.objects.all(), label="User") class Meta: - model = models.PortfolioInvitation - fields = "__all__" - widgets = { - "roles": FilteredSelectMultipleArrayWidget( - "roles", is_stacked=False, choices=UserPortfolioRoleChoices.choices - ), - "additional_permissions": FilteredSelectMultipleArrayWidget( - "additional_permissions", - is_stacked=False, - choices=UserPortfolioPermissionChoices.choices, - ), - } + """ + Meta class defining the model and fields to be used in the form. + """ + + model = models.UserPortfolioPermission # Uses the UserPortfolioPermission model + fields = ["user", "portfolio", "role", "domain_permissions", "request_permissions", "member_permissions"] + + +class PortfolioInvitationForm(PortfolioPermissionsForm): + """ + Form for sending portfolio invitations in Django admin. + + Extends PortfolioPermissionsForm to include an email field for inviting users, + allowing them to be assigned a role and permissions within a portfolio before they join. + """ + + class Meta: + """ + Meta class defining the model and fields to be used in the form. + """ + + model = models.PortfolioInvitation # Uses the PortfolioInvitation model + fields = [ + "email", + "portfolio", + "role", + "domain_permissions", + "request_permissions", + "member_permissions", + "status", + ] class DomainInformationAdminForm(forms.ModelForm): @@ -1721,7 +1753,7 @@ class DomainInvitationAdmin(BaseInvitationAdmin): class PortfolioInvitationAdmin(BaseInvitationAdmin): """Custom portfolio invitation admin class.""" - form = PortfolioInvitationAdminForm + form = PortfolioInvitationForm class Meta: model = models.PortfolioInvitation diff --git a/src/registrar/assets/src/js/getgov-admin/main.js b/src/registrar/assets/src/js/getgov-admin/main.js index 5c6de20ab..3bde1e5bb 100644 --- a/src/registrar/assets/src/js/getgov-admin/main.js +++ b/src/registrar/assets/src/js/getgov-admin/main.js @@ -13,6 +13,7 @@ import { initDynamicDomainRequestFields } from './domain-request-form.js'; import { initDomainFormTargetBlankButtons } from './domain-form.js'; import { initDynamicPortfolioFields } from './portfolio-form.js'; +import { initDynamicPortfolioPermissionFields } from './portfolio-permissions-form.js' import { initDynamicDomainInformationFields } from './domain-information-form.js'; import { initDynamicDomainFields } from './domain-form.js'; import { initAnalyticsDashboard } from './analytics.js'; @@ -40,6 +41,9 @@ initDynamicDomainFields(); // Portfolio initDynamicPortfolioFields(); +// Portfolio permissions +initDynamicPortfolioPermissionFields(); + // Domain information initDynamicDomainInformationFields(); diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-permissions-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-permissions-form.js new file mode 100644 index 000000000..09e11c5d0 --- /dev/null +++ b/src/registrar/assets/src/js/getgov-admin/portfolio-permissions-form.js @@ -0,0 +1,67 @@ +import { hideElement, showElement } from './helpers-admin.js'; + +/** + * A function for dynamically changing fields on the UserPortfolioPermissions + * and PortfolioInvitation admin forms + */ +function handlePortfolioPermissionFields(){ + + const roleDropdown = document.getElementById("id_role"); + const domainPermissionsField = document.querySelector(".field-domain_permissions"); + const domainRequestPermissionsField = document.querySelector(".field-request_permissions"); + const memberPermissionsField = document.querySelector(".field-member_permissions"); + + /** + * Updates the visibility of portfolio permissions fields based on the selected role. + * + * This function checks the value of the role dropdown (`roleDropdown`): + * - If the selected role is "organization_admin": + * - Hides the domain permissions field (`domainPermissionsField`). + * - Hides the domain request permissions field (`domainRequestPermissionsField`). + * - Hides the member permissions field (`memberPermissionsField`). + * - Otherwise: + * - Shows all the above fields. + * + * The function ensures that the appropriate fields are dynamically displayed + * or hidden depending on the role selection in the form. + */ + function updatePortfolioPermissionsFormVisibility() { + if (roleDropdown && domainPermissionsField && domainRequestPermissionsField && memberPermissionsField) { + if (roleDropdown.value === "organization_admin") { + hideElement(domainPermissionsField); + hideElement(domainRequestPermissionsField); + hideElement(memberPermissionsField); + } else { + showElement(domainPermissionsField); + showElement(domainRequestPermissionsField); + showElement(memberPermissionsField); + } + } + } + + + /** + * Sets event listeners for key UI elements. + */ + function setEventListeners() { + if (roleDropdown) { + roleDropdown.addEventListener("change", function() { + updatePortfolioPermissionsFormVisibility(); + }) + } + } + + // Run initial setup functions + updatePortfolioPermissionsFormVisibility(); + setEventListeners(); +} + +export function initDynamicPortfolioPermissionFields() { + document.addEventListener('DOMContentLoaded', function() { + let isPortfolioPermissionPage = document.getElementById("userportfoliopermission_form"); + let isPortfolioInvitationPage = document.getElementById("portfolioinvitation_form") + if (isPortfolioPermissionPage || isPortfolioInvitationPage) { + handlePortfolioPermissionFields(); + } + }); +} diff --git a/src/registrar/management/commands/disclose_security_emails.py b/src/registrar/management/commands/disclose_security_emails.py index 62989e4c0..15c0bb750 100644 --- a/src/registrar/management/commands/disclose_security_emails.py +++ b/src/registrar/management/commands/disclose_security_emails.py @@ -1,4 +1,4 @@ -"""" +""" Converts all ready and DNS needed domains with a non-default public contact to disclose their public contact. Created for Issue#1535 to resolve disclose issue of domains with missing security emails. diff --git a/src/registrar/management/commands/master_domain_migrations.py b/src/registrar/management/commands/master_domain_migrations.py index 7f702e047..2907add06 100644 --- a/src/registrar/management/commands/master_domain_migrations.py +++ b/src/registrar/management/commands/master_domain_migrations.py @@ -1,8 +1,8 @@ """Data migration: - 1 - generates a report of data integrity across all - transition domain related tables - 2 - allows users to run all migration scripts for - transition domain data +1 - generates a report of data integrity across all +transition domain related tables +2 - allows users to run all migration scripts for +transition domain data """ import logging diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index dd8ceb3b2..1c7d1e980 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -1,4 +1,4 @@ -"""" +""" Data migration: Renaming deprecated Federal Agencies to their new updated names ie (U.S. Peace Corps to Peace Corps) within Domain Information and Domain Requests diff --git a/src/registrar/views/user_profile.py b/src/registrar/views/user_profile.py index 2012d12ab..41584bc82 100644 --- a/src/registrar/views/user_profile.py +++ b/src/registrar/views/user_profile.py @@ -1,5 +1,4 @@ -"""Views for a User Profile. -""" +"""Views for a User Profile.""" import logging From 5ffce473b2478eceb17cadf3906ee2ae955a9446 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 21 Feb 2025 23:10:14 -0500 Subject: [PATCH 04/18] migrations --- ...itation_additional_permissions_and_more.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 src/registrar/migrations/0141_alter_portfolioinvitation_additional_permissions_and_more.py diff --git a/src/registrar/migrations/0141_alter_portfolioinvitation_additional_permissions_and_more.py b/src/registrar/migrations/0141_alter_portfolioinvitation_additional_permissions_and_more.py new file mode 100644 index 000000000..6c0c9d942 --- /dev/null +++ b/src/registrar/migrations/0141_alter_portfolioinvitation_additional_permissions_and_more.py @@ -0,0 +1,86 @@ +# Generated by Django 4.2.17 on 2025-02-22 04:09 + +import django.contrib.postgres.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0140_alter_portfolioinvitation_additional_permissions_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="portfolioinvitation", + name="additional_permissions", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("view_all_domains", "Viewer"), + ("view_managed_domains", "Viewer, limited (domains they manage)"), + ("view_members", "View members"), + ("edit_members", "Manager"), + ("view_all_requests", "Viewer"), + ("edit_requests", "Creator"), + ("view_portfolio", "Viewer"), + ("edit_portfolio", "Manager"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more additional permissions.", + null=True, + size=None, + ), + ), + migrations.AlterField( + model_name="portfolioinvitation", + name="roles", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[("organization_admin", "Admin"), ("organization_member", "Basic")], max_length=50 + ), + blank=True, + help_text="Select one or more roles.", + 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", "Viewer"), + ("view_managed_domains", "Viewer, limited (domains they manage)"), + ("view_members", "View members"), + ("edit_members", "Manager"), + ("view_all_requests", "Viewer"), + ("edit_requests", "Creator"), + ("view_portfolio", "Viewer"), + ("edit_portfolio", "Manager"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more additional permissions.", + null=True, + size=None, + ), + ), + migrations.AlterField( + model_name="userportfoliopermission", + name="roles", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[("organization_admin", "Admin"), ("organization_member", "Basic")], max_length=50 + ), + blank=True, + help_text="Select one or more roles.", + null=True, + size=None, + ), + ), + ] From ab12e9f27dc7c482dcdc3998fe92b5b64391cf95 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 22 Feb 2025 00:22:56 -0500 Subject: [PATCH 05/18] tests --- src/registrar/admin.py | 2 + src/registrar/tests/test_admin.py | 139 ++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c662f91d7..c0a1f1c35 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -281,6 +281,7 @@ class PortfolioPermissionsForm(forms.ModelForm): # Store the selected role as a list (assuming single role assignment) self.instance.roles = [cleaned_data.get("role")] if cleaned_data.get("role") else [] + cleaned_data["roles"] = self.instance.roles # If the selected role is "organization_member," store additional permissions if self.instance.roles == [UserPortfolioRoleChoices.ORGANIZATION_MEMBER]: @@ -297,6 +298,7 @@ class PortfolioPermissionsForm(forms.ModelForm): else: # If the user is an admin, clear any additional permissions self.instance.additional_permissions = [] + cleaned_data["additional_permissions"] = self.instance.additional_permissions return cleaned_data diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index ccd0e6cc2..7a8a9b783 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2,6 +2,7 @@ from datetime import datetime from django.utils import timezone from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite +from registrar import models from registrar.utility.email import EmailSendingError from registrar.utility.errors import MissingEmailError from waffle.testutils import override_flag @@ -19,6 +20,7 @@ from registrar.admin import ( MyHostAdmin, PortfolioInvitationAdmin, UserDomainRoleAdmin, + UserPortfolioPermissionsForm, VerifiedByStaffAdmin, FsmModelResource, WebsiteAdmin, @@ -1638,6 +1640,143 @@ class TestPortfolioInvitationAdmin(TestCase): self.assertIn(expected_message, response.content.decode("utf-8")) +class PortfolioPermissionsFormTest(TestCase): + + def setUp(self): + # Create a mock portfolio for testing + self.user = create_test_user() + self.portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test Portfolio", creator=self.user) + + def tearDown(self): + UserPortfolioPermission.objects.all().delete() + Portfolio.objects.all().delete() + User.objects.all().delete() + + def test_form_valid_with_required_fields(self): + """Test that the form is valid when required fields are filled correctly.""" + # Mock the instance or use a test instance + test_instance = models.UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + ], + ) + form_data = { + "portfolio": self.portfolio.id, + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER, + "request_permissions": "view_all_requests", + "domain_permissions": "view_all_domains", + "member_permissions": "view_members", + "user": self.user.id, + } + form = UserPortfolioPermissionsForm(data=form_data, instance=test_instance) + self.assertTrue(form.is_valid()) + + def test_form_invalid_without_role(self): + """Test that the form is invalid if role is missing.""" + # Mock the instance or use a test instance + test_instance = models.UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + ], + ) + form_data = { + "portfolio": self.portfolio.id, + "role": "", # Missing role + } + form = UserPortfolioPermissionsForm(data=form_data, instance=test_instance) + self.assertFalse(form.is_valid()) + self.assertIn("role", form.errors) + + def test_member_role_preserves_permissions(self): + """Ensure that selecting 'organization_member' keeps the additional permissions.""" + # Mock the instance or use a test instance + test_instance = models.UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + ], + ) + form_data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER, + "request_permissions": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + "domain_permissions": UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + "member_permissions": UserPortfolioPermissionChoices.VIEW_MEMBERS, + "portfolio": self.portfolio.id, + "user": self.user.id, + } + form = UserPortfolioPermissionsForm(data=form_data, instance=test_instance) + + # Check if form is valid + self.assertTrue(form.is_valid()) + + # Test if permissions are correctly preserved + cleaned_data = form.cleaned_data + self.assertIn(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, cleaned_data["request_permissions"]) + self.assertIn(UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, cleaned_data["domain_permissions"]) + + def test_admin_role_clears_permissions(self): + """Ensure that selecting 'organization_admin' clears additional permissions.""" + # Mock the instance or use a test instance + test_instance = models.UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + ], + ) + form_data = { + "portfolio": self.portfolio.id, + "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + "request_permissions": "view_all_requests", + "domain_permissions": "view_all_domains", + "member_permissions": "view_members", + "user": self.user.id, + } + form = UserPortfolioPermissionsForm(data=form_data, instance=test_instance) + self.assertTrue(form.is_valid()) + + # Simulate form save to check cleaned data behavior + cleaned_data = form.clean() + self.assertEqual(cleaned_data["role"], UserPortfolioRoleChoices.ORGANIZATION_ADMIN) + self.assertNotIn("request_permissions", cleaned_data["additional_permissions"]) # Permissions should be removed + self.assertNotIn("domain_permissions", cleaned_data["additional_permissions"]) + self.assertNotIn("member_permissions", cleaned_data["additional_permissions"]) + + def test_invalid_permission_choice(self): + """Ensure invalid permissions are not accepted.""" + # Mock the instance or use a test instance + test_instance = models.UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + ], + ) + form_data = { + "portfolio": self.portfolio.id, + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER, + "request_permissions": "invalid_permission", # Invalid choice + } + form = UserPortfolioPermissionsForm(data=form_data, instance=test_instance) + self.assertFalse(form.is_valid()) + self.assertIn("request_permissions", form.errors) + + class TestHostAdmin(TestCase): """Tests for the HostAdmin class as super user From 28b158d424e774de1af22d45298a3d558e0f7881 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 24 Feb 2025 16:05:18 -0500 Subject: [PATCH 06/18] ordered domain privilege options properly --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c0a1f1c35..93cf4b096 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -208,8 +208,8 @@ class PortfolioPermissionsForm(forms.ModelForm): ] DOMAIN_PERMISSIONS = [ - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, ] MEMBER_PERMISSIONS = [ From c3a106ce7ba8065e719b4a361743c2e7e5f4612a Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 28 Feb 2025 12:12:57 -0500 Subject: [PATCH 07/18] View members => Viewer --- ...r_portfolioinvitation_additional_permissions_and_more.py | 6 +++--- src/registrar/models/utility/portfolio_helper.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/migrations/0141_alter_portfolioinvitation_additional_permissions_and_more.py b/src/registrar/migrations/0141_alter_portfolioinvitation_additional_permissions_and_more.py index 6c0c9d942..c100f04b2 100644 --- a/src/registrar/migrations/0141_alter_portfolioinvitation_additional_permissions_and_more.py +++ b/src/registrar/migrations/0141_alter_portfolioinvitation_additional_permissions_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.17 on 2025-02-22 04:09 +# Generated by Django 4.2.17 on 2025-02-28 17:11 import django.contrib.postgres.fields from django.db import migrations, models @@ -19,7 +19,7 @@ class Migration(migrations.Migration): choices=[ ("view_all_domains", "Viewer"), ("view_managed_domains", "Viewer, limited (domains they manage)"), - ("view_members", "View members"), + ("view_members", "Viewer"), ("edit_members", "Manager"), ("view_all_requests", "Viewer"), ("edit_requests", "Creator"), @@ -55,7 +55,7 @@ class Migration(migrations.Migration): choices=[ ("view_all_domains", "Viewer"), ("view_managed_domains", "Viewer, limited (domains they manage)"), - ("view_members", "View members"), + ("view_members", "Viewer"), ("edit_members", "Manager"), ("view_all_requests", "Viewer"), ("edit_requests", "Creator"), diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 6863af34e..679ad7b9b 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -33,7 +33,7 @@ class UserPortfolioPermissionChoices(models.TextChoices): VIEW_ALL_DOMAINS = "view_all_domains", "Viewer" VIEW_MANAGED_DOMAINS = "view_managed_domains", "Viewer, limited (domains they manage)" - VIEW_MEMBERS = "view_members", "View members" + VIEW_MEMBERS = "view_members", "Viewer" EDIT_MEMBERS = "edit_members", "Manager" VIEW_ALL_REQUESTS = "view_all_requests", "Viewer" From aa7e9d83a46104cf48fd4cacdfe914690d56b7ba Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 28 Feb 2025 13:57:18 -0500 Subject: [PATCH 08/18] combo boxes for portfolio and user --- src/registrar/admin.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 93cf4b096..3352a5b81 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -217,7 +217,15 @@ class PortfolioPermissionsForm(forms.ModelForm): ] # Dropdown to select a portfolio - portfolio = forms.ModelChoiceField(queryset=models.Portfolio.objects.all(), label="Portfolio") + portfolio = forms.ModelChoiceField( + queryset=models.Portfolio.objects.all(), + label="Portfolio", + widget=AutocompleteSelectWithPlaceholder( + models.PortfolioInvitation._meta.get_field("portfolio"), + admin.site, + attrs={"data-placeholder": "---------"} # Customize placeholder + ), + ) # Dropdown for selecting the user role (e.g., Admin or Basic) role = forms.ChoiceField( @@ -312,7 +320,15 @@ class UserPortfolioPermissionsForm(PortfolioPermissionsForm): """ # Dropdown to select a user from the database - user = forms.ModelChoiceField(queryset=models.User.objects.all(), label="User") + user = forms.ModelChoiceField( + queryset=models.User.objects.all(), + label="User", + widget=AutocompleteSelectWithPlaceholder( + models.UserPortfolioPermission._meta.get_field("user"), + admin.site, + attrs={"data-placeholder": "---------"} # Customize placeholder + ), + ) class Meta: """ @@ -347,7 +363,6 @@ class PortfolioInvitationForm(PortfolioPermissionsForm): "status", ] - class DomainInformationAdminForm(forms.ModelForm): """This form utilizes the custom widget for its class's ManyToMany UIs.""" From c7aa2f3a389c793830e749cae72f3302361555de Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 1 Mar 2025 07:46:44 -0500 Subject: [PATCH 09/18] added placeholder to role selection, and handled validation error when user not selected --- src/registrar/admin.py | 2 +- .../portfolio-permissions-form.js | 20 +++++++++---------- .../models/user_portfolio_permission.py | 7 ++++++- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 3352a5b81..a3b9a3a0f 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -229,7 +229,7 @@ class PortfolioPermissionsForm(forms.ModelForm): # Dropdown for selecting the user role (e.g., Admin or Basic) role = forms.ChoiceField( - choices=UserPortfolioRoleChoices.choices, + choices=[("", "---------")] + UserPortfolioRoleChoices.choices, required=True, widget=forms.Select(attrs={"class": "admin-dropdown"}), label="Member access", diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-permissions-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-permissions-form.js index 09e11c5d0..3e331ea46 100644 --- a/src/registrar/assets/src/js/getgov-admin/portfolio-permissions-form.js +++ b/src/registrar/assets/src/js/getgov-admin/portfolio-permissions-form.js @@ -15,26 +15,26 @@ function handlePortfolioPermissionFields(){ * Updates the visibility of portfolio permissions fields based on the selected role. * * This function checks the value of the role dropdown (`roleDropdown`): - * - If the selected role is "organization_admin": - * - Hides the domain permissions field (`domainPermissionsField`). - * - Hides the domain request permissions field (`domainRequestPermissionsField`). - * - Hides the member permissions field (`memberPermissionsField`). + * - If the selected role is "organization_member": + * - Shows the domain permissions field (`domainPermissionsField`). + * - Shows the domain request permissions field (`domainRequestPermissionsField`). + * - Shows the member permissions field (`memberPermissionsField`). * - Otherwise: - * - Shows all the above fields. + * - Hides all the above fields. * * The function ensures that the appropriate fields are dynamically displayed * or hidden depending on the role selection in the form. */ function updatePortfolioPermissionsFormVisibility() { if (roleDropdown && domainPermissionsField && domainRequestPermissionsField && memberPermissionsField) { - if (roleDropdown.value === "organization_admin") { - hideElement(domainPermissionsField); - hideElement(domainRequestPermissionsField); - hideElement(memberPermissionsField); - } else { + if (roleDropdown.value === "organization_member") { showElement(domainPermissionsField); showElement(domainRequestPermissionsField); showElement(memberPermissionsField); + } else { + hideElement(domainPermissionsField); + hideElement(domainRequestPermissionsField); + hideElement(memberPermissionsField); } } } diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 0a758ff6a..e2ab28782 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -275,7 +275,12 @@ class UserPortfolioPermission(TimeStampedModel): def clean(self): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() - validate_user_portfolio_permission(self) + # Ensure user exists before running further validation + # In django admin, this clean method is called before form validation checks + # for required fields. Since validation below requires user, skip if user does + # not exist + if self.user_id: + validate_user_portfolio_permission(self) def delete(self, *args, **kwargs): From 80ade1e4a9bfe4453c5005097ffd1733b5d378f9 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 1 Mar 2025 07:57:08 -0500 Subject: [PATCH 10/18] added helper text below role selection --- src/registrar/admin.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a3b9a3a0f..8ec39bb88 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -223,7 +223,7 @@ class PortfolioPermissionsForm(forms.ModelForm): widget=AutocompleteSelectWithPlaceholder( models.PortfolioInvitation._meta.get_field("portfolio"), admin.site, - attrs={"data-placeholder": "---------"} # Customize placeholder + attrs={"data-placeholder": "---------"}, # Customize placeholder ), ) @@ -233,6 +233,7 @@ class PortfolioPermissionsForm(forms.ModelForm): required=True, widget=forms.Select(attrs={"class": "admin-dropdown"}), label="Member access", + help_text="Only admins can manage member permissions and organization metadata.", ) # Dropdown for selecting request permissions, with a default "No access" option @@ -326,7 +327,7 @@ class UserPortfolioPermissionsForm(PortfolioPermissionsForm): widget=AutocompleteSelectWithPlaceholder( models.UserPortfolioPermission._meta.get_field("user"), admin.site, - attrs={"data-placeholder": "---------"} # Customize placeholder + attrs={"data-placeholder": "---------"}, # Customize placeholder ), ) @@ -363,6 +364,7 @@ class PortfolioInvitationForm(PortfolioPermissionsForm): "status", ] + class DomainInformationAdminForm(forms.ModelForm): """This form utilizes the custom widget for its class's ManyToMany UIs.""" From 31d3a0059fe31b3bba8242c3ccdbba7f3da0d773 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 1 Mar 2025 08:20:58 -0500 Subject: [PATCH 11/18] updated alert messages in tables in django admin --- .../django/admin/domain_invitation_change_form.html | 6 +++++- .../admin/domain_invitation_delete_confirmation.html | 10 ++++++---- ...domain_invitation_delete_selected_confirmation.html | 10 ++++++---- .../django/admin/portfolio_invitation_change_form.html | 6 +++++- .../portfolio_invitation_delete_confirmation.html | 6 +++--- .../django/admin/user_domain_role_change_form.html | 5 ++++- .../admin/user_domain_role_delete_confirmation.html | 2 +- .../user_domain_role_delete_selected_confirmation.html | 2 +- .../admin/user_portfolio_permission_change_form.html | 6 +++++- .../user_portfolio_permission_delete_confirmation.html | 2 +- 10 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/registrar/templates/django/admin/domain_invitation_change_form.html b/src/registrar/templates/django/admin/domain_invitation_change_form.html index 699760fa8..886cfc834 100644 --- a/src/registrar/templates/django/admin/domain_invitation_change_form.html +++ b/src/registrar/templates/django/admin/domain_invitation_change_form.html @@ -6,7 +6,11 @@

- If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain when you click "save." If you don't want to trigger those emails, use the User domain roles permissions table instead. + If you invite someone to a domain here, it will trigger email notifications. If you don't want to trigger emails, use the + + User Domain Roles + + table instead.

diff --git a/src/registrar/templates/django/admin/domain_invitation_delete_confirmation.html b/src/registrar/templates/django/admin/domain_invitation_delete_confirmation.html index 215bf5ada..7315c9ddc 100644 --- a/src/registrar/templates/django/admin/domain_invitation_delete_confirmation.html +++ b/src/registrar/templates/django/admin/domain_invitation_delete_confirmation.html @@ -5,10 +5,12 @@ diff --git a/src/registrar/templates/django/admin/domain_invitation_delete_selected_confirmation.html b/src/registrar/templates/django/admin/domain_invitation_delete_selected_confirmation.html index 2e15347c1..8c1614f5d 100644 --- a/src/registrar/templates/django/admin/domain_invitation_delete_selected_confirmation.html +++ b/src/registrar/templates/django/admin/domain_invitation_delete_selected_confirmation.html @@ -5,10 +5,12 @@ diff --git a/src/registrar/templates/django/admin/portfolio_invitation_change_form.html b/src/registrar/templates/django/admin/portfolio_invitation_change_form.html index 959e8f8bf..c679b36cd 100644 --- a/src/registrar/templates/django/admin/portfolio_invitation_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_invitation_change_form.html @@ -6,7 +6,11 @@

- If you add someone to a portfolio here, it will trigger an invitation email when you click "save." If you don't want to trigger an email, use the User portfolio permissions table instead. + If you invite someone to a portfolio here, it will trigger email notifications. If you don't want to trigger emails, use the + + User Portfolio Permissions + + table instead.

diff --git a/src/registrar/templates/django/admin/portfolio_invitation_delete_confirmation.html b/src/registrar/templates/django/admin/portfolio_invitation_delete_confirmation.html index 932822766..187b5d2c7 100644 --- a/src/registrar/templates/django/admin/portfolio_invitation_delete_confirmation.html +++ b/src/registrar/templates/django/admin/portfolio_invitation_delete_confirmation.html @@ -4,12 +4,12 @@

- If you cancel the portfolio invitation here, it won't trigger any emails. It also won't remove the user's - portfolio access if they already logged in. Go to the + If you cancel the portfolio invitation here, it won't trigger any email notifications. + It also won't remove the user's portfolio access if they already logged in. Go to the User Portfolio Permissions - table if you want to remove the user from a portfolio. + table if you want to remove their portfolio access.

diff --git a/src/registrar/templates/django/admin/user_domain_role_change_form.html b/src/registrar/templates/django/admin/user_domain_role_change_form.html index d8c298bc1..5c589952f 100644 --- a/src/registrar/templates/django/admin/user_domain_role_change_form.html +++ b/src/registrar/templates/django/admin/user_domain_role_change_form.html @@ -6,7 +6,10 @@

- If you add someone to a domain here, it will not trigger any emails. To trigger emails, use the User Domain Role invitations table instead. + If you add someone to a domain here, it won't trigger any email notifications. To trigger emails, use the + + Domain Invitations + table instead.

diff --git a/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html index 171f4c3ea..68ae1765d 100644 --- a/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html +++ b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html @@ -5,7 +5,7 @@ diff --git a/src/registrar/templates/django/admin/user_domain_role_delete_selected_confirmation.html b/src/registrar/templates/django/admin/user_domain_role_delete_selected_confirmation.html index 392d1aebc..c06a5493e 100644 --- a/src/registrar/templates/django/admin/user_domain_role_delete_selected_confirmation.html +++ b/src/registrar/templates/django/admin/user_domain_role_delete_selected_confirmation.html @@ -5,7 +5,7 @@ diff --git a/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html b/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html index 489d67bc5..76e37d568 100644 --- a/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html +++ b/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html @@ -6,7 +6,11 @@

- If you add someone to a portfolio here, it will not trigger an invitation email. To trigger an email, use the Portfolio invitations table instead. + If you add someone to a portfolio here, it won't trigger any email notifications. To trigger emails, use the + + Portfolio Invitations + + table instead.

diff --git a/src/registrar/templates/django/admin/user_portfolio_permission_delete_confirmation.html b/src/registrar/templates/django/admin/user_portfolio_permission_delete_confirmation.html index 71c789a63..eb2dd078a 100644 --- a/src/registrar/templates/django/admin/user_portfolio_permission_delete_confirmation.html +++ b/src/registrar/templates/django/admin/user_portfolio_permission_delete_confirmation.html @@ -4,7 +4,7 @@

- If you remove someone from a portfolio here, it will not send any emails when you click "Save". + If you remove someone from a portfolio here, it won't trigger any email notifications.

From 1adf059c76eb4688aa392995f5a046197c1946ec Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 1 Mar 2025 08:31:04 -0500 Subject: [PATCH 12/18] updated alert messages on bulk delete actions --- src/registrar/admin.py | 2 ++ ...invitation_delete_selected_confirmation.html | 17 +++++++++++++++++ ...permission_delete_selected_confirmation.html | 12 ++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 src/registrar/templates/django/admin/portfolio_invitation_delete_selected_confirmation.html create mode 100644 src/registrar/templates/django/admin/user_portfolio_permission_delete_selected_confirmation.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8ec39bb88..0c646ba71 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1467,6 +1467,7 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): change_form_template = "django/admin/user_portfolio_permission_change_form.html" delete_confirmation_template = "django/admin/user_portfolio_permission_delete_confirmation.html" + delete_selected_confirmation_template = "django/admin/user_portfolio_permission_delete_selected_confirmation.html" def get_roles(self, obj): readable_roles = obj.get_readable_roles() @@ -1811,6 +1812,7 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin): change_form_template = "django/admin/portfolio_invitation_change_form.html" delete_confirmation_template = "django/admin/portfolio_invitation_delete_confirmation.html" + delete_selected_confirmation_template = "django/admin/portfolio_invitation_delete_selected_confirmation.html" # Select portfolio invitations to change -> Portfolio invitations def changelist_view(self, request, extra_context=None): diff --git a/src/registrar/templates/django/admin/portfolio_invitation_delete_selected_confirmation.html b/src/registrar/templates/django/admin/portfolio_invitation_delete_selected_confirmation.html new file mode 100644 index 000000000..2d8bed70b --- /dev/null +++ b/src/registrar/templates/django/admin/portfolio_invitation_delete_selected_confirmation.html @@ -0,0 +1,17 @@ +{% extends "admin/delete_selected_confirmation.html" %} + +{% block content_subtitle %} +
+
+

+ If you cancel the portfolio invitation here, it won't trigger any email notifications. + It also won't remove the user's portfolio access if they already logged in. Go to the + + User Portfolio Permissions + + table if you want to remove their portfolio access. +

+
+
+ {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/django/admin/user_portfolio_permission_delete_selected_confirmation.html b/src/registrar/templates/django/admin/user_portfolio_permission_delete_selected_confirmation.html new file mode 100644 index 000000000..d0d586f07 --- /dev/null +++ b/src/registrar/templates/django/admin/user_portfolio_permission_delete_selected_confirmation.html @@ -0,0 +1,12 @@ +{% extends "admin/delete_selected_confirmation.html" %} + +{% block content_subtitle %} +
+
+

+ If you remove someone from a portfolio here, it won't trigger any email notifications. +

+
+
+ {{ block.super }} +{% endblock %} From 7a235dcb974004519418880a5039019722a3afd2 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 1 Mar 2025 08:48:50 -0500 Subject: [PATCH 13/18] updated table descriptions --- .../templates/admin/model_descriptions.html | 2 ++ .../domain_invitation_description.html | 14 ++++++-------- .../portfolio_invitation_description.html | 16 ++++++++++------ .../user_domain_role_description.html | 13 ++++++++----- .../user_portfolio_permission_description.html | 11 +++++++++++ 5 files changed, 37 insertions(+), 19 deletions(-) create mode 100644 src/registrar/templates/django/admin/includes/descriptions/user_portfolio_permission_description.html diff --git a/src/registrar/templates/admin/model_descriptions.html b/src/registrar/templates/admin/model_descriptions.html index 9f13245fe..9b163c407 100644 --- a/src/registrar/templates/admin/model_descriptions.html +++ b/src/registrar/templates/admin/model_descriptions.html @@ -30,6 +30,8 @@ {% include "django/admin/includes/descriptions/verified_by_staff_description.html" %} {% elif opts.model_name == 'website' %} {% include "django/admin/includes/descriptions/website_description.html" %} + {% elif opts.model_name == 'userportfoliopermission' %} + {% include "django/admin/includes/descriptions/user_portfolio_permission_description.html" %} {% elif opts.model_name == 'portfolioinvitation' %} {% include "django/admin/includes/descriptions/portfolio_invitation_description.html" %} {% elif opts.model_name == 'allowedemail' %} diff --git a/src/registrar/templates/django/admin/includes/descriptions/domain_invitation_description.html b/src/registrar/templates/django/admin/includes/descriptions/domain_invitation_description.html index ff277a444..6b2cae5ce 100644 --- a/src/registrar/templates/django/admin/includes/descriptions/domain_invitation_description.html +++ b/src/registrar/templates/django/admin/includes/descriptions/domain_invitation_description.html @@ -1,16 +1,14 @@

-Domain invitations contain all individuals who have been invited to manage a .gov domain. -Invitations are sent via email, and the recipient must log in to the registrar to officially -accept and become a domain manager. + This table contains all individuals who have been invited to manage a .gov domain. + These individuals must log in to the registrar to officially accept and become a domain manager.

-An “invited” status indicates that the recipient has not logged in to the registrar since the invitation was sent. Deleting an invitation with an "invited" status will prevent the user from signing in. -A “received” status indicates that the recipient has logged in. Deleting an invitation with a "received" status will not revoke that user's access from the domain. To remove a user who has already signed in, go to User domain roles and delete the role for the correct domain/manager combination. + An “invited” status indicates that the recipient has not logged in to the registrar since the invitation was sent. + A “received” status indicates that the recipient has logged in.

-If an invitation is created in this table, an email will not be sent. -To have an email sent, go to the domain in Domains, -click the “Manage domain” button, and add a domain manager. + If you invite someone to a domain by using this table, they’ll receive an email notification. + The existing managers of the domain will also be notified. However, canceling an invitation here won’t trigger any emails.

diff --git a/src/registrar/templates/django/admin/includes/descriptions/portfolio_invitation_description.html b/src/registrar/templates/django/admin/includes/descriptions/portfolio_invitation_description.html index 51515bcb2..9a486f944 100644 --- a/src/registrar/templates/django/admin/includes/descriptions/portfolio_invitation_description.html +++ b/src/registrar/templates/django/admin/includes/descriptions/portfolio_invitation_description.html @@ -1,11 +1,15 @@

-Portfolio invitations contain all individuals who have been invited to become members of an organization. -Invitations are sent via email, and the recipient must log in to the registrar to officially -accept and become a member. + This table contains all individuals who have been invited to become members of a portfolio. + These individuals must log in to the registrar to officially accept and become a member.

-An “invited” status indicates that the recipient has not logged in to the registrar since the invitation was sent -or that the recipient has logged in but is already a member of an organization. -A “received” status indicates that the recipient has logged in. + An “invited” status indicates that the recipient has not logged in to the registrar since the invitation + was sent or that the recipient has logged in but is already a member of another portfolio. A “received” + status indicates that the recipient has logged in. +

+ +

+ If you invite someone to a portfolio by using this table, they’ll receive an email notification. + If you assign them "admin" access, the existing portfolio admins will also be notified. However, canceling an invitation here won’t trigger any emails.

diff --git a/src/registrar/templates/django/admin/includes/descriptions/user_domain_role_description.html b/src/registrar/templates/django/admin/includes/descriptions/user_domain_role_description.html index 7066fcb93..f0e7f5a5f 100644 --- a/src/registrar/templates/django/admin/includes/descriptions/user_domain_role_description.html +++ b/src/registrar/templates/django/admin/includes/descriptions/user_domain_role_description.html @@ -1,10 +1,13 @@

-This table represents the managers who are assigned to each domain in the registrar. -There are separate records for each domain/manager combination. -Managers can update information related to a domain, such as DNS data and security contact. + This table represents the managers who are assigned to each domain in the registrar. There are separate records for each domain/manager combination. + Managers can update information related to a domain, such as DNS data and security contact.

-The creator of an approved domain request automatically becomes a manager for that domain. -Anyone who retrieves a domain invitation is also assigned the manager role. + The creator of an approved domain request automatically becomes a manager for that domain. + Anyone who retrieves a domain invitation will also appear in this table as a manager. +

+ +

+ If you add or remove someone to a domain by using this table, those actions won’t trigger notification emails.

diff --git a/src/registrar/templates/django/admin/includes/descriptions/user_portfolio_permission_description.html b/src/registrar/templates/django/admin/includes/descriptions/user_portfolio_permission_description.html new file mode 100644 index 000000000..fd8919b8f --- /dev/null +++ b/src/registrar/templates/django/admin/includes/descriptions/user_portfolio_permission_description.html @@ -0,0 +1,11 @@ +

+ This table represents the members of each portfolio in the registrar. There are separate records for each member/portfolio combination. +

+ +

+ Each member is assigned one of two access levels: admin or basic. Only admins can manage member permissions and organization metadata. +

+ +

+ If you add or remove someone to a portfolio by using this table, those actions won’t trigger notification emails. +

\ No newline at end of file From c6357435edfefe96f9159e73c146e6b34461ba24 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 1 Mar 2025 09:24:25 -0500 Subject: [PATCH 14/18] updated portfolio members admin section and updated tests --- src/registrar/admin.py | 14 +++++++------- src/registrar/tests/test_admin.py | 16 ++++++++-------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0c646ba71..2497c4667 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -4385,21 +4385,21 @@ class PortfolioAdmin(ListHeaderAdmin): if admin_count > 0: url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={obj.id}" # Create a clickable link with the count - return format_html(f'{admin_count} administrators') - return "No administrators found." + return format_html(f'{admin_count} admins') + return "No admins found." - display_admins.short_description = "Administrators" # type: ignore + display_admins.short_description = "Admins" # type: ignore def display_members(self, obj): - """Returns the number of members for this portfolio""" + """Returns the number of basic members for this portfolio""" member_count = len(self.get_user_portfolio_permission_non_admins(obj)) if member_count > 0: url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={obj.id}" # Create a clickable link with the count - return format_html(f'{member_count} members') - return "No additional members found." + return format_html(f'{member_count} basic members') + return "No basic members found." - display_members.short_description = "Members" # type: ignore + display_members.short_description = "Basic members" # type: ignore # Creates select2 fields (with search bars) autocomplete_fields = [ diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 7a8a9b783..b0c453c60 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -177,7 +177,7 @@ class TestDomainInvitationAdmin(WebTest): # Test for a description snippet self.assertContains( - response, "Domain invitations contain all individuals who have been invited to manage a .gov domain." + response, "This table contains all individuals who have been invited to manage a .gov domain." ) self.assertContains(response, "Show more") @@ -201,7 +201,7 @@ class TestDomainInvitationAdmin(WebTest): # Test for a description snippet self.assertContains( response, - "If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain", + "If you invite someone to a domain here, it will trigger email notifications.", ) @less_console_noise_decorator @@ -217,7 +217,7 @@ class TestDomainInvitationAdmin(WebTest): ) # Assert that the filters are added - self.assertContains(response, "invited", count=5) + self.assertContains(response, "invited", count=4) self.assertContains(response, "Invited", count=2) self.assertContains(response, "retrieved", count=2) self.assertContains(response, "Retrieved", count=2) @@ -1168,7 +1168,7 @@ class TestUserPortfolioPermissionAdmin(TestCase): # Test for a description snippet self.assertContains( response, - "If you add someone to a portfolio here, it will not trigger an invitation email.", + "If you add someone to a portfolio here, it won't trigger any email notifications.", ) @less_console_noise_decorator @@ -1183,7 +1183,7 @@ class TestUserPortfolioPermissionAdmin(TestCase): response = self.client.get(delete_url) # Check if the response contains the expected static message - expected_message = "If you remove someone from a portfolio here, it will not send any emails" + expected_message = "If you remove someone from a portfolio here, it won't trigger any email notifications." self.assertIn(expected_message, response.content.decode("utf-8")) @@ -1232,7 +1232,7 @@ class TestPortfolioInvitationAdmin(TestCase): # Test for a description snippet self.assertContains( response, - "Portfolio invitations contain all individuals who have been invited to become members of an organization.", + "This table contains all individuals who have been invited to become members of a portfolio.", ) self.assertContains(response, "Show more") @@ -1256,7 +1256,7 @@ class TestPortfolioInvitationAdmin(TestCase): # Test for a description snippet self.assertContains( response, - "If you add someone to a portfolio here, it will trigger an invitation email when you click", + "If you invite someone to a portfolio here, it will trigger email notifications.", ) @less_console_noise_decorator @@ -2325,7 +2325,7 @@ class TestUserDomainRoleAdmin(WebTest): # Test for a description snippet self.assertContains( response, - "If you add someone to a domain here, it will not trigger any emails.", + "If you add someone to a domain here, it won't trigger any email notifications.", ) def test_domain_sortable(self): From 427f396391965ff82a0bd3c133edcd4f987eba5c Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 1 Mar 2025 09:41:23 -0500 Subject: [PATCH 15/18] column changes in tables --- src/registrar/admin.py | 11 ++++++++--- src/registrar/models/portfolio_invitation.py | 5 +++++ src/registrar/models/user_portfolio_permission.py | 8 ++------ src/registrar/models/utility/portfolio_helper.py | 8 ++++++++ 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 2497c4667..699912f8e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1473,7 +1473,7 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): readable_roles = obj.get_readable_roles() return ", ".join(readable_roles) - get_roles.short_description = "Roles" # type: ignore + get_roles.short_description = "Member access" # type: ignore def delete_queryset(self, request, queryset): """We override the delete method in the model. @@ -1786,8 +1786,7 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin): list_display = [ "email", "portfolio", - "roles", - "additional_permissions", + "get_roles", "status", ] @@ -1814,6 +1813,12 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin): delete_confirmation_template = "django/admin/portfolio_invitation_delete_confirmation.html" delete_selected_confirmation_template = "django/admin/portfolio_invitation_delete_selected_confirmation.html" + def get_roles(self, obj): + readable_roles = obj.get_readable_roles() + return ", ".join(readable_roles) + + get_roles.short_description = "Member access" # type: ignore + # Select portfolio invitations to change -> Portfolio invitations def changelist_view(self, request, extra_context=None): if extra_context is None: diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index fafa99856..5c5c62142 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -15,6 +15,7 @@ from .utility.portfolio_helper import ( get_domains_display, get_members_description_display, get_members_display, + get_readable_roles, get_role_display, validate_portfolio_invitation, ) # type: ignore @@ -78,6 +79,10 @@ class PortfolioInvitation(TimeStampedModel): def __str__(self): return f"Invitation for {self.email} on {self.portfolio} is {self.status}" + def get_readable_roles(self): + """Returns a readable list of self.roles""" + return get_readable_roles(self.roles) + def get_managed_domains_count(self): """Return the count of domain invitations managed by the invited user for this portfolio.""" # Filter the UserDomainRole model to get domains where the user has a manager role diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index e2ab28782..38a87722d 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -12,6 +12,7 @@ from registrar.models.utility.portfolio_helper import ( get_domains_description_display, get_members_display, get_members_description_display, + get_readable_roles, get_role_display, validate_user_portfolio_permission, ) @@ -94,12 +95,7 @@ class UserPortfolioPermission(TimeStampedModel): def get_readable_roles(self): """Returns a readable list of self.roles""" - readable_roles = [] - if self.roles: - readable_roles = sorted( - [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles] - ) - return readable_roles + return get_readable_roles(self.roles) def get_managed_domains_count(self): """Return the count of domains managed by the user for this portfolio.""" diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 679ad7b9b..70be16d5b 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -79,6 +79,14 @@ class MemberPermissionDisplay(StrEnum): NONE = "None" +def get_readable_roles(roles): + readable_roles = [] + if roles: + readable_roles = sorted( + [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in roles] + ) + return readable_roles + def get_role_display(roles): """ Returns a user-friendly display name for a given list of user roles. From 87d13e299491b7ee2d0929bb5ca4ca472306d91e Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 1 Mar 2025 09:52:26 -0500 Subject: [PATCH 16/18] formatted model files for readability --- src/registrar/admin.py | 4 ++-- src/registrar/models/portfolio_invitation.py | 2 +- src/registrar/models/utility/portfolio_helper.py | 5 ++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 699912f8e..1b1e0978f 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1816,9 +1816,9 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin): def get_roles(self, obj): readable_roles = obj.get_readable_roles() return ", ".join(readable_roles) - + get_roles.short_description = "Member access" # type: ignore - + # Select portfolio invitations to change -> Portfolio invitations def changelist_view(self, request, extra_context=None): if extra_context is None: diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 5c5c62142..1da0fcdd1 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -82,7 +82,7 @@ class PortfolioInvitation(TimeStampedModel): def get_readable_roles(self): """Returns a readable list of self.roles""" return get_readable_roles(self.roles) - + def get_managed_domains_count(self): """Return the count of domain invitations managed by the invited user for this portfolio.""" # Filter the UserDomainRole model to get domains where the user has a manager role diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 70be16d5b..03dca897f 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -82,11 +82,10 @@ class MemberPermissionDisplay(StrEnum): def get_readable_roles(roles): readable_roles = [] if roles: - readable_roles = sorted( - [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in roles] - ) + readable_roles = sorted([UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in roles]) return readable_roles + def get_role_display(roles): """ Returns a user-friendly display name for a given list of user roles. From 989380784ee1a4a83799719a84cb3c3ec9ab4c8f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 1 Mar 2025 10:01:53 -0500 Subject: [PATCH 17/18] fixed a test --- src/registrar/tests/test_admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index b0c453c60..4eda6a30d 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -3699,10 +3699,10 @@ class TestPortfolioAdmin(TestCase): display_admins = self.admin.display_admins(self.portfolio) url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={self.portfolio.id}" - self.assertIn(f'2 administrators', display_admins) + self.assertIn(f'2 admins', display_admins) display_members = self.admin.display_members(self.portfolio) - self.assertIn(f'2 members', display_members) + self.assertIn(f'2 basic members', display_members) @less_console_noise_decorator def test_senior_official_readonly_for_federal_org(self): From 472d920489ca945a3002091983ea40ee89e1578d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 5 Mar 2025 15:22:23 -0500 Subject: [PATCH 18/18] fixing test --- src/registrar/tests/test_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 211e90a2b..1de6b1be3 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -217,7 +217,7 @@ class TestDomainInvitationAdmin(WebTest): ) # Assert that the filters are added - self.assertContains(response, "invited", count=4) + self.assertContains(response, "invited", count=5) self.assertContains(response, "Invited", count=2) self.assertContains(response, "retrieved", count=3) self.assertContains(response, "Retrieved", count=2)