refactor form and view for add portfolio member

src/registrar/assets/src/js/getgov/main.js

@@ -23,8 +23,8 @@ hookupYesNoListener("additional_details-has_anything_else_text",'anything-else',
 hookupRadioTogglerListener(
   'member_access_level', 
   {
-    'admin': 'new-member-admin-permissions',
+    'organization_admin': 'new-member-admin-permissions',
-    'basic': 'new-member-basic-permissions'
+    'organization_member': 'new-member-basic-permissions'
   }
 );
 hookupYesNoListener("additional_details-has_cisa_representative",'cisa-representative', null);

src/registrar/assets/src/js/getgov/portfolio-member-page.js

@@ -49,7 +49,7 @@ export function initPortfolioMemberPageToggle() {
  * on the Add New Member page.
  */
 export function initAddNewMemberPageListeners() {
-  add_member_form = document.getElementById("add_member_form")
+  let add_member_form = document.getElementById("add_member_form")
   if (!add_member_form){
      return;
   }
@@ -156,7 +156,7 @@ export function initAddNewMemberPageListeners() {
       document.getElementById('modalAccessLevel').textContent = accessText;
 
       // Populate permission details based on access level
-      if (selectedAccess && selectedAccess.value === 'admin') {
+      if (selectedAccess && selectedAccess.value === 'organization_admin') {
         populatePermissionDetails('new-member-admin-permissions');
       } else {
         populatePermissionDetails('new-member-basic-permissions');

src/registrar/config/urls.py

@@ -136,7 +136,7 @@ urlpatterns = [
     # ),
     path(
         "members/new-member/",
-        views.PortfolioNewMemberView.as_view(),
+        views.PortfolioAddMemberView.as_view(),
         name="new-member",
     ),
     path(

src/registrar/forms/portfolio.py

@@ -167,7 +167,7 @@ class PortfolioInvitedMemberForm(forms.ModelForm):
 class PortfolioNewMemberForm(forms.ModelForm):
     member_access_level = forms.ChoiceField(
         label="Select permission",
-        choices=[("admin", "Admin Access"), ("basic", "Basic Access")],
+        choices=[("organization_admin", "Admin Access"), ("organization_member", "Basic Access")],
         widget=forms.RadioSelect(attrs={"class": "usa-radio__input  usa-radio__input--tile"}),
         required=True,
         error_messages={
@@ -176,7 +176,7 @@ class PortfolioNewMemberForm(forms.ModelForm):
     )
     admin_org_domain_request_permissions = forms.ChoiceField(
         label="Select permission",
-        choices=[("view_only", "View all requests"), ("view_and_create", "View all requests plus create requests")],
+        choices=[("view_all_requests", "View all requests"), ("edit_requests", "View all requests plus create requests")],
         widget=forms.RadioSelect,
         required=True,
         error_messages={
@@ -185,7 +185,7 @@ class PortfolioNewMemberForm(forms.ModelForm):
     )
     admin_org_members_permissions = forms.ChoiceField(
         label="Select permission",
-        choices=[("view_only", "View all members"), ("view_and_create", "View all members plus manage members")],
+        choices=[("view_members", "View all members"), ("edit_members", "View all members plus manage members")],
         widget=forms.RadioSelect,
         required=True,
         error_messages={
@@ -195,9 +195,9 @@ class PortfolioNewMemberForm(forms.ModelForm):
     basic_org_domain_request_permissions = forms.ChoiceField(
         label="Select permission",
         choices=[
-            ("view_only", "View all requests"),
+            ("view_all_requests", "View all requests"),
-            ("view_and_create", "View all requests plus create requests"),
+            ("edit_requests", "View all requests plus create requests"),
-            ("no_access", "No access"),
+            ("", "No access"),
         ],
         widget=forms.RadioSelect,
         required=True,
@@ -226,52 +226,52 @@ class PortfolioNewMemberForm(forms.ModelForm):
         model = PortfolioInvitation
         fields = ["email"]
 
-    def _post_clean(self):
-        logger.info("in _post_clean")
-        super()._post_clean()
-
     def clean(self):
-        cleaned_data = super().clean()
-
         # Lowercase the value of the 'email' field
-        email_value = cleaned_data.get("email")
+        email_value = self.cleaned_data.get("email")
         if email_value:
-            cleaned_data["email"] = email_value.lower()
+            self.cleaned_data["email"] = email_value.lower()
 
-        ##########################################
+        # Get the selected member access level
-        # TODO: future ticket
+        member_access_level = self.cleaned_data.get("member_access_level")
-        # (invite new member)
-        ##########################################
-        # Check for an existing user (if there isn't any, send an invite)
-        # if email_value:
-        #     try:
-        #         existingUser = User.objects.get(email=email_value)
-        #     except User.DoesNotExist:
-        #         raise forms.ValidationError("User with this email does not exist.")
 
-        member_access_level = cleaned_data.get("member_access_level")
+        # If no member access level is selected, remove errors for hidden inputs
-
-        # Intercept the error messages so that we don't validate hidden inputs
         if not member_access_level:
-            # If no member access level has been selected, delete error messages
+            self._remove_hidden_field_errors(exclude_fields=["email", "member_access_level"])
-            # for all hidden inputs (which is everything except the e-mail input
+            return self.cleaned_data
-            # and member access selection)
+
-            for field in self.fields:
+        # Define field names for validation cleanup
-                if field in self.errors and field != "email" and field != "member_access_level":
+        field_error_map = {
+            "organization_admin": ["basic_org_domain_request_permissions"],  # Fields irrelevant to "admin"
+            "organization_member": ["admin_org_domain_request_permissions", "admin_org_members_permissions"],  # Fields irrelevant to "basic"
+        }
+
+        # Remove errors for irrelevant fields based on the selected access level
+        irrelevant_fields = field_error_map.get(member_access_level, [])
+        for field in irrelevant_fields:
+            if field in self.errors:
                 del self.errors[field]
-            return cleaned_data
 
-        basic_dom_req_error = "basic_org_domain_request_permissions"
+        # Map roles and additional permissions to cleaned_data
-        admin_dom_req_error = "admin_org_domain_request_permissions"
+        self.cleaned_data["roles"] = [member_access_level]
-        admin_member_error = "admin_org_members_permissions"
+        additional_permissions = [
+            self.cleaned_data.get("admin_org_domain_request_permissions"),
+            self.cleaned_data.get("basic_org_domain_request_permissions"),
+            self.cleaned_data.get("admin_org_members_permissions"),
+        ]
+        # Filter out None values
+        self.cleaned_data["additional_permissions"] = [perm for perm in additional_permissions if perm]
+
+        return super().clean()
+
+    def _remove_hidden_field_errors(self, exclude_fields=None):
+        """
+        Helper method to remove errors for fields that are not relevant
+        (e.g., hidden inputs), except for explicitly excluded fields.
+        """
+        exclude_fields = exclude_fields or []
+        hidden_fields = [field for field in self.fields if field not in exclude_fields]
+        for field in hidden_fields:
+            if field in self.errors:
+                del self.errors[field]
 
-        if member_access_level == "admin" and basic_dom_req_error in self.errors:
-            # remove the error messages pertaining to basic permission inputs
-            del self.errors[basic_dom_req_error]
-        elif member_access_level == "basic":
-            # remove the error messages pertaining to admin permission inputs
-            if admin_dom_req_error in self.errors:
-                del self.errors[admin_dom_req_error]
-            if admin_member_error in self.errors:
-                del self.errors[admin_member_error]
-        return cleaned_data

src/registrar/models/portfolio_invitation.py

@@ -111,11 +111,8 @@ class PortfolioInvitation(TimeStampedModel):
             user_portfolio_permission.additional_permissions = self.additional_permissions
         user_portfolio_permission.save()
 
-    def full_clean(self, exclude=None, validate_unique=True):
-        logger.info("portfolio invitation full clean")
-        super().full_clean(exclude, validate_unique)
-
     def clean(self):
         """Extends clean method to perform additional validation, which can raise errors in django admin."""
+        print(f'portfolio invitation model clean')
         super().clean()
         validate_portfolio_invitation(self)

src/registrar/models/utility/portfolio_helper.py

@@ -155,7 +155,6 @@ def validate_portfolio_invitation(portfolio_invitation):
     Raises:
         ValidationError: If any of the validation rules are violated.
     """
-    logger.info("portfolio invitataion validation")
     PortfolioInvitation = apps.get_model("registrar.PortfolioInvitation")
     UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission")
     User = get_user_model()
@@ -163,11 +162,21 @@ def validate_portfolio_invitation(portfolio_invitation):
     has_portfolio = bool(portfolio_invitation.portfolio_id)
     portfolio_permissions = set(portfolio_invitation.get_portfolio_permissions())
 
+    print(f"has_portfolio {has_portfolio}")
+
+    print(f"portfolio_permissions {portfolio_permissions}")
+
+    print(f"roles {portfolio_invitation.roles}")
+
+    print(f"additional permissions {portfolio_invitation.additional_permissions}")
+
     # == Validate required fields == #
     if not has_portfolio and portfolio_permissions:
+        print(f"not has_portfolio and portfolio_permissions {portfolio_permissions}")
         raise ValidationError("When portfolio roles or additional permissions are assigned, portfolio is required.")
 
     if has_portfolio and not portfolio_permissions:
+        print(f"has_portfolio and not portfolio_permissions {portfolio_permissions}")
         raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.")
 
     # == Validate role permissions. Compares existing permissions to forbidden ones. == #

src/registrar/templates/portfolio_members_add_new.html

@@ -134,7 +134,6 @@
     id="invite-member-modal"
     aria-labelledby="invite-member-heading"
     aria-describedby="confirm-invite-description"
-    style="display: none;"
 >
     <div class="usa-modal__content">
         <div class="usa-modal__main">

src/registrar/utility/email_invitations.py

@@ -114,7 +114,7 @@ def send_portfolio_invitation_email(email: str, requestor, portfolio):
         if invite.status == PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED:
             raise AlreadyPortfolioMemberError(email)
         else:
-            raise AlreadyPortfolioInvitedError(email)
+            raise AlreadyPortfolioInvitedError(email, portfolio)
     except PortfolioInvitation.DoesNotExist:
         pass
 

src/registrar/utility/errors.py

@@ -53,8 +53,8 @@ class AlreadyPortfolioMemberError(InvitationError):
 class AlreadyPortfolioInvitedError(InvitationError):
     """Raised when the user has already been invited to the portfolio."""
 
-    def __init__(self, email):
+    def __init__(self, email, portfolio):
-        super().__init__(f"{email} has already been invited to this portfolio.")
+        super().__init__(f"{email} has already been invited to {portfolio}.")
 
 
 class MissingEmailError(InvitationError):

src/registrar/views/portfolios.py

@@ -469,82 +469,82 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View):
         return render(request, "portfolio_members.html")
 
 
-class PortfolioNewMemberView(PortfolioMembersPermissionView, FormMixin):
+class PortfolioAddMemberView(PortfolioMembersPermissionView, FormMixin):
 
     template_name = "portfolio_members_add_new.html"
     form_class = portfolioForms.PortfolioNewMemberForm
 
-    # def get_object(self, queryset=None):
-    #     """Get the portfolio object based on the session."""
-    #     portfolio = self.request.session.get("portfolio")
-    #     if portfolio is None:
-    #         raise Http404("No organization found for this user")
-    #     return portfolio
-
-    # def get_form_kwargs(self):
-    #     """Include the instance in the form kwargs."""
-    #     kwargs = super().get_form_kwargs()
-    #     kwargs["instance"] = self.get_object()
-    #     return kwargs
-
     def get(self, request, *args, **kwargs):
         """Handle GET requests to display the form."""
-        self.object = self.request.session.get("portfolio")
+        self.object = None  # No existing PortfolioInvitation instance
         form = self.get_form()
         return self.render_to_response(self.get_context_data(form=form))
     
+    def is_ajax(self):
+        return self.request.headers.get("X-Requested-With") == "XMLHttpRequest"
+    
+    def form_invalid(self, form):
+        if self.is_ajax():
+            return JsonResponse({"is_valid": False})  # Return a JSON response
+        else:
+            return super().form_invalid(form)  # Handle non-AJAX requests normally
+        
+    def form_valid(self, form):
+
+        if self.is_ajax():
+            return JsonResponse({"is_valid": True})  # Return a JSON response
+        else:
+            return self.submit_new_member(form)
+
     def post(self, request, *args, **kwargs):
         """Handle POST requests to process form submission."""
-        
+        self.object = None  # For a new invitation, there's no existing model instance
-        # self.object = self.get_object()
         form = self.get_form()
 
+        print('before is_valid')
         if form.is_valid():
+            print('form is_valid')
             return self.form_valid(form)
         else:
+            print('form NOT is_valid')
             return self.form_invalid(form)
 
-    # def is_ajax(self):
-    #     return self.request.headers.get("X-Requested-With") == "XMLHttpRequest"
-
-    # def form_invalid(self, form):
-    #     if self.is_ajax():
-    #         return JsonResponse({"is_valid": False})  # Return a JSON response
-    #     else:
-    #         return super().form_invalid(form)  # Handle non-AJAX requests normally
-
-    # def form_valid(self, form):
-
-    #     if self.is_ajax():
-    #         return JsonResponse({"is_valid": True})  # Return a JSON response
-    #     else:
-    #         return self.submit_new_member(form)
-
     def get_success_url(self):
         """Redirect to members table."""
         return reverse("members")
 
-    def form_valid(self, form):
+    def submit_new_member(self, form):
         """Add the specified user as a member for this portfolio."""
-        requested_email = form.cleaned_data["email"]
+        # Retrieve the portfolio from the session
-        requestor = self.request.user
         portfolio = self.request.session.get("portfolio")
+        if not portfolio:
+            messages.error(self.request, "No portfolio found in session.")
+            return self.form_invalid(form)
+        
+        # Save the invitation instance
+        invitation = form.save(commit=False)
+        invitation.portfolio = portfolio
+
+        # Send invitation email and show a success message
+        send_portfolio_invitation_email(
+            email=invitation.email,
+            requestor=self.request.user,
+            portfolio=portfolio,
+        )
+
+       # Use processed data from the form
+        invitation.roles = form.cleaned_data["roles"]
+        invitation.additional_permissions = form.cleaned_data["additional_permissions"]
+        invitation.save()
+
+        messages.success(self.request, f"{invitation.email} has been invited.")
 
-        requested_user = User.objects.filter(email=requested_email).first()
-        permission_exists = UserPortfolioPermission.objects.filter(user=requested_user, portfolio=portfolio).exists()
-        try:
-            if not requested_user or not permission_exists:
-                send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=portfolio)
-                ## NOTE : this is not yet accounting properly for roles and permissions
-                PortfolioInvitation.objects.get_or_create(email=requested_email, portfolio=portfolio)
-                messages.success(self.request, f"{requested_email} has been invited.")
-            else:
-                if permission_exists:
-                    messages.warning(self.request, "User is already a member of this portfolio.")
-        except Exception as e:
-            self._handle_exceptions(e, portfolio, requested_email)
         return redirect(self.get_success_url())
 
+    def get_success_url(self):
+        """Redirect to the members page."""
+        return reverse("members")
+
     def _handle_exceptions(self, exception, portfolio, email):
         """Handle exceptions raised during the process."""
         if isinstance(exception, EmailSendingError):