diff --git a/src/registrar/assets/src/js/getgov/main.js b/src/registrar/assets/src/js/getgov/main.js index bd4bed01b..f5ebc83a3 100644 --- a/src/registrar/assets/src/js/getgov/main.js +++ b/src/registrar/assets/src/js/getgov/main.js @@ -10,8 +10,7 @@ import { initDomainRequestsTable } from './table-domain-requests.js'; import { initMembersTable } from './table-members.js'; import { initMemberDomainsTable } from './table-member-domains.js'; import { initEditMemberDomainsTable } from './table-edit-member-domains.js'; -import { initPortfolioMemberPageToggle } from './portfolio-member-page.js'; -import { initAddNewMemberPageListeners } from './portfolio-member-page.js'; +import { initPortfolioNewMemberPageToggle, initAddNewMemberPageListeners, initPortfolioMemberPageRadio } from './portfolio-member-page.js'; initDomainValidators(); @@ -21,13 +20,6 @@ nameserversFormListener(); hookupYesNoListener("other_contacts-has_other_contacts",'other-employees', 'no-other-employees'); hookupYesNoListener("additional_details-has_anything_else_text",'anything-else', null); -hookupRadioTogglerListener( - 'member_access_level', - { - 'admin': 'new-member-admin-permissions', - 'basic': 'new-member-basic-permissions' - } -); hookupYesNoListener("additional_details-has_cisa_representative",'cisa-representative', null); initializeUrbanizationToggle(); @@ -44,5 +36,7 @@ initMembersTable(); initMemberDomainsTable(); initEditMemberDomainsTable(); -initPortfolioMemberPageToggle(); +// Init the portfolio new member page +initPortfolioMemberPageRadio(); +initPortfolioNewMemberPageToggle(); initAddNewMemberPageListeners(); diff --git a/src/registrar/assets/src/js/getgov/portfolio-member-page.js b/src/registrar/assets/src/js/getgov/portfolio-member-page.js index ba874cfb1..02d927438 100644 --- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js +++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js @@ -2,9 +2,10 @@ import { uswdsInitializeModals } from './helpers-uswds.js'; import { getCsrfToken } from './helpers.js'; import { generateKebabHTML } from './table-base.js'; import { MembersTable } from './table-members.js'; +import { hookupRadioTogglerListener } from './radios.js'; // This is specifically for the Member Profile (Manage Member) Page member/invitation removal -export function initPortfolioMemberPageToggle() { +export function initPortfolioNewMemberPageToggle() { document.addEventListener("DOMContentLoaded", () => { const wrapperDeleteAction = document.getElementById("wrapper-delete-action") if (wrapperDeleteAction) { @@ -169,4 +170,29 @@ export function initAddNewMemberPageListeners() { } } -} \ No newline at end of file +} + +// Initalize the radio for the member pages +export function initPortfolioMemberPageRadio() { + document.addEventListener("DOMContentLoaded", () => { + let memberForm = document.getElementById("member_form"); + let newMemberForm = document.getElementById("add_member_form") + if (memberForm) { + hookupRadioTogglerListener( + 'role', + { + 'organization_admin': 'member-admin-permissions', + 'organization_member': 'member-basic-permissions' + } + ); + }else if (newMemberForm){ + hookupRadioTogglerListener( + 'member_access_level', + { + 'admin': 'new-member-admin-permissions', + 'basic': 'new-member-basic-permissions' + } + ); + } + }); +} diff --git a/src/registrar/assets/src/js/getgov/radios.js b/src/registrar/assets/src/js/getgov/radios.js index 248865e8b..055bdf621 100644 --- a/src/registrar/assets/src/js/getgov/radios.js +++ b/src/registrar/assets/src/js/getgov/radios.js @@ -38,21 +38,21 @@ export function hookupYesNoListener(radioButtonName, elementIdToShowIfYes, eleme **/ export function hookupRadioTogglerListener(radioButtonName, valueToElementMap) { // Get the radio buttons - let radioButtons = document.querySelectorAll('input[name="'+radioButtonName+'"]'); + let radioButtons = document.querySelectorAll(`input[name="${radioButtonName}"]`); // Extract the list of all element IDs from the valueToElementMap let allElementIds = Object.values(valueToElementMap); - + function handleRadioButtonChange() { // Find the checked radio button - let radioButtonChecked = document.querySelector('input[name="'+radioButtonName+'"]:checked'); + let radioButtonChecked = document.querySelector(`input[name="${radioButtonName}"]:checked`); let selectedValue = radioButtonChecked ? radioButtonChecked.value : null; // Hide all elements by default allElementIds.forEach(function (elementId) { let element = document.getElementById(elementId); if (element) { - hideElement(element); + hideElement(element); } }); @@ -64,8 +64,8 @@ export function hookupRadioTogglerListener(radioButtonName, valueToElementMap) { } } } - - if (radioButtons.length) { + + if (radioButtons && radioButtons.length) { // Add event listener to each radio button radioButtons.forEach(function (radioButton) { radioButton.addEventListener('change', handleRadioButtonChange); diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 5309f7263..eaa885a85 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -4,6 +4,7 @@ from django import forms from django.core.validators import RegexValidator from django.core.validators import MaxLengthValidator +from django.utils.safestring import mark_safe from registrar.models import ( PortfolioInvitation, @@ -271,3 +272,210 @@ def clean(self): if admin_member_error in self.errors: del self.errors[admin_member_error] return cleaned_data + + +class BasePortfolioMemberForm(forms.Form): + """Base form for the PortfolioMemberForm and PortfolioInvitedMemberForm""" + + # The label for each of these has a red "required" star. We can just embed that here for simplicity. + required_star = '*' + role = forms.ChoiceField( + choices=[ + # Uses .value because the choice has a different label (on /admin) + (UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value, "Admin access"), + (UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, "Basic access"), + ], + widget=forms.RadioSelect, + required=True, + error_messages={ + "required": "Member access level is required", + }, + ) + + domain_request_permission_admin = forms.ChoiceField( + label=mark_safe(f"Select permission {required_star}"), # nosec + choices=[ + (UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "View all requests"), + (UserPortfolioPermissionChoices.EDIT_REQUESTS.value, "View all requests plus create requests"), + ], + widget=forms.RadioSelect, + required=False, + error_messages={ + "required": "Admin domain request permission is required", + }, + ) + + member_permission_admin = forms.ChoiceField( + label=mark_safe(f"Select permission {required_star}"), # nosec + choices=[ + (UserPortfolioPermissionChoices.VIEW_MEMBERS.value, "View all members"), + (UserPortfolioPermissionChoices.EDIT_MEMBERS.value, "View all members plus manage members"), + ], + widget=forms.RadioSelect, + required=False, + error_messages={ + "required": "Admin member permission is required", + }, + ) + + domain_request_permission_member = forms.ChoiceField( + label=mark_safe(f"Select permission {required_star}"), # nosec + choices=[ + (UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "View all requests"), + (UserPortfolioPermissionChoices.EDIT_REQUESTS.value, "View all requests plus create requests"), + ("no_access", "No access"), + ], + widget=forms.RadioSelect, + required=False, + error_messages={ + "required": "Basic member permission is required", + }, + ) + + # Tracks what form elements are required for a given role choice. + # All of the fields included here have "required=False" by default as they are conditionally required. + # see def clean() for more details. + ROLE_REQUIRED_FIELDS = { + UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ + "domain_request_permission_admin", + "member_permission_admin", + ], + UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ + "domain_request_permission_member", + ], + } + + def __init__(self, *args, instance=None, **kwargs): + """Initialize self.instance, self.initial, and descriptions under each radio button. + Uses map_instance_to_initial to set the initial dictionary.""" + super().__init__(*args, **kwargs) + if instance: + self.instance = instance + self.initial = self.map_instance_to_initial(self.instance) + # Adds a

description beneath each role option + self.fields["role"].descriptions = { + "organization_admin": UserPortfolioRoleChoices.get_role_description( + UserPortfolioRoleChoices.ORGANIZATION_ADMIN + ), + "organization_member": UserPortfolioRoleChoices.get_role_description( + UserPortfolioRoleChoices.ORGANIZATION_MEMBER + ), + } + + def save(self): + """Saves self.instance by grabbing data from self.cleaned_data. + Uses map_cleaned_data_to_instance. + """ + self.instance = self.map_cleaned_data_to_instance(self.cleaned_data, self.instance) + self.instance.save() + return self.instance + + def clean(self): + """Validates form data based on selected role and its required fields.""" + cleaned_data = super().clean() + role = cleaned_data.get("role") + + # Get required fields for the selected role. Then validate all required fields for the role. + required_fields = self.ROLE_REQUIRED_FIELDS.get(role, []) + for field_name in required_fields: + # Helpful error for if this breaks + if field_name not in self.fields: + raise ValueError(f"ROLE_REQUIRED_FIELDS referenced a non-existent field: {field_name}.") + + if not cleaned_data.get(field_name): + self.add_error(field_name, self.fields.get(field_name).error_messages.get("required")) + + # Edgecase: Member uses a special form value for None called "no_access". + if cleaned_data.get("domain_request_permission_member") == "no_access": + cleaned_data["domain_request_permission_member"] = None + + return cleaned_data + + # Explanation of how map_instance_to_initial / map_cleaned_data_to_instance work: + # map_instance_to_initial => called on init to set self.initial. + # Converts the incoming object (usually PortfolioInvitation or UserPortfolioPermission) + # into a dictionary representation for the form to use automatically. + + # map_cleaned_data_to_instance => called on save() to save the instance to the db. + # Takes the self.cleaned_data dict, and converts this dict back to the object. + + def map_instance_to_initial(self, instance): + """ + Maps self.instance to self.initial, handling roles and permissions. + Returns form data dictionary with appropriate permission levels based on user role: + { + "role": "organization_admin" or "organization_member", + "member_permission_admin": permission level if admin, + "domain_request_permission_admin": permission level if admin, + "domain_request_permission_member": permission level if member + } + """ + # Function variables + form_data = {} + perms = UserPortfolioPermission.get_portfolio_permissions( + instance.roles, instance.additional_permissions, get_list=False + ) + + # Get the available options for roles, domains, and member. + roles = [ + UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + UserPortfolioRoleChoices.ORGANIZATION_MEMBER, + ] + domain_perms = [ + UserPortfolioPermissionChoices.EDIT_REQUESTS, + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + ] + member_perms = [ + UserPortfolioPermissionChoices.EDIT_MEMBERS, + UserPortfolioPermissionChoices.VIEW_MEMBERS, + ] + + # Build form data based on role (which options are available). + # Get which one should be "selected" by assuming that EDIT takes precedence over view, + # and ADMIN takes precedence over MEMBER. + roles = instance.roles or [] + selected_role = next((role for role in roles if role in roles), None) + form_data = {"role": selected_role} + is_admin = selected_role == UserPortfolioRoleChoices.ORGANIZATION_ADMIN + if is_admin: + selected_domain_permission = next((perm for perm in domain_perms if perm in perms), None) + selected_member_permission = next((perm for perm in member_perms if perm in perms), None) + form_data["domain_request_permission_admin"] = selected_domain_permission + form_data["member_permission_admin"] = selected_member_permission + else: + # Edgecase: Member uses a special form value for None called "no_access". This ensures a form selection. + selected_domain_permission = next((perm for perm in domain_perms if perm in perms), "no_access") + form_data["domain_request_permission_member"] = selected_domain_permission + + return form_data + + def map_cleaned_data_to_instance(self, cleaned_data, instance): + """ + Maps self.cleaned_data to self.instance, setting roles and permissions. + Args: + cleaned_data (dict): Cleaned data containing role and permission choices + instance: Instance to update + + Returns: + instance: Updated instance + """ + role = cleaned_data.get("role") + + # Handle roles + instance.roles = [role] + + # Handle additional_permissions + valid_fields = self.ROLE_REQUIRED_FIELDS.get(role, []) + additional_permissions = {cleaned_data.get(field) for field in valid_fields if cleaned_data.get(field)} + + # Handle EDIT permissions (should be accompanied with a view permission) + if UserPortfolioPermissionChoices.EDIT_MEMBERS in additional_permissions: + additional_permissions.add(UserPortfolioPermissionChoices.VIEW_MEMBERS) + + if UserPortfolioPermissionChoices.EDIT_REQUESTS in additional_permissions: + additional_permissions.add(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) + + # Only set unique permissions not already defined in the base role + role_permissions = UserPortfolioPermission.get_portfolio_permissions(instance.roles, [], get_list=False) + instance.additional_permissions = list(additional_permissions - role_permissions) + return instance diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index a149a9476..25abb6748 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -110,8 +110,13 @@ def _get_portfolio_permissions(self): return self.get_portfolio_permissions(self.roles, self.additional_permissions) @classmethod - def get_portfolio_permissions(cls, roles, additional_permissions): - """Class method to return a list of permissions based on roles and addtl permissions""" + def get_portfolio_permissions(cls, roles, additional_permissions, get_list=True): + """Class method to return a list of permissions based on roles and addtl permissions. + Params: + roles => An array of roles + additional_permissions => An array of additional_permissions + get_list => If true, returns a list of perms. If false, returns a set of perms. + """ # Use a set to avoid duplicate permissions portfolio_permissions = set() if roles: @@ -119,7 +124,7 @@ def get_portfolio_permissions(cls, roles, additional_permissions): portfolio_permissions.update(cls.PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) if additional_permissions: portfolio_permissions.update(additional_permissions) - return list(portfolio_permissions) + return list(portfolio_permissions) if get_list else portfolio_permissions @classmethod def get_domain_request_permission_display(cls, roles, additional_permissions): diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 3768aa77a..cde28e4bd 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -4,6 +4,9 @@ from django.forms import ValidationError from registrar.utility.waffle import flag_is_active_for_user from django.contrib.auth import get_user_model +import logging + +logger = logging.getLogger(__name__) class UserPortfolioRoleChoices(models.TextChoices): @@ -16,7 +19,28 @@ class UserPortfolioRoleChoices(models.TextChoices): @classmethod def get_user_portfolio_role_label(cls, user_portfolio_role): - return cls(user_portfolio_role).label if user_portfolio_role else None + try: + return cls(user_portfolio_role).label if user_portfolio_role else None + except ValueError: + logger.warning(f"Invalid portfolio role: {user_portfolio_role}") + return f"Unknown ({user_portfolio_role})" + + @classmethod + def get_role_description(cls, user_portfolio_role): + """Returns a detailed description for a given role.""" + descriptions = { + cls.ORGANIZATION_ADMIN: ( + "Grants this member access to the organization-wide information " + "on domains, domain requests, and members. Domain management can be assigned separately." + ), + cls.ORGANIZATION_MEMBER: ( + "Grants this member access to the organization. They can be given extra permissions to view all " + "organization domain requests and submit domain requests on behalf of the organization. Basic access " + "members can’t view all members of an organization or manage them. " + "Domain management can be assigned separately." + ), + } + return descriptions.get(user_portfolio_role) class UserPortfolioPermissionChoices(models.TextChoices): diff --git a/src/registrar/templates/django/forms/widgets/multiple_input.html b/src/registrar/templates/django/forms/widgets/multiple_input.html index 90c241366..cc0e11989 100644 --- a/src/registrar/templates/django/forms/widgets/multiple_input.html +++ b/src/registrar/templates/django/forms/widgets/multiple_input.html @@ -1,3 +1,5 @@ +{% load static custom_filters %} +

{% for group, options, index in widget.optgroups %} {% if group %}
{% endif %} @@ -13,7 +15,17 @@ + > + {{ option.label }} + {% comment %} Add a description on each, if available {% endcomment %} + {% if field and field.field and field.field.descriptions %} + {% with description=field.field.descriptions|get_dict_value:option.value %} + {% if description %} +

{{ description }}

+ {% endif %} + {% endwith %} + {% endif %} + {% endfor %} {% if group %}
{% endif %} {% endfor %} diff --git a/src/registrar/templates/portfolio_member_permissions.html b/src/registrar/templates/portfolio_member_permissions.html index 02d120360..66becaa9e 100644 --- a/src/registrar/templates/portfolio_member_permissions.html +++ b/src/registrar/templates/portfolio_member_permissions.html @@ -1,42 +1,132 @@ {% extends 'portfolio_base.html' %} -{% load static field_helpers%} +{% load static url_helpers %} +{% load field_helpers %} -{% block title %}Organization member {% endblock %} +{% block title %}Organization member{% endblock %} -{% load static %} +{% block wrapper_class %} + {{ block.super }} dashboard--grey-1 +{% endblock %} {% block portfolio_content %} -
-
- - {% block messages %} - {% include "includes/form_messages.html" %} - {% endblock %} - -

