From 24480ec434b77fc9c6d8591cbe7e61f4dcef8340 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 13 Dec 2024 11:54:52 -0700
Subject: [PATCH] Polish of paint

---
 src/registrar/admin.py                |  3 +++
 src/registrar/forms/portfolio.py      | 30 +++++++++++++++++----------
 src/registrar/views/portfolios.py     | 27 ++++++++----------------
 src/registrar/views/utility/mixins.py |  1 +
 4 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 44b8d7345..6afa78a55 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -3791,7 +3791,10 @@ 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)
 
 
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 5549aa11e..9101bcbf8 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -124,8 +124,7 @@ class BasePortfolioMemberForm(forms.Form):
     )
 
     domain_request_permission_admin = forms.ChoiceField(
-        # nosec B308 - required_star is a hardcoded HTML string
-        label=mark_safe(f"Select permission {required_star}"),
+        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"),
@@ -138,8 +137,7 @@ class BasePortfolioMemberForm(forms.Form):
     )
 
     member_permission_admin = forms.ChoiceField(
-        # nosec B308 - required_star is a hardcoded HTML string
-        label=mark_safe(f"Select permission {required_star}"),
+        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"),
@@ -153,7 +151,7 @@ class BasePortfolioMemberForm(forms.Form):
 
     domain_request_permission_member = forms.ChoiceField(
         # nosec B308 - required_star is a hardcoded HTML string
-        label=mark_safe(f"Select permission {required_star}"),
+        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"),
@@ -195,7 +193,7 @@ class BasePortfolioMemberForm(forms.Form):
     def clean(self):
         """
         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"]
@@ -290,17 +288,17 @@ 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
-            
+
         Returns:
             instance: Updated instance
         """
@@ -355,7 +353,7 @@ class NewMemberForm(BasePortfolioMemberForm):
     )
 
     def __init__(self, *args, **kwargs):
-        self.portfolio = kwargs.pop('portfolio', None)
+        self.portfolio = kwargs.pop("portfolio", None)
         super().__init__(*args, **kwargs)
 
     def clean(self):
@@ -369,7 +367,7 @@ class NewMemberForm(BasePortfolioMemberForm):
             # Check if user is already a member
             if UserPortfolioPermission.objects.filter(user__email=email_value, portfolio=self.portfolio).exists():
                 self.add_error("email", "User is already a member of this portfolio.")
-            
+
             if PortfolioInvitation.objects.filter(email=email_value, portfolio=self.portfolio).exists():
                 self.add_error("email", "An invitation already exists for this user.")
         ##########################################
@@ -383,3 +381,13 @@ class NewMemberForm(BasePortfolioMemberForm):
         #     except User.DoesNotExist:
         #         raise forms.ValidationError("User with this email does not exist.")
         return cleaned_data
+
+    def map_cleaned_data_to_instance(self, cleaned_data, instance):
+        """Override of the base class to add portfolio and email."""
+        instance = super().map_cleaned_data_to_instance(cleaned_data, instance)
+        email = cleaned_data.get("email")
+        if email and isinstance(email, str):
+            email = email.lower()
+        instance.email = email
+        instance.portfolio = self.portfolio
+        return instance
diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py
index 7b80221d9..95b7238ff 100644
--- a/src/registrar/views/portfolios.py
+++ b/src/registrar/views/portfolios.py
@@ -163,21 +163,17 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View):
     def post(self, request, pk):
         portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk)
         user = portfolio_permission.user
-        is_editing_self = request.user == 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
-            old_roles = set(portfolio_permission.roles)
-            new_roles = set(form.cleaned_data.get("role", []))
-            removing_admin_role = (
-                is_editing_self 
-                and UserPortfolioRoleChoices.ORGANIZATION_ADMIN in old_roles
-                and UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in new_roles
+            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 else redirect("home")
+            return redirect("member", pk=pk) if not removing_admin_role_on_self else redirect("home")
 
         return render(
             request,
@@ -518,7 +514,7 @@ class NewMemberView(PortfolioInvitationCreatePermissionView):
     def get_form_kwargs(self):
         """Pass request and portfolio to form."""
         kwargs = super().get_form_kwargs()
-        kwargs['portfolio'] = self.request.session.get("portfolio")
+        kwargs["portfolio"] = self.request.session.get("portfolio")
         return kwargs
 
     def get_success_url(self):
@@ -535,14 +531,9 @@ class NewMemberView(PortfolioInvitationCreatePermissionView):
         # if not send_success:
         #     return
 
-        # Create instance using form's mapping method
-        self.object = form.map_cleaned_data_to_instance(
-            form.cleaned_data, 
-            PortfolioInvitation(
-                email=form.cleaned_data.get("email"),
-                portfolio=self.request.session.get("portfolio")
-            )
-        )
+        # Create instance using form's mapping method.
+        # Pass in a new object since we are adding a new record.
+        self.object = form.map_cleaned_data_to_instance(form.cleaned_data, PortfolioInvitation())
         self.object.save()
         messages.success(self.request, f"{self.object.email} has been invited.")
         return redirect(self.get_success_url())
diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py
index 155fa9f11..e62944c40 100644
--- a/src/registrar/views/utility/mixins.py
+++ b/src/registrar/views/utility/mixins.py
@@ -483,6 +483,7 @@ class PortfolioInvitationCreatePermission(PortfolioBasePermission):
         portfolio = self.request.session.get("portfolio")
         return self.request.user.has_edit_members_portfolio_permission(portfolio)
 
+
 class PortfolioDomainsPermission(PortfolioBasePermission):
     """Permission mixin that allows access to portfolio domain pages if user
     has access, otherwise 403"""