PR suggestions

src/registrar/admin.py

@@ -4004,10 +4004,6 @@ class WaffleFlagAdmin(FlagAdmin):
         if extra_context is None:
             extra_context = {}
         extra_context["dns_prototype_flag"] = flag_is_active_for_user(request.user, "dns_prototype_flag")
-        # Normally you have to first enable the org feature then navigate to an org before you see these.
-        # Lets just auto-populate it on page load to make development easier.
-        extra_context["organization_members"] = flag_is_active_for_user(request.user, "organization_members")
-        extra_context["organization_requests"] = flag_is_active_for_user(request.user, "organization_requests")
         return super().changelist_view(request, extra_context=extra_context)
 
 

src/registrar/forms/__init__.py

@@ -13,4 +13,5 @@ from .domain import (
 )
 from .portfolio import (
     PortfolioOrgAddressForm,
+    PortfolioMemberForm,
 )

src/registrar/forms/portfolio.py

@@ -273,7 +273,11 @@ class NewMemberForm(forms.ModelForm):
                 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 = '<abbr class="usa-hint usa-hint--required" title="required">*</abbr>'
     role = forms.ChoiceField(
         choices=[
@@ -354,25 +358,11 @@ class BasePortfolioMemberForm(forms.Form):
         }
 
     def clean(self):
-        """
+        """Validates form data based on selected role and its required fields."""
-        Validates form data based on selected role and its required fields.
-
-        Since form fields are dynamically shown/hidden via JavaScript based on role selection,
-        we only validate fields that are relevant to the selected role:
-        - organization_admin: ["member_permission_admin", "domain_request_permission_admin"]
-        - organization_member: ["domain_request_permission_member"]
-        This ensures users aren't required to fill out hidden fields and maintains
-        proper validation based on their role selection.
-
-        NOTE: This page uses ROLE_REQUIRED_FIELDS for the aforementioned mapping.
-        Raises:
-            ValueError: If ROLE_REQUIRED_FIELDS references a non-existent form field
-        """
         cleaned_data = super().clean()
         role = cleaned_data.get("role")
 
-        # Get required fields for the selected role.
+        # Get required fields for the selected role. Then validate all required fields for the 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
@@ -394,15 +384,17 @@ class BasePortfolioMemberForm(forms.Form):
         self.instance.save()
         return self.instance
 
+    # Explanation of how map_instance_to_form / map_cleaned_data_to_instance work:
+    # map_instance_to_form => called on init to set self.instance.
+    # 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_form(self, instance):
         """
         Maps user instance to form fields, handling roles and permissions.
-
-        Determines:
-        - User's role (admin vs member)
-        - Domain request permissions (EDIT_REQUESTS, VIEW_ALL_REQUESTS, or "no_access")
-        - Member management permissions (EDIT_MEMBERS or VIEW_MEMBERS)
-
         Returns form data dictionary with appropriate permission levels based on user role:
         {
             "role": "organization_admin" or "organization_member",
@@ -462,13 +454,6 @@ class BasePortfolioMemberForm(forms.Form):
     def map_cleaned_data_to_instance(self, cleaned_data, instance):
         """
         Maps cleaned data to a member instance, setting roles and permissions.
-
-        Additional permissions logic:
-        - For org admins: Adds domain request and member admin permissions if selected
-        - For other roles: Adds domain request member permissions if not 'no_access'
-        - Automatically adds VIEW permissions when EDIT permissions are granted
-        - Filters out permissions already granted by base role
-
         Args:
             cleaned_data (dict): Cleaned data containing role and permission choices
             instance: Instance to update

src/registrar/models/user_portfolio_permission.py

@@ -111,7 +111,12 @@ class UserPortfolioPermission(TimeStampedModel):
 
     @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"""
+        """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:

src/registrar/views/utility/permission_views.py

@@ -1,6 +1,7 @@
 """View classes that enforce authorization."""
 
 import abc  # abstract base class
+
 from django.views.generic import DetailView, DeleteView, TemplateView, UpdateView
 from registrar.models import Domain, DomainRequest, DomainInvitation, Portfolio
 from registrar.models.user import User