Manage member

- -

- {% if member %} - {{ member.email }} - {% elif invitation %} - {{ invitation.email }} - {% endif %} -

- -
- -
- {% csrf_token %} - {% input_with_errors form.roles %} - {% input_with_errors form.additional_permissions %} - -
- - - -
-
-{% endblock %} +{% include "includes/form_errors.html" with form=form %} + + + + + +

Member access and permissions

+ +{% include "includes/required_fields.html" with remove_margin_top=True %} + +
+ {% csrf_token %} +
+ + {% if member and member.email or invitation and invitation.email %} +

Member email

+ {% else %} +

Member

+ {% endif %} +
+

+ {% comment %} + Show member email if possible, then invitation email. + If neither of these are true, show the name or as a last resort just "None". + {% endcomment %} + {% if member %} + {% if member.email %} + {{ member.email }} + {% else %} + {{ member.get_formatted_name }} + {% endif %} + {% elif invitation %} + {% if invitation.email %} + {{ invitation.email }} + {% else %} + None + {% endif %} + {% endif %} +

+ +
+ + +
+ +

Member Access

+
+ + Select the level of access for this member. * + + {% with add_class="usa-radio__input--tile" add_legend_class="usa-sr-only" %} + {% input_with_errors form.role %} + {% endwith %} + +
+ + +
+

Admin access permissions

+

Member permissions available for admin-level acccess.

+ +

Organization domain requests

+ {% with group_classes="usa-form-editable usa-form-editable--no-border padding-top-0" %} + {% input_with_errors form.domain_request_permission_admin %} + {% endwith %} + +

Organization members

+ {% with group_classes="usa-form-editable usa-form-editable--no-border padding-top-0" %} + {% input_with_errors form.member_permission_admin %} + {% endwith %} +
+ + +
+

Basic member permissions

+

Member permissions available for basic-level acccess.

+ +

Organization domain requests

+ {% with group_classes="usa-form-editable usa-form-editable--no-border padding-top-0" %} + {% input_with_errors form.domain_request_permission_member %} + {% endwith %} +
+ + +
+ + Cancel + + +
+
+{% endblock portfolio_content%} diff --git a/src/registrar/templates/portfolio_no_domains.html b/src/registrar/templates/portfolio_no_domains.html index ac6a8c036..995f391a2 100644 --- a/src/registrar/templates/portfolio_no_domains.html +++ b/src/registrar/templates/portfolio_no_domains.html @@ -5,12 +5,6 @@ {% block title %} Domains | {% endblock %} {% block portfolio_content %} - -{% block messages %} - {% include "includes/form_messages.html" %} -{% endblock %} - -

Domains

diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index e88830156..6140130c8 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -282,3 +282,11 @@ def display_requesting_entity(domain_request): ) return display + + +@register.filter +def get_dict_value(dictionary, key): + """Get a value from a dictionary. Returns a string on empty.""" + if isinstance(dictionary, dict): + return dictionary.get(key, "") + return "" diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index de27b7059..01383ae77 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -2642,3 +2642,160 @@ def test_member_invite_for_existing_member(self): # Validate Database has not changed invite_count_after = PortfolioInvitation.objects.count() self.assertEqual(invite_count_after, invite_count_before) + + +class TestEditPortfolioMemberView(WebTest): + """Tests for the edit member page on portfolios""" + + def setUp(self): + self.user = create_user() + # Create Portfolio + self.portfolio = Portfolio.objects.create(creator=self.user, organization_name="Test Portfolio") + + # Add an invited member who has been invited to manage domains + self.invited_member_email = "invited@example.com" + self.invitation = PortfolioInvitation.objects.create( + email=self.invited_member_email, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + ], + ) + + # Assign permissions to the user making requests + UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + + def tearDown(self): + PortfolioInvitation.objects.all().delete() + UserPortfolioPermission.objects.all().delete() + Portfolio.objects.all().delete() + User.objects.all().delete() + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_edit_member_permissions_basic_to_admin(self): + """Tests converting a basic member to admin with full permissions.""" + self.client.force_login(self.user) + + # Create a basic member to edit + basic_member = create_test_user() + basic_permission = UserPortfolioPermission.objects.create( + user=basic_member, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS], + ) + + response = self.client.post( + reverse("member-permissions", kwargs={"pk": basic_permission.id}), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + "domain_request_permission_admin": UserPortfolioPermissionChoices.EDIT_REQUESTS, + "member_permission_admin": UserPortfolioPermissionChoices.EDIT_MEMBERS, + }, + ) + + # Verify redirect and success message + self.assertEqual(response.status_code, 302) + + # Verify database changes + basic_permission.refresh_from_db() + self.assertEqual(basic_permission.roles, [UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) + self.assertEqual( + set(basic_permission.additional_permissions), + { + UserPortfolioPermissionChoices.EDIT_REQUESTS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + }, + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_edit_member_permissions_validation(self): + """Tests form validation for required fields based on role.""" + self.client.force_login(self.user) + + member = create_test_user() + permission = UserPortfolioPermission.objects.create( + user=member, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + ) + + # Test missing required admin permissions + response = self.client.post( + reverse("member-permissions", kwargs={"pk": permission.id}), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + # Missing required admin fields + }, + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual( + response.context["form"].errors["domain_request_permission_admin"][0], + "Admin domain request permission is required", + ) + self.assertEqual( + response.context["form"].errors["member_permission_admin"][0], "Admin member permission is required" + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_edit_invited_member_permissions(self): + """Tests editing permissions for an invited (but not yet joined) member.""" + self.client.force_login(self.user) + + # Test updating invitation permissions + response = self.client.post( + reverse("invitedmember-permissions", kwargs={"pk": self.invitation.id}), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + "domain_request_permission_admin": UserPortfolioPermissionChoices.EDIT_REQUESTS, + "member_permission_admin": UserPortfolioPermissionChoices.EDIT_MEMBERS, + }, + ) + + self.assertEqual(response.status_code, 302) + + # Verify invitation was updated + updated_invitation = PortfolioInvitation.objects.get(pk=self.invitation.id) + self.assertEqual(updated_invitation.roles, [UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) + self.assertEqual( + set(updated_invitation.additional_permissions), + { + UserPortfolioPermissionChoices.EDIT_REQUESTS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + }, + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_admin_removing_own_admin_role(self): + """Tests an admin removing their own admin role redirects to home.""" + self.client.force_login(self.user) + + # Get the user's admin permission + admin_permission = UserPortfolioPermission.objects.get(user=self.user, portfolio=self.portfolio) + + response = self.client.post( + reverse("member-permissions", kwargs={"pk": admin_permission.id}), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + }, + ) + + self.assertEqual(response.status_code, 302) + self.assertEqual(response["Location"], reverse("home")) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 00c1e6abf..66809777b 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1105,22 +1105,12 @@ class DomainDataFull(DomainExport): # converted_so_email => senior_official__email @classmethod def get_fields(cls, model): - # These vars are copied from the base class. - # converted_generic_org_type => generic_org_type - # converted_federal_type = federal_type - domain_org_type = model.get("generic_org_type") - human_readable_domain_org_type = DomainRequest.OrgChoicesElectionOffice.get_org_label(domain_org_type) - domain_federal_type = model.get("federal_type") - human_readable_domain_federal_type = BranchChoices.get_branch_label(domain_federal_type) - domain_type = human_readable_domain_org_type - if domain_federal_type and domain_org_type == DomainRequest.OrgChoicesElectionOffice.FEDERAL: - domain_type = f"{human_readable_domain_org_type} - {human_readable_domain_federal_type}" FIELDS = { "Domain name": model.get("domain__name"), "Status": model.get("status"), "First ready on": model.get("first_ready_on"), "Expiration date": model.get("expiration_date"), - "Domain type": domain_type, + "Domain type": model.get("domain_type"), "Agency": model.get("federal_agency__agency"), "Organization name": model.get("organization_name"), "City": model.get("city"), @@ -1235,22 +1225,12 @@ class DomainDataFederal(DomainExport): # converted_so_email => senior_official__email @classmethod def get_fields(cls, model): - # These vars are copied from the base class. - # converted_generic_org_type => generic_org_type - # converted_federal_type = federal_type - domain_org_type = model.get("generic_org_type") - human_readable_domain_org_type = DomainRequest.OrgChoicesElectionOffice.get_org_label(domain_org_type) - domain_federal_type = model.get("federal_type") - human_readable_domain_federal_type = BranchChoices.get_branch_label(domain_federal_type) - domain_type = human_readable_domain_org_type - if domain_federal_type and domain_org_type == DomainRequest.OrgChoicesElectionOffice.FEDERAL: - domain_type = f"{human_readable_domain_org_type} - {human_readable_domain_federal_type}" FIELDS = { "Domain name": model.get("domain__name"), "Status": model.get("status"), "First ready on": model.get("first_ready_on"), "Expiration date": model.get("expiration_date"), - "Domain type": domain_type, + "Domain type": model.get("domain_type"), "Agency": model.get("federal_agency__agency"), "Organization name": model.get("organization_name"), "City": model.get("city"), diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 90313339b..855194f6b 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -6,7 +6,6 @@ from django.urls import reverse from django.utils.safestring import mark_safe from django.contrib import messages - from registrar.forms import portfolio as portfolioForms from registrar.models import Portfolio, User from registrar.models.portfolio_invitation import PortfolioInvitation @@ -144,7 +143,7 @@ def post(self, request, pk): class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): template_name = "portfolio_member_permissions.html" - form_class = portfolioForms.PortfolioMemberForm + form_class = portfolioForms.BasePortfolioMemberForm def get(self, request, pk): portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) @@ -164,12 +163,17 @@ def get(self, request, pk): def post(self, request, pk): portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) user = portfolio_permission.user - form = self.form_class(request.POST, instance=portfolio_permission) - if form.is_valid(): + # Check if user is removing their own admin or edit role + removing_admin_role_on_self = ( + request.user == user + and UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_permission.roles + and UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in form.cleaned_data.get("role", []) + ) form.save() - return redirect("member", pk=pk) + messages.success(self.request, "The member access and permission changes have been saved.") + return redirect("member", pk=pk) if not removing_admin_role_on_self else redirect("home") return render( request, @@ -278,7 +282,7 @@ def post(self, request, pk): class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View): template_name = "portfolio_member_permissions.html" - form_class = portfolioForms.PortfolioInvitedMemberForm + form_class = portfolioForms.BasePortfolioMemberForm def get(self, request, pk): portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) @@ -298,6 +302,7 @@ def post(self, request, pk): form = self.form_class(request.POST, instance=portfolio_invitation) if form.is_valid(): form.save() + messages.success(self.request, "The member access and permission changes have been saved.") return redirect("invitedmember", pk=pk) return render( diff --git a/src/zap.conf b/src/zap.conf index 65468773a..a0a60bdc7 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -70,6 +70,7 @@ 10038 OUTOFSCOPE http://app:8080/org-name-address 10038 OUTOFSCOPE http://app:8080/domain_requests/ 10038 OUTOFSCOPE http://app:8080/domains/ +10038 OUTOFSCOPE http://app:8080/domains/edit 10038 OUTOFSCOPE http://app:8080/organization/ 10038 OUTOFSCOPE http://app:8080/permissions 10038 OUTOFSCOPE http://app:8080/suborganization/