@@ -176,4 +190,3 @@
{% endblock portfolio_content%}
-
diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py
index 53c500f51..a238de3fc 100644
--- a/src/registrar/views/portfolios.py
+++ b/src/registrar/views/portfolios.py
@@ -4,7 +4,7 @@ from django.shortcuts import get_object_or_404, redirect, render
from django.urls import reverse
from django.utils.safestring import mark_safe
from django.contrib import messages
-
+from django.conf import settings
from registrar.forms import portfolio as portfolioForms
from registrar.models import Portfolio, User
from registrar.models.portfolio_invitation import PortfolioInvitation
@@ -16,7 +16,6 @@ from registrar.views.utility.permission_views import (
PortfolioDomainsPermissionView,
PortfolioBasePermissionView,
NoPortfolioDomainsPermissionView,
- PortfolioInvitationCreatePermissionView,
PortfolioMemberDomainsPermissionView,
PortfolioMemberDomainsEditPermissionView,
PortfolioMemberEditPermissionView,
@@ -506,45 +505,163 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View):
return render(request, "portfolio_members.html")
-class NewMemberView(PortfolioInvitationCreatePermissionView):
+
+class NewMemberView(PortfolioMembersPermissionView, FormMixin):
+
template_name = "portfolio_members_add_new.html"
form_class = portfolioForms.NewMemberForm
+ 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):
- """Pass request and portfolio to form."""
+ """Include the instance in the form kwargs."""
kwargs = super().get_form_kwargs()
- kwargs["portfolio"] = self.request.session.get("portfolio")
+ kwargs["instance"] = self.get_object()
return kwargs
- def get_success_url(self):
- return reverse("members")
+ def get(self, request, *args, **kwargs):
+ """Handle GET requests to display the form."""
+ self.object = self.get_object()
+ form = self.get_form()
+ return self.render_to_response(self.get_context_data(form=form))
- def form_valid(self, form):
- """Create portfolio invitation from form data."""
- if self.is_ajax():
- return JsonResponse({"is_valid": True})
+ def post(self, request, *args, **kwargs):
+ """Handle POST requests to process form submission."""
+ self.object = self.get_object()
+ form = self.get_form()
- # TODO: #3019 - this will probably have to be a small try/catch. Stub for posterity.
- # requested_email = form.cleaned_data.get("email")
- # send_success = self.send_portfolio_invitation_email(requested_email)
- # if not send_success:
- # return
-
- # 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())
-
- # TODO: #3019
- # def send_portfolio_invitation_email(self, email):
- # pass
-
- def form_invalid(self, form):
- if self.is_ajax():
- return JsonResponse({"is_valid": False})
- return super().form_invalid(form)
+ if form.is_valid():
+ return self.form_valid(form)
+ else:
+ 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 _send_portfolio_invitation_email(self, email: str, requestor: User, add_success=True):
+ """Performs the sending of the member invitation email
+ email: string- email to send to
+ add_success: bool- default True indicates:
+ adding a success message to the view if the email sending succeeds
+
+ raises EmailSendingError
+ """
+
+ # Set a default email address to send to for staff
+ requestor_email = settings.DEFAULT_FROM_EMAIL
+
+ # Check if the email requestor has a valid email address
+ if not requestor.is_staff and requestor.email is not None and requestor.email.strip() != "":
+ requestor_email = requestor.email
+ elif not requestor.is_staff:
+ messages.error(self.request, "Can't send invitation email. No email is associated with your account.")
+ logger.error(
+ f"Can't send email to '{email}' on domain '{self.object}'."
+ f"No email exists for the requestor '{requestor.username}'.",
+ exc_info=True,
+ )
+ return None
+
+ # Check to see if an invite has already been sent
+ try:
+ invite = PortfolioInvitation.objects.get(email=email, portfolio=self.object)
+ if invite: # We have an existin invite
+ # check if the invite has already been accepted
+ if invite.status == PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED:
+ add_success = False
+ messages.warning(
+ self.request,
+ f"{email} is already a manager for this portfolio.",
+ )
+ else:
+ add_success = False
+ # it has been sent but not accepted
+ messages.warning(self.request, f"{email} has already been invited to this portfolio")
+ return
+ except Exception as err:
+ logger.error(f"_send_portfolio_invitation_email() => An error occured: {err}")
+
+ try:
+ logger.debug("requestor email: " + requestor_email)
+
+ # send_templated_email(
+ # "emails/portfolio_invitation.txt",
+ # "emails/portfolio_invitation_subject.txt",
+ # to_address=email,
+ # context={
+ # "portfolio": self.object,
+ # "requestor_email": requestor_email,
+ # },
+ # )
+ except EmailSendingError as exc:
+ logger.warn(
+ "Could not sent email invitation to %s for domain %s",
+ email,
+ self.object,
+ exc_info=True,
+ )
+ raise EmailSendingError("Could not send email invitation.") from exc
+ else:
+ if add_success:
+ messages.success(self.request, f"{email} has been invited.")
+
+ def _make_invitation(self, email_address: str, requestor: User, add_success=True):
+ """Make a Member invitation for this email and redirect with a message."""
+ try:
+ self._send_portfolio_invitation_email(email=email_address, requestor=requestor, add_success=add_success)
+ except EmailSendingError:
+ logger.warn(
+ "Could not send email invitation (EmailSendingError)",
+ self.object,
+ exc_info=True,
+ )
+ messages.warning(self.request, "Could not send email invitation.")
+ except Exception:
+ logger.warn(
+ "Could not send email invitation (Other Exception)",
+ self.object,
+ exc_info=True,
+ )
+ messages.warning(self.request, "Could not send email invitation.")
+ else:
+ # (NOTE: only create a MemberInvitation if the e-mail sends correctly)
+ PortfolioInvitation.objects.get_or_create(email=email_address, portfolio=self.object)
+ return redirect(self.get_success_url())
+
+ def submit_new_member(self, form):
+ """Add the specified user as a member
+ for this portfolio.
+ Throws EmailSendingError."""
+ requested_email = form.cleaned_data["email"]
+ requestor = self.request.user
+
+ requested_user = User.objects.filter(email=requested_email).first()
+ permission_exists = UserPortfolioPermission.objects.filter(user=requested_user, portfolio=self.object).exists()
+ if not requested_user or not permission_exists:
+ return self._make_invitation(requested_email, requestor)
+ else:
+ if permission_exists:
+ messages.warning(self.request, "User is already a member of this portfolio.")
+ return redirect(self.get_success_url())
diff --git a/src/registrar/views/utility/__init__.py b/src/registrar/views/utility/__init__.py
index fbf44fda1..6798eb4ee 100644
--- a/src/registrar/views/utility/__init__.py
+++ b/src/registrar/views/utility/__init__.py
@@ -9,6 +9,5 @@ from .permission_views import (
PortfolioMembersPermission,
DomainRequestPortfolioViewonlyView,
DomainInvitationPermissionCancelView,
- PortfolioInvitationCreatePermissionView,
)
from .api_views import get_senior_official_from_federal_agency_json
diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py
index 45474bddc..54ce5a921 100644
--- a/src/registrar/views/utility/permission_views.py
+++ b/src/registrar/views/utility/permission_views.py
@@ -1,7 +1,6 @@
"""View classes that enforce authorization."""
import abc # abstract base class
-from django.views.generic.edit import CreateView
from django.views.generic import DetailView, DeleteView, TemplateView, UpdateView
from registrar.models import Domain, DomainRequest, DomainInvitation, Portfolio
from registrar.models.portfolio_invitation import PortfolioInvitation
@@ -227,25 +226,6 @@ class PortfolioBasePermissionView(PortfolioBasePermission, DetailView, abc.ABC):
raise NotImplementedError
-class PortfolioInvitationCreatePermissionView(PortfolioInvitationCreatePermission, CreateView, abc.ABC):
- """Abstract base view for portfolio views that enforces permissions.
-
- This abstract view cannot be instantiated. Actual views must specify
- `template_name`.
- """
-
- # DetailView property for what model this is viewing
- model = PortfolioInvitation
- # variable name in template context for the model object
- context_object_name = "portfolio_invitation"
-
- # Abstract property enforces NotImplementedError on an attribute.
- @property
- @abc.abstractmethod
- def template_name(self):
- raise NotImplementedError
-
-
class PortfolioDomainsPermissionView(PortfolioDomainsPermission, PortfolioBasePermissionView, abc.ABC):
"""Abstract base view for portfolio domains views that enforces permissions.
From 08082fb0aac6628e3994f63718fd3b9e77e91ea5 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 11:51:03 -0700
Subject: [PATCH 23/32] cleanup pt 2
---
.../src/js/getgov/portfolio-member-page.js | 236 +++++++++---------
.../templates/portfolio_members_add_new.html | 1 +
src/registrar/tests/test_views_portfolio.py | 31 +--
src/registrar/views/utility/mixins.py | 18 --
.../views/utility/permission_views.py | 2 -
5 files changed, 135 insertions(+), 153 deletions(-)
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 af25f0f1d..280c087f0 100644
--- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js
+++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js
@@ -7,41 +7,41 @@ import { hookupRadioTogglerListener } from './radios.js';
// This is specifically for the Member Profile (Manage Member) Page member/invitation removal
export function initPortfolioNewMemberPageToggle() {
document.addEventListener("DOMContentLoaded", () => {
- const wrapperDeleteAction = document.getElementById("wrapper-delete-action")
- if (wrapperDeleteAction) {
- const member_type = wrapperDeleteAction.getAttribute("data-member-type");
- const member_id = wrapperDeleteAction.getAttribute("data-member-id");
- const num_domains = wrapperDeleteAction.getAttribute("data-num-domains");
- const member_name = wrapperDeleteAction.getAttribute("data-member-name");
- const member_email = wrapperDeleteAction.getAttribute("data-member-email");
- const member_delete_url = `${member_type}-${member_id}/delete`;
- const unique_id = `${member_type}-${member_id}`;
-
- let cancelInvitationButton = member_type === "invitedmember" ? "Cancel invitation" : "Remove member";
- wrapperDeleteAction.innerHTML = generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `for ${member_name}`);
-
- // This easter egg is only for fixtures that dont have names as we are displaying their emails
- // All prod users will have emails linked to their account
- MembersTable.addMemberModal(num_domains, member_email || "Samwise Gamgee", member_delete_url, unique_id, wrapperDeleteAction);
-
- uswdsInitializeModals();
-
- // Now the DOM and modals are ready, add listeners to the submit buttons
- const modals = document.querySelectorAll('.usa-modal__content');
-
- modals.forEach(modal => {
- const submitButton = modal.querySelector('.usa-modal__submit');
- const closeButton = modal.querySelector('.usa-modal__close');
- submitButton.addEventListener('click', () => {
- closeButton.click();
- let delete_member_form = document.getElementById("member-delete-form");
- if (delete_member_form) {
- delete_member_form.submit();
- }
- });
+ const wrapperDeleteAction = document.getElementById("wrapper-delete-action")
+ if (wrapperDeleteAction) {
+ const member_type = wrapperDeleteAction.getAttribute("data-member-type");
+ const member_id = wrapperDeleteAction.getAttribute("data-member-id");
+ const num_domains = wrapperDeleteAction.getAttribute("data-num-domains");
+ const member_name = wrapperDeleteAction.getAttribute("data-member-name");
+ const member_email = wrapperDeleteAction.getAttribute("data-member-email");
+ const member_delete_url = `${member_type}-${member_id}/delete`;
+ const unique_id = `${member_type}-${member_id}`;
+
+ let cancelInvitationButton = member_type === "invitedmember" ? "Cancel invitation" : "Remove member";
+ wrapperDeleteAction.innerHTML = generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `for ${member_name}`);
+
+ // This easter egg is only for fixtures that dont have names as we are displaying their emails
+ // All prod users will have emails linked to their account
+ MembersTable.addMemberModal(num_domains, member_email || "Samwise Gamgee", member_delete_url, unique_id, wrapperDeleteAction);
+
+ uswdsInitializeModals();
+
+ // Now the DOM and modals are ready, add listeners to the submit buttons
+ const modals = document.querySelectorAll('.usa-modal__content');
+
+ modals.forEach(modal => {
+ const submitButton = modal.querySelector('.usa-modal__submit');
+ const closeButton = modal.querySelector('.usa-modal__close');
+ submitButton.addEventListener('click', () => {
+ closeButton.click();
+ let delete_member_form = document.getElementById("member-delete-form");
+ if (delete_member_form) {
+ delete_member_form.submit();
+ }
});
- }
- });
+ });
+ }
+});
}
@@ -52,122 +52,122 @@ export function initPortfolioNewMemberPageToggle() {
export function initAddNewMemberPageListeners() {
let add_member_form = document.getElementById("add_member_form");
if (!add_member_form){
- return;
+ return;
}
document.getElementById("confirm_new_member_submit").addEventListener("click", function() {
- // Upon confirmation, submit the form
- document.getElementById("add_member_form").submit();
+// Upon confirmation, submit the form
+document.getElementById("add_member_form").submit();
});
document.getElementById("add_member_form").addEventListener("submit", function(event) {
- event.preventDefault(); // Prevents the form from submitting
- const form = document.getElementById("add_member_form")
- const formData = new FormData(form);
+event.preventDefault(); // Prevents the form from submitting
+const form = document.getElementById("add_member_form")
+const formData = new FormData(form);
- // Check if the form is valid
- // If the form is valid, open the confirmation modal
- // If the form is invalid, submit it to trigger error
- fetch(form.action, {
- method: "POST",
- body: formData,
- headers: {
- "X-Requested-With": "XMLHttpRequest",
- "X-CSRFToken": getCsrfToken()
- }
- })
- .then(response => response.json())
- .then(data => {
- if (data.is_valid) {
- // If the form is valid, show the confirmation modal before submitting
- openAddMemberConfirmationModal();
- } else {
- // If the form is not valid, trigger error messages by firing a submit event
- form.submit();
- }
- });
+// Check if the form is valid
+// If the form is valid, open the confirmation modal
+// If the form is invalid, submit it to trigger error
+fetch(form.action, {
+ method: "POST",
+ body: formData,
+ headers: {
+ "X-Requested-With": "XMLHttpRequest",
+ "X-CSRFToken": getCsrfToken()
+ }
+})
+.then(response => response.json())
+.then(data => {
+ if (data.is_valid) {
+ // If the form is valid, show the confirmation modal before submitting
+ openAddMemberConfirmationModal();
+ } else {
+ // If the form is not valid, trigger error messages by firing a submit event
+ form.submit();
+ }
+});
});
/*
- Helper function to capitalize the first letter in a string (for display purposes)
+Helper function to capitalize the first letter in a string (for display purposes)
*/
function capitalizeFirstLetter(text) {
- if (!text) return ''; // Return empty string if input is falsy
- return text.charAt(0).toUpperCase() + text.slice(1);
+if (!text) return ''; // Return empty string if input is falsy
+return text.charAt(0).toUpperCase() + text.slice(1);
}
/*
- Populates contents of the "Add Member" confirmation modal
+Populates contents of the "Add Member" confirmation modal
*/
function populatePermissionDetails(permission_details_div_id) {
- const permissionDetailsContainer = document.getElementById("permission_details");
- permissionDetailsContainer.innerHTML = ""; // Clear previous content
+const permissionDetailsContainer = document.getElementById("permission_details");
+permissionDetailsContainer.innerHTML = ""; // Clear previous content
- // Get all permission sections (divs with h3 and radio inputs)
- const permissionSections = document.querySelectorAll(`#${permission_details_div_id} > h3`);
+// Get all permission sections (divs with h3 and radio inputs)
+const permissionSections = document.querySelectorAll(`#${permission_details_div_id} > h3`);
- permissionSections.forEach(section => {
- // Find the
element text
- const sectionTitle = section.textContent;
+permissionSections.forEach(section => {
+ // Find the element text
+ const sectionTitle = section.textContent;
- // Find the associated radio buttons container (next fieldset)
- const fieldset = section.nextElementSibling;
+ // Find the associated radio buttons container (next fieldset)
+ const fieldset = section.nextElementSibling;
- if (fieldset && fieldset.tagName.toLowerCase() === 'fieldset') {
- // Get the selected radio button within this fieldset
- const selectedRadio = fieldset.querySelector('input[type="radio"]:checked');
+ if (fieldset && fieldset.tagName.toLowerCase() === 'fieldset') {
+ // Get the selected radio button within this fieldset
+ const selectedRadio = fieldset.querySelector('input[type="radio"]:checked');
- // If a radio button is selected, get its label text
- let selectedPermission = "No permission selected";
- if (selectedRadio) {
- const label = fieldset.querySelector(`label[for="${selectedRadio.id}"]`);
- selectedPermission = label ? label.textContent : "No permission selected";
- }
+ // If a radio button is selected, get its label text
+ let selectedPermission = "No permission selected";
+ if (selectedRadio) {
+ const label = fieldset.querySelector(`label[for="${selectedRadio.id}"]`);
+ selectedPermission = label ? label.textContent : "No permission selected";
+ }
- // Create new elements for the modal content
- const titleElement = document.createElement("h4");
- titleElement.textContent = sectionTitle;
- titleElement.classList.add("text-primary");
- titleElement.classList.add("margin-bottom-0");
+ // Create new elements for the modal content
+ const titleElement = document.createElement("h4");
+ titleElement.textContent = sectionTitle;
+ titleElement.classList.add("text-primary");
+ titleElement.classList.add("margin-bottom-0");
- const permissionElement = document.createElement("p");
- permissionElement.textContent = selectedPermission;
- permissionElement.classList.add("margin-top-0");
+ const permissionElement = document.createElement("p");
+ permissionElement.textContent = selectedPermission;
+ permissionElement.classList.add("margin-top-0");
- // Append to the modal content container
- permissionDetailsContainer.appendChild(titleElement);
- permissionDetailsContainer.appendChild(permissionElement);
- }
- });
+ // Append to the modal content container
+ permissionDetailsContainer.appendChild(titleElement);
+ permissionDetailsContainer.appendChild(permissionElement);
+ }
+});
}
/*
- Updates and opens the "Add Member" confirmation modal.
+Updates and opens the "Add Member" confirmation modal.
*/
function openAddMemberConfirmationModal() {
- //------- Populate modal details
- // Get email value
- let emailValue = document.getElementById('id_email').value;
- document.getElementById('modalEmail').textContent = emailValue;
+ //------- Populate modal details
+ // Get email value
+ let emailValue = document.getElementById('id_email').value;
+ document.getElementById('modalEmail').textContent = emailValue;
- // Get selected radio button for access level
- let selectedAccess = document.querySelector('input[name="member_access_level"]:checked');
- // Set the selected permission text to 'Basic' or 'Admin' (the value of the selected radio button)
- // This value does not have the first letter capitalized so let's capitalize it
- let accessText = selectedAccess ? capitalizeFirstLetter(selectedAccess.value) : "No access level selected";
- document.getElementById('modalAccessLevel').textContent = accessText;
+ // Get selected radio button for access level
+ let selectedAccess = document.querySelector('input[name="member_access_level"]:checked');
+ // Set the selected permission text to 'Basic' or 'Admin' (the value of the selected radio button)
+ // This value does not have the first letter capitalized so let's capitalize it
+ let accessText = selectedAccess ? capitalizeFirstLetter(selectedAccess.value) : "No access level selected";
+ document.getElementById('modalAccessLevel').textContent = accessText;
- // Populate permission details based on access level
- if (selectedAccess && selectedAccess.value === 'admin') {
- populatePermissionDetails('new-member-admin-permissions');
- } else {
- populatePermissionDetails('new-member-basic-permissions');
+ // Populate permission details based on access level
+ if (selectedAccess && selectedAccess.value === 'admin') {
+ populatePermissionDetails('new-member-admin-permissions');
+ } else {
+ populatePermissionDetails('new-member-basic-permissions');
+ }
+
+ //------- Show the modal
+ let modalTrigger = document.querySelector("#invite_member_trigger");
+ if (modalTrigger) {
+ modalTrigger.click();
}
-
- //------- Show the modal
- let modalTrigger = document.querySelector("#invite_member_trigger");
- if (modalTrigger) {
- modalTrigger.click();
- }
}
}
diff --git a/src/registrar/templates/portfolio_members_add_new.html b/src/registrar/templates/portfolio_members_add_new.html
index 466358915..655b01852 100644
--- a/src/registrar/templates/portfolio_members_add_new.html
+++ b/src/registrar/templates/portfolio_members_add_new.html
@@ -190,3 +190,4 @@
{% endblock portfolio_content%}
+
diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index f5f1a4401..01383ae77 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -2567,20 +2567,18 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
final_response = self.client.post(
reverse("new-member"),
{
- "role": "organization_member",
- "domain_request_permission_member": "view_all_requests",
+ "member_access_level": "basic",
+ "basic_org_domain_request_permissions": "view_only",
"email": self.new_member_email,
},
)
# Ensure the final submission is successful
self.assertEqual(final_response.status_code, 302) # redirects after success
+
# Validate Database Changes
portfolio_invite = PortfolioInvitation.objects.filter(
- email=self.new_member_email,
- portfolio=self.portfolio,
- roles__exact=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
- additional_permissions__exact=[UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS],
+ email=self.new_member_email, portfolio=self.portfolio
).first()
self.assertIsNotNone(portfolio_invite)
self.assertEqual(portfolio_invite.email, self.new_member_email)
@@ -2602,14 +2600,15 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
response = self.client.post(
reverse("new-member"),
{
- "role": "organization_member",
- "domain_request_permission_member": "view_all_requests",
+ "member_access_level": "basic",
+ "basic_org_domain_request_permissions": "view_only",
"email": self.invited_member_email,
},
)
- # Unsucessful form submissions return the same page with a 200
- self.assertEqual(response.status_code, 200)
- self.assertEqual(response.context["form"].errors["email"][0], "An invitation already exists for this user.")
+ self.assertEqual(response.status_code, 302) # Redirects
+
+ # TODO: verify messages
+
# Validate Database has not changed
invite_count_after = PortfolioInvitation.objects.count()
self.assertEqual(invite_count_after, invite_count_before)
@@ -2631,13 +2630,14 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
response = self.client.post(
reverse("new-member"),
{
- "role": "organization_member",
- "domain_request_permissions_member": "view_all_requests",
+ "member_access_level": "basic",
+ "basic_org_domain_request_permissions": "view_only",
"email": self.user.email,
},
)
- self.assertEqual(response.status_code, 200)
- self.assertEqual(response.context["form"].errors["email"][0], "User is already a member of this portfolio.")
+ self.assertEqual(response.status_code, 302) # Redirects
+
+ # TODO: verify messages
# Validate Database has not changed
invite_count_after = PortfolioInvitation.objects.count()
@@ -2645,6 +2645,7 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
class TestEditPortfolioMemberView(WebTest):
+ """Tests for the edit member page on portfolios"""
def setUp(self):
self.user = create_user()
diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py
index e62944c40..11384ca09 100644
--- a/src/registrar/views/utility/mixins.py
+++ b/src/registrar/views/utility/mixins.py
@@ -466,24 +466,6 @@ class PortfolioBasePermission(PermissionsLoginMixin):
return self.request.user.is_org_user(self.request)
-class PortfolioInvitationCreatePermission(PortfolioBasePermission):
- """Permission mixin that redirects to portfolio pages if user
- has access, otherwise 403"""
-
- def has_permission(self):
- """Check if this user has access to this portfolio.
-
- The user is in self.request.user and the portfolio can be looked
- up from the portfolio's primary key in self.kwargs["pk"]
- """
- has_perm = super().has_permission()
- if not has_perm:
- return False
-
- 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"""
diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py
index 54ce5a921..c49f2daa1 100644
--- a/src/registrar/views/utility/permission_views.py
+++ b/src/registrar/views/utility/permission_views.py
@@ -3,7 +3,6 @@
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.portfolio_invitation import PortfolioInvitation
from registrar.models.user import User
from registrar.models.user_domain_role import UserDomainRole
@@ -15,7 +14,6 @@ from .mixins import (
DomainRequestWizardPermission,
PortfolioDomainRequestsPermission,
PortfolioDomainsPermission,
- PortfolioInvitationCreatePermission,
PortfolioMemberDomainsPermission,
PortfolioMemberDomainsEditPermission,
PortfolioMemberEditPermission,
From 6965607f61d28aa400505efe231d4ecdc80c6c8a Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 11:55:17 -0700
Subject: [PATCH 24/32] Update portfolio-member-page.js
---
.../src/js/getgov/portfolio-member-page.js | 304 +++++++++---------
1 file changed, 152 insertions(+), 152 deletions(-)
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 280c087f0..83fee661c 100644
--- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js
+++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js
@@ -6,169 +6,169 @@ import { hookupRadioTogglerListener } from './radios.js';
// This is specifically for the Member Profile (Manage Member) Page member/invitation removal
export function initPortfolioNewMemberPageToggle() {
- document.addEventListener("DOMContentLoaded", () => {
- const wrapperDeleteAction = document.getElementById("wrapper-delete-action")
- if (wrapperDeleteAction) {
- const member_type = wrapperDeleteAction.getAttribute("data-member-type");
- const member_id = wrapperDeleteAction.getAttribute("data-member-id");
- const num_domains = wrapperDeleteAction.getAttribute("data-num-domains");
- const member_name = wrapperDeleteAction.getAttribute("data-member-name");
- const member_email = wrapperDeleteAction.getAttribute("data-member-email");
- const member_delete_url = `${member_type}-${member_id}/delete`;
- const unique_id = `${member_type}-${member_id}`;
-
- let cancelInvitationButton = member_type === "invitedmember" ? "Cancel invitation" : "Remove member";
- wrapperDeleteAction.innerHTML = generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `for ${member_name}`);
-
- // This easter egg is only for fixtures that dont have names as we are displaying their emails
- // All prod users will have emails linked to their account
- MembersTable.addMemberModal(num_domains, member_email || "Samwise Gamgee", member_delete_url, unique_id, wrapperDeleteAction);
-
- uswdsInitializeModals();
-
- // Now the DOM and modals are ready, add listeners to the submit buttons
- const modals = document.querySelectorAll('.usa-modal__content');
-
- modals.forEach(modal => {
- const submitButton = modal.querySelector('.usa-modal__submit');
- const closeButton = modal.querySelector('.usa-modal__close');
- submitButton.addEventListener('click', () => {
- closeButton.click();
- let delete_member_form = document.getElementById("member-delete-form");
- if (delete_member_form) {
- delete_member_form.submit();
- }
- });
- });
- }
-});
+ document.addEventListener("DOMContentLoaded", () => {
+ const wrapperDeleteAction = document.getElementById("wrapper-delete-action")
+ if (wrapperDeleteAction) {
+ const member_type = wrapperDeleteAction.getAttribute("data-member-type");
+ const member_id = wrapperDeleteAction.getAttribute("data-member-id");
+ const num_domains = wrapperDeleteAction.getAttribute("data-num-domains");
+ const member_name = wrapperDeleteAction.getAttribute("data-member-name");
+ const member_email = wrapperDeleteAction.getAttribute("data-member-email");
+ const member_delete_url = `${member_type}-${member_id}/delete`;
+ const unique_id = `${member_type}-${member_id}`;
+
+ let cancelInvitationButton = member_type === "invitedmember" ? "Cancel invitation" : "Remove member";
+ wrapperDeleteAction.innerHTML = generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `for ${member_name}`);
+
+ // This easter egg is only for fixtures that dont have names as we are displaying their emails
+ // All prod users will have emails linked to their account
+ MembersTable.addMemberModal(num_domains, member_email || "Samwise Gamgee", member_delete_url, unique_id, wrapperDeleteAction);
+
+ uswdsInitializeModals();
+
+ // Now the DOM and modals are ready, add listeners to the submit buttons
+ const modals = document.querySelectorAll('.usa-modal__content');
+
+ modals.forEach(modal => {
+ const submitButton = modal.querySelector('.usa-modal__submit');
+ const closeButton = modal.querySelector('.usa-modal__close');
+ submitButton.addEventListener('click', () => {
+ closeButton.click();
+ let delete_member_form = document.getElementById("member-delete-form");
+ if (delete_member_form) {
+ delete_member_form.submit();
+ }
+ });
+ });
+ }
+ });
}
/**
-* Hooks up specialized listeners for handling form validation and modals
-* on the Add New Member page.
-*/
+ * Hooks up specialized listeners for handling form validation and modals
+ * on the Add New Member page.
+ */
export function initAddNewMemberPageListeners() {
-let add_member_form = document.getElementById("add_member_form");
-if (!add_member_form){
- return;
-}
-document.getElementById("confirm_new_member_submit").addEventListener("click", function() {
-// Upon confirmation, submit the form
-document.getElementById("add_member_form").submit();
-});
+ let add_member_form = document.getElementById("add_member_form");
+ if (!add_member_form){
+ return;
+ }
+ document.getElementById("confirm_new_member_submit").addEventListener("click", function() {
+ // Upon confirmation, submit the form
+ document.getElementById("add_member_form").submit();
+ });
-document.getElementById("add_member_form").addEventListener("submit", function(event) {
-event.preventDefault(); // Prevents the form from submitting
-const form = document.getElementById("add_member_form")
-const formData = new FormData(form);
+ document.getElementById("add_member_form").addEventListener("submit", function(event) {
+ event.preventDefault(); // Prevents the form from submitting
+ const form = document.getElementById("add_member_form")
+ const formData = new FormData(form);
-// Check if the form is valid
-// If the form is valid, open the confirmation modal
-// If the form is invalid, submit it to trigger error
-fetch(form.action, {
- method: "POST",
- body: formData,
- headers: {
- "X-Requested-With": "XMLHttpRequest",
- "X-CSRFToken": getCsrfToken()
- }
-})
-.then(response => response.json())
-.then(data => {
- if (data.is_valid) {
- // If the form is valid, show the confirmation modal before submitting
- openAddMemberConfirmationModal();
- } else {
- // If the form is not valid, trigger error messages by firing a submit event
- form.submit();
- }
-});
-});
-
-/*
-Helper function to capitalize the first letter in a string (for display purposes)
-*/
-function capitalizeFirstLetter(text) {
-if (!text) return ''; // Return empty string if input is falsy
-return text.charAt(0).toUpperCase() + text.slice(1);
-}
-
-/*
-Populates contents of the "Add Member" confirmation modal
-*/
-function populatePermissionDetails(permission_details_div_id) {
-const permissionDetailsContainer = document.getElementById("permission_details");
-permissionDetailsContainer.innerHTML = ""; // Clear previous content
-
-// Get all permission sections (divs with h3 and radio inputs)
-const permissionSections = document.querySelectorAll(`#${permission_details_div_id} > h3`);
-
-permissionSections.forEach(section => {
- // Find the element text
- const sectionTitle = section.textContent;
-
- // Find the associated radio buttons container (next fieldset)
- const fieldset = section.nextElementSibling;
-
- if (fieldset && fieldset.tagName.toLowerCase() === 'fieldset') {
- // Get the selected radio button within this fieldset
- const selectedRadio = fieldset.querySelector('input[type="radio"]:checked');
-
- // If a radio button is selected, get its label text
- let selectedPermission = "No permission selected";
- if (selectedRadio) {
- const label = fieldset.querySelector(`label[for="${selectedRadio.id}"]`);
- selectedPermission = label ? label.textContent : "No permission selected";
+ // Check if the form is valid
+ // If the form is valid, open the confirmation modal
+ // If the form is invalid, submit it to trigger error
+ fetch(form.action, {
+ method: "POST",
+ body: formData,
+ headers: {
+ "X-Requested-With": "XMLHttpRequest",
+ "X-CSRFToken": getCsrfToken()
}
+ })
+ .then(response => response.json())
+ .then(data => {
+ if (data.is_valid) {
+ // If the form is valid, show the confirmation modal before submitting
+ openAddMemberConfirmationModal();
+ } else {
+ // If the form is not valid, trigger error messages by firing a submit event
+ form.submit();
+ }
+ });
+ });
- // Create new elements for the modal content
- const titleElement = document.createElement("h4");
- titleElement.textContent = sectionTitle;
- titleElement.classList.add("text-primary");
- titleElement.classList.add("margin-bottom-0");
-
- const permissionElement = document.createElement("p");
- permissionElement.textContent = selectedPermission;
- permissionElement.classList.add("margin-top-0");
-
- // Append to the modal content container
- permissionDetailsContainer.appendChild(titleElement);
- permissionDetailsContainer.appendChild(permissionElement);
- }
-});
-}
-
-/*
-Updates and opens the "Add Member" confirmation modal.
-*/
-function openAddMemberConfirmationModal() {
- //------- Populate modal details
- // Get email value
- let emailValue = document.getElementById('id_email').value;
- document.getElementById('modalEmail').textContent = emailValue;
-
- // Get selected radio button for access level
- let selectedAccess = document.querySelector('input[name="member_access_level"]:checked');
- // Set the selected permission text to 'Basic' or 'Admin' (the value of the selected radio button)
- // This value does not have the first letter capitalized so let's capitalize it
- let accessText = selectedAccess ? capitalizeFirstLetter(selectedAccess.value) : "No access level selected";
- document.getElementById('modalAccessLevel').textContent = accessText;
-
- // Populate permission details based on access level
- if (selectedAccess && selectedAccess.value === 'admin') {
- populatePermissionDetails('new-member-admin-permissions');
- } else {
- populatePermissionDetails('new-member-basic-permissions');
+ /*
+ Helper function to capitalize the first letter in a string (for display purposes)
+ */
+ function capitalizeFirstLetter(text) {
+ if (!text) return ''; // Return empty string if input is falsy
+ return text.charAt(0).toUpperCase() + text.slice(1);
}
- //------- Show the modal
- let modalTrigger = document.querySelector("#invite_member_trigger");
- if (modalTrigger) {
- modalTrigger.click();
- }
-}
+ /*
+ Populates contents of the "Add Member" confirmation modal
+ */
+ function populatePermissionDetails(permission_details_div_id) {
+ const permissionDetailsContainer = document.getElementById("permission_details");
+ permissionDetailsContainer.innerHTML = ""; // Clear previous content
+
+ // Get all permission sections (divs with h3 and radio inputs)
+ const permissionSections = document.querySelectorAll(`#${permission_details_div_id} > h3`);
+
+ permissionSections.forEach(section => {
+ // Find the element text
+ const sectionTitle = section.textContent;
+
+ // Find the associated radio buttons container (next fieldset)
+ const fieldset = section.nextElementSibling;
+
+ if (fieldset && fieldset.tagName.toLowerCase() === 'fieldset') {
+ // Get the selected radio button within this fieldset
+ const selectedRadio = fieldset.querySelector('input[type="radio"]:checked');
+
+ // If a radio button is selected, get its label text
+ let selectedPermission = "No permission selected";
+ if (selectedRadio) {
+ const label = fieldset.querySelector(`label[for="${selectedRadio.id}"]`);
+ selectedPermission = label ? label.textContent : "No permission selected";
+ }
+
+ // Create new elements for the modal content
+ const titleElement = document.createElement("h4");
+ titleElement.textContent = sectionTitle;
+ titleElement.classList.add("text-primary");
+ titleElement.classList.add("margin-bottom-0");
+
+ const permissionElement = document.createElement("p");
+ permissionElement.textContent = selectedPermission;
+ permissionElement.classList.add("margin-top-0");
+
+ // Append to the modal content container
+ permissionDetailsContainer.appendChild(titleElement);
+ permissionDetailsContainer.appendChild(permissionElement);
+ }
+ });
+ }
+
+ /*
+ Updates and opens the "Add Member" confirmation modal.
+ */
+ function openAddMemberConfirmationModal() {
+ //------- Populate modal details
+ // Get email value
+ let emailValue = document.getElementById('id_email').value;
+ document.getElementById('modalEmail').textContent = emailValue;
+
+ // Get selected radio button for access level
+ let selectedAccess = document.querySelector('input[name="member_access_level"]:checked');
+ // Set the selected permission text to 'Basic' or 'Admin' (the value of the selected radio button)
+ // This value does not have the first letter capitalized so let's capitalize it
+ let accessText = selectedAccess ? capitalizeFirstLetter(selectedAccess.value) : "No access level selected";
+ document.getElementById('modalAccessLevel').textContent = accessText;
+
+ // Populate permission details based on access level
+ if (selectedAccess && selectedAccess.value === 'admin') {
+ populatePermissionDetails('new-member-admin-permissions');
+ } else {
+ populatePermissionDetails('new-member-basic-permissions');
+ }
+
+ //------- Show the modal
+ let modalTrigger = document.querySelector("#invite_member_trigger");
+ if (modalTrigger) {
+ modalTrigger.click();
+ }
+ }
}
From adfd6be7bbba732cce481c42b0769cf79e2bbeac Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 11:59:33 -0700
Subject: [PATCH 25/32] Fix some merge things
---
src/registrar/views/portfolios.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py
index a238de3fc..855194f6b 100644
--- a/src/registrar/views/portfolios.py
+++ b/src/registrar/views/portfolios.py
@@ -1,15 +1,17 @@
import logging
+from django.conf import settings
+
from django.http import Http404, JsonResponse
from django.shortcuts import get_object_or_404, redirect, render
from django.urls import reverse
from django.utils.safestring import mark_safe
from django.contrib import messages
-from django.conf import settings
from registrar.forms import portfolio as portfolioForms
from registrar.models import Portfolio, User
from registrar.models.portfolio_invitation import PortfolioInvitation
from registrar.models.user_portfolio_permission import UserPortfolioPermission
from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
+from registrar.utility.email import EmailSendingError
from registrar.views.utility.mixins import PortfolioMemberPermission
from registrar.views.utility.permission_views import (
PortfolioDomainRequestsPermissionView,
@@ -505,7 +507,6 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View):
return render(request, "portfolio_members.html")
-
class NewMemberView(PortfolioMembersPermissionView, FormMixin):
template_name = "portfolio_members_add_new.html"
From c3480893dab229ce6a9f8e5ef65cfa052126c2d3 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 12:06:40 -0700
Subject: [PATCH 26/32] Readd modelforms
---
src/registrar/forms/portfolio.py | 276 +++++++++++++++++++------------
1 file changed, 166 insertions(+), 110 deletions(-)
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 34d334a3b..935c7c019 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -5,12 +5,14 @@ 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 (
- User,
+ PortfolioInvitation,
UserPortfolioPermission,
DomainInformation,
Portfolio,
SeniorOfficial,
+ User,
)
from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
@@ -109,6 +111,169 @@ class PortfolioSeniorOfficialForm(forms.ModelForm):
return cleaned_data
+class PortfolioMemberForm(forms.ModelForm):
+ """
+ Form for updating a portfolio member.
+ """
+
+ roles = forms.MultipleChoiceField(
+ choices=UserPortfolioRoleChoices.choices,
+ widget=forms.SelectMultiple(attrs={"class": "usa-select"}),
+ required=False,
+ label="Roles",
+ )
+
+ additional_permissions = forms.MultipleChoiceField(
+ choices=UserPortfolioPermissionChoices.choices,
+ widget=forms.SelectMultiple(attrs={"class": "usa-select"}),
+ required=False,
+ label="Additional Permissions",
+ )
+
+ class Meta:
+ model = UserPortfolioPermission
+ fields = [
+ "roles",
+ "additional_permissions",
+ ]
+
+
+class PortfolioInvitedMemberForm(forms.ModelForm):
+ """
+ Form for updating a portfolio invited member.
+ """
+
+ roles = forms.MultipleChoiceField(
+ choices=UserPortfolioRoleChoices.choices,
+ widget=forms.SelectMultiple(attrs={"class": "usa-select"}),
+ required=False,
+ label="Roles",
+ )
+
+ additional_permissions = forms.MultipleChoiceField(
+ choices=UserPortfolioPermissionChoices.choices,
+ widget=forms.SelectMultiple(attrs={"class": "usa-select"}),
+ required=False,
+ label="Additional Permissions",
+ )
+
+ class Meta:
+ model = PortfolioInvitation
+ fields = [
+ "roles",
+ "additional_permissions",
+ ]
+
+
+class NewMemberForm(forms.ModelForm):
+ member_access_level = forms.ChoiceField(
+ label="Select permission",
+ choices=[("admin", "Admin Access"), ("basic", "Basic Access")],
+ widget=forms.RadioSelect(attrs={"class": "usa-radio__input usa-radio__input--tile"}),
+ required=True,
+ error_messages={
+ "required": "Member access level is required",
+ },
+ )
+ 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")],
+ widget=forms.RadioSelect,
+ required=True,
+ error_messages={
+ "required": "Admin domain request permission is required",
+ },
+ )
+ admin_org_members_permissions = forms.ChoiceField(
+ label="Select permission",
+ choices=[("view_only", "View all members"), ("view_and_create", "View all members plus manage members")],
+ widget=forms.RadioSelect,
+ required=True,
+ error_messages={
+ "required": "Admin member permission is required",
+ },
+ )
+ basic_org_domain_request_permissions = forms.ChoiceField(
+ label="Select permission",
+ choices=[
+ ("view_only", "View all requests"),
+ ("view_and_create", "View all requests plus create requests"),
+ ("no_access", "No access"),
+ ],
+ widget=forms.RadioSelect,
+ required=True,
+ error_messages={
+ "required": "Basic member permission is required",
+ },
+ )
+
+ email = forms.EmailField(
+ label="Enter the email of the member you'd like to invite",
+ max_length=None,
+ error_messages={
+ "invalid": ("Enter an email address in the required format, like name@example.com."),
+ "required": ("Enter an email address in the required format, like name@example.com."),
+ },
+ validators=[
+ MaxLengthValidator(
+ 320,
+ message="Response must be less than 320 characters.",
+ )
+ ],
+ required=True,
+ )
+
+ class Meta:
+ model = User
+ fields = ["email"]
+
+ def clean(self):
+ cleaned_data = super().clean()
+
+ # Lowercase the value of the 'email' field
+ email_value = cleaned_data.get("email")
+ if email_value:
+ cleaned_data["email"] = email_value.lower()
+
+ ##########################################
+ # TODO: future ticket
+ # (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")
+
+ # 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
+ # for all hidden inputs (which is everything except the e-mail input
+ # and member access selection)
+ for field in self.fields:
+ if field in self.errors and field != "email" and field != "member_access_level":
+ del self.errors[field]
+ return cleaned_data
+
+ basic_dom_req_error = "basic_org_domain_request_permissions"
+ admin_dom_req_error = "admin_org_domain_request_permissions"
+ admin_member_error = "admin_org_members_permissions"
+
+ 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
+
+
class BasePortfolioMemberForm(forms.Form):
required_star = '*'
role = forms.ChoiceField(
@@ -332,112 +497,3 @@ class BasePortfolioMemberForm(forms.Form):
role_permissions = UserPortfolioPermission.get_portfolio_permissions(instance.roles, [], get_list=False)
instance.additional_permissions = list(additional_permissions - role_permissions)
return instance
-
-
-class NewMemberForm(forms.ModelForm):
- member_access_level = forms.ChoiceField(
- label="Select permission",
- choices=[("admin", "Admin Access"), ("basic", "Basic Access")],
- widget=forms.RadioSelect(attrs={"class": "usa-radio__input usa-radio__input--tile"}),
- required=True,
- error_messages={
- "required": "Member access level is required",
- },
- )
- 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")],
- widget=forms.RadioSelect,
- required=True,
- error_messages={
- "required": "Admin domain request permission is required",
- },
- )
- admin_org_members_permissions = forms.ChoiceField(
- label="Select permission",
- choices=[("view_only", "View all members"), ("view_and_create", "View all members plus manage members")],
- widget=forms.RadioSelect,
- required=True,
- error_messages={
- "required": "Admin member permission is required",
- },
- )
- basic_org_domain_request_permissions = forms.ChoiceField(
- label="Select permission",
- choices=[
- ("view_only", "View all requests"),
- ("view_and_create", "View all requests plus create requests"),
- ("no_access", "No access"),
- ],
- widget=forms.RadioSelect,
- required=True,
- error_messages={
- "required": "Basic member permission is required",
- },
- )
-
- email = forms.EmailField(
- label="Enter the email of the member you'd like to invite",
- max_length=None,
- error_messages={
- "invalid": ("Enter an email address in the required format, like name@example.com."),
- "required": ("Enter an email address in the required format, like name@example.com."),
- },
- validators=[
- MaxLengthValidator(
- 320,
- message="Response must be less than 320 characters.",
- )
- ],
- required=True,
- )
-
- class Meta:
- model = User
- fields = ["email"]
-
- def clean(self):
- cleaned_data = super().clean()
-
- # Lowercase the value of the 'email' field
- email_value = cleaned_data.get("email")
- if email_value:
- cleaned_data["email"] = email_value.lower()
-
- ##########################################
- # TODO: future ticket
- # (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")
-
- # 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
- # for all hidden inputs (which is everything except the e-mail input
- # and member access selection)
- for field in self.fields:
- if field in self.errors and field != "email" and field != "member_access_level":
- del self.errors[field]
- return cleaned_data
-
- basic_dom_req_error = "basic_org_domain_request_permissions"
- admin_dom_req_error = "admin_org_domain_request_permissions"
- admin_member_error = "admin_org_members_permissions"
-
- 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
From a9923f4cc951022ac5f4068bf6d9ae106c7f9fd3 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 12:09:01 -0700
Subject: [PATCH 27/32] Update portfolio.py
---
src/registrar/forms/portfolio.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 935c7c019..5e3a7b324 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -273,7 +273,6 @@ class NewMemberForm(forms.ModelForm):
del self.errors[admin_member_error]
return cleaned_data
-
class BasePortfolioMemberForm(forms.Form):
required_star = '*'
role = forms.ChoiceField(
From f19ff3cd66e5ea4d9b1ac671c2c69d3c689929c7 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 12:25:21 -0700
Subject: [PATCH 28/32] PR suggestions
---
src/registrar/admin.py | 4 --
src/registrar/forms/__init__.py | 1 +
src/registrar/forms/portfolio.py | 43 ++++++-------------
.../models/user_portfolio_permission.py | 7 ++-
.../views/utility/permission_views.py | 1 +
5 files changed, 22 insertions(+), 34 deletions(-)
diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 72eff6d79..4465b7098 100644
--- a/src/registrar/admin.py
+++ b/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)
diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py
index 033e955ed..121e2b3f7 100644
--- a/src/registrar/forms/__init__.py
+++ b/src/registrar/forms/__init__.py
@@ -13,4 +13,5 @@ from .domain import (
)
from .portfolio import (
PortfolioOrgAddressForm,
+ PortfolioMemberForm,
)
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 5e3a7b324..ecd21b8ee 100644
--- a/src/registrar/forms/portfolio.py
+++ b/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 = '*'
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.
-
- 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
- """
+ """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.
+ # 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
@@ -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
diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py
index f312f3dd0..25abb6748 100644
--- a/src/registrar/models/user_portfolio_permission.py
+++ b/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:
diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py
index c49f2daa1..a3067d3a2 100644
--- a/src/registrar/views/utility/permission_views.py
+++ b/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
From 0e1d07d59bcadbe81d1c60b527a92f3482ef1718 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 12:32:47 -0700
Subject: [PATCH 29/32] Update portfolio.py
---
src/registrar/forms/portfolio.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index ecd21b8ee..6e1e7d43c 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -331,7 +331,9 @@ class BasePortfolioMemberForm(forms.Form):
},
)
- # Tracks what form elements are required for a given role choice
+ # 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",
From a314649de2e42c209b02e7565ab19f3736443545 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 12:33:34 -0700
Subject: [PATCH 30/32] Update portfolio.py
---
src/registrar/forms/portfolio.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 6e1e7d43c..ddfa93bc1 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -387,7 +387,7 @@ class BasePortfolioMemberForm(forms.Form):
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.
+ # map_instance_to_form => 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.
From dec9e3362dc2534b6953b122a46bc599585ef87e Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 13:02:27 -0700
Subject: [PATCH 31/32] Condense logic and change names
---
src/registrar/forms/portfolio.py | 89 +++++++++++++++-----------------
1 file changed, 42 insertions(+), 47 deletions(-)
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index ddfa93bc1..ce164607e 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -281,6 +281,7 @@ class BasePortfolioMemberForm(forms.Form):
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"),
],
@@ -345,10 +346,12 @@ class BasePortfolioMemberForm(forms.Form):
}
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_form(self.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(
@@ -359,6 +362,14 @@ class BasePortfolioMemberForm(forms.Form):
),
}
+ 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()
@@ -380,23 +391,17 @@ class BasePortfolioMemberForm(forms.Form):
return cleaned_data
- def save(self):
- """Save the form data to the instance"""
- self.instance = self.map_cleaned_data_to_instance(self.cleaned_data, self.instance)
- 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.initial.
+ # 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_form(self, instance):
+ def map_instance_to_initial(self, instance):
"""
- Maps user instance to form fields, handling roles and permissions.
+ 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",
@@ -405,57 +410,47 @@ class BasePortfolioMemberForm(forms.Form):
"domain_request_permission_member": permission level if member
}
"""
- if not instance:
- return {}
-
# Function variables
form_data = {}
perms = UserPortfolioPermission.get_portfolio_permissions(
instance.roles, instance.additional_permissions, get_list=False
)
- # Explanation of this logic pattern: we can only display one item in the list at a time.
- # But how do we determine what is most important to display in a list? Order-based hierarchy.
- # Example: print(instance.roles) => (output) ["organization_admin", "organization_member"]
- # If we can only pick one item in this list, we should pick organization_admin.
-
- # Get role
- roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN, UserPortfolioRoleChoices.ORGANIZATION_MEMBER]
- role = next((role for role in roles if role in instance.roles), None)
- is_admin = role == UserPortfolioRoleChoices.ORGANIZATION_ADMIN
-
- # Get domain request permission level
- # First we get permissions we expect to display (ordered hierarchically).
- # Then we check if this item exists in the list and return the first instance of it.
- domain_permissions = [
+ # 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,
]
- domain_request_permission = next((perm for perm in domain_permissions if perm in perms), None)
+ member_perms = [
+ UserPortfolioPermissionChoices.EDIT_MEMBERS,
+ UserPortfolioPermissionChoices.VIEW_MEMBERS,
+ ]
- # Get member permission level.
- member_permissions = [UserPortfolioPermissionChoices.EDIT_MEMBERS, UserPortfolioPermissionChoices.VIEW_MEMBERS]
- member_permission = next((perm for perm in member_permissions if perm in perms), None)
-
- # Build form data based on role.
- form_data = {
- "role": role,
- "member_permission_admin": getattr(member_permission, "value", None) if is_admin else None,
- "domain_request_permission_admin": getattr(domain_request_permission, "value", None) if is_admin else None,
- "domain_request_permission_member": (
- getattr(domain_request_permission, "value", None) if not is_admin else None
- ),
- }
-
- # Edgecase: Member uses a special form value for None called "no_access". This ensures a form selection.
- if domain_request_permission is None and not is_admin:
- form_data["domain_request_permission_member"] = "no_access"
+ # 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.
+ selected_role = next((role for role in roles if role in instance.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 cleaned data to a member instance, setting roles and permissions.
+ 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
From 297be33e645bbf454fe43084099db661912c3286 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 18 Dec 2024 14:42:44 -0700
Subject: [PATCH 32/32] remove log
---
src/registrar/assets/src/js/getgov/portfolio-member-page.js | 1 -
src/registrar/forms/portfolio.py | 3 ++-
2 files changed, 2 insertions(+), 2 deletions(-)
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 83fee661c..02d927438 100644
--- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js
+++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js
@@ -175,7 +175,6 @@ export function initAddNewMemberPageListeners() {
// Initalize the radio for the member pages
export function initPortfolioMemberPageRadio() {
document.addEventListener("DOMContentLoaded", () => {
- console.log("new content 2")
let memberForm = document.getElementById("member_form");
let newMemberForm = document.getElementById("add_member_form")
if (memberForm) {
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index ce164607e..eaa885a85 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -433,7 +433,8 @@ class BasePortfolioMemberForm(forms.Form):
# 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.
- selected_role = next((role for role in roles if role in instance.roles), None)
+ 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: