diff --git a/src/registrar/assets/src/js/getgov/main.js b/src/registrar/assets/src/js/getgov/main.js index f5ebc83a3..bd4bed01b 100644 --- a/src/registrar/assets/src/js/getgov/main.js +++ b/src/registrar/assets/src/js/getgov/main.js @@ -10,7 +10,8 @@ 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 { initPortfolioNewMemberPageToggle, initAddNewMemberPageListeners, initPortfolioMemberPageRadio } from './portfolio-member-page.js'; +import { initPortfolioMemberPageToggle } from './portfolio-member-page.js'; +import { initAddNewMemberPageListeners } from './portfolio-member-page.js'; initDomainValidators(); @@ -20,6 +21,13 @@ 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(); @@ -36,7 +44,5 @@ initMembersTable(); initMemberDomainsTable(); initEditMemberDomainsTable(); -// Init the portfolio new member page -initPortfolioMemberPageRadio(); -initPortfolioNewMemberPageToggle(); +initPortfolioMemberPageToggle(); 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 02d927438..ba874cfb1 100644 --- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js +++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js @@ -2,10 +2,9 @@ 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 initPortfolioNewMemberPageToggle() { +export function initPortfolioMemberPageToggle() { document.addEventListener("DOMContentLoaded", () => { const wrapperDeleteAction = document.getElementById("wrapper-delete-action") if (wrapperDeleteAction) { @@ -170,29 +169,4 @@ export function initAddNewMemberPageListeners() { } } -} - -// 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' - } - ); - } - }); -} +} \ No newline at end of file diff --git a/src/registrar/assets/src/js/getgov/radios.js b/src/registrar/assets/src/js/getgov/radios.js index 055bdf621..248865e8b 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 && radioButtons.length) { + + if (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 eaa885a85..5309f7263 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -4,7 +4,6 @@ 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, @@ -272,210 +271,3 @@ 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 25abb6748..a149a9476 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -110,13 +110,8 @@ def _get_portfolio_permissions(self): return self.get_portfolio_permissions(self.roles, self.additional_permissions) @classmethod - 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. - """ + def get_portfolio_permissions(cls, roles, additional_permissions): + """Class method to return a list of permissions based on roles and addtl permissions""" # Use a set to avoid duplicate permissions portfolio_permissions = set() if roles: @@ -124,7 +119,7 @@ def get_portfolio_permissions(cls, roles, additional_permissions, get_list=True) portfolio_permissions.update(cls.PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) if additional_permissions: portfolio_permissions.update(additional_permissions) - return list(portfolio_permissions) if get_list else portfolio_permissions + return list(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 cde28e4bd..3768aa77a 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -4,9 +4,6 @@ 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): @@ -19,28 +16,7 @@ class UserPortfolioRoleChoices(models.TextChoices): @classmethod def get_user_portfolio_role_label(cls, user_portfolio_role): - 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) + return cls(user_portfolio_role).label if user_portfolio_role else None 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 cc0e11989..90c241366 100644 --- a/src/registrar/templates/django/forms/widgets/multiple_input.html +++ b/src/registrar/templates/django/forms/widgets/multiple_input.html @@ -1,5 +1,3 @@ -{% load static custom_filters %} -

{% for group, options, index in widget.optgroups %} {% if group %}
{% endif %} @@ -15,17 +13,7 @@ + >{{ option.label }} {% endfor %} {% if group %}
{% endif %} {% endfor %} diff --git a/src/registrar/templates/portfolio_member_permissions.html b/src/registrar/templates/portfolio_member_permissions.html index 66becaa9e..02d120360 100644 --- a/src/registrar/templates/portfolio_member_permissions.html +++ b/src/registrar/templates/portfolio_member_permissions.html @@ -1,132 +1,42 @@ {% extends 'portfolio_base.html' %} -{% load static url_helpers %} -{% load field_helpers %} +{% load static field_helpers%} -{% block title %}Organization member{% endblock %} +{% block title %}Organization member {% endblock %} -{% block wrapper_class %} - {{ block.super }} dashboard--grey-1 -{% endblock %} +{% load static %} {% block portfolio_content %} -{% 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%} +
+
+ + {% 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 %} diff --git a/src/registrar/templates/portfolio_no_domains.html b/src/registrar/templates/portfolio_no_domains.html index 995f391a2..ac6a8c036 100644 --- a/src/registrar/templates/portfolio_no_domains.html +++ b/src/registrar/templates/portfolio_no_domains.html @@ -5,6 +5,12 @@ {% 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 6140130c8..e88830156 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -282,11 +282,3 @@ 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 01383ae77..de27b7059 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -2642,160 +2642,3 @@ 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 66809777b..00c1e6abf 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1105,12 +1105,22 @@ 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": model.get("domain_type"), + "Domain type": domain_type, "Agency": model.get("federal_agency__agency"), "Organization name": model.get("organization_name"), "City": model.get("city"), @@ -1225,12 +1235,22 @@ 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": model.get("domain_type"), + "Domain type": 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 855194f6b..90313339b 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -6,6 +6,7 @@ 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 @@ -143,7 +144,7 @@ def post(self, request, pk): class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): template_name = "portfolio_member_permissions.html" - form_class = portfolioForms.BasePortfolioMemberForm + form_class = portfolioForms.PortfolioMemberForm def get(self, request, pk): portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) @@ -163,17 +164,12 @@ 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() - 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 redirect("member", pk=pk) return render( request, @@ -282,7 +278,7 @@ def post(self, request, pk): class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View): template_name = "portfolio_member_permissions.html" - form_class = portfolioForms.BasePortfolioMemberForm + form_class = portfolioForms.PortfolioInvitedMemberForm def get(self, request, pk): portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) @@ -302,7 +298,6 @@ 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 a0a60bdc7..65468773a 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -70,7 +70,6 @@ 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/