diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 52e214bb9..88c064379 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -21,10 +21,14 @@ from registrar.utility.admin_helpers import ( get_field_links_as_list, ) from django.conf import settings +from django.contrib.messages import get_messages +from django.contrib.admin.helpers import AdminForm from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField from registrar.models import DomainInformation, Portfolio, UserPortfolioPermission, DomainInvitation from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from registrar.utility.email import EmailSendingError +from registrar.utility.email_invitations import send_portfolio_invitation_email from waffle.decorators import flag_is_active from django.contrib import admin, messages from django.contrib.auth.admin import UserAdmin as BaseUserAdmin @@ -37,7 +41,7 @@ from waffle.admin import FlagAdmin from waffle.models import Sample, Switch from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.constants import BranchChoices -from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes +from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes, MissingEmailError from registrar.utility.waffle import flag_is_active_for_user from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR @@ -1312,6 +1316,8 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): search_fields = ["user__first_name", "user__last_name", "user__email", "portfolio__organization_name"] search_help_text = "Search by first name, last name, email, or portfolio." + change_form_template = "django/admin/user_portfolio_permission_change_form.html" + def get_roles(self, obj): readable_roles = obj.get_readable_roles() return ", ".join(readable_roles) @@ -1468,7 +1474,7 @@ class PortfolioInvitationAdmin(ListHeaderAdmin): autocomplete_fields = ["portfolio"] - change_form_template = "django/admin/email_clipboard_change_form.html" + change_form_template = "django/admin/portfolio_invitation_change_form.html" # Select portfolio invitations to change -> Portfolio invitations def changelist_view(self, request, extra_context=None): @@ -1478,6 +1484,118 @@ class PortfolioInvitationAdmin(ListHeaderAdmin): # Get the filtered values return super().changelist_view(request, extra_context=extra_context) + def save_model(self, request, obj, form, change): + """ + Override the save_model method. + + Only send email on creation of the PortfolioInvitation object. Not on updates. + Emails sent to requested user / email. + When exceptions are raised, return without saving model. + """ + if not change: # Only send email if this is a new PortfolioInvitation (creation) + portfolio = obj.portfolio + requested_email = obj.email + requestor = request.user + + permission_exists = UserPortfolioPermission.objects.filter( + user__email=requested_email, portfolio=portfolio, user__email__isnull=False + ).exists() + try: + if not permission_exists: + # if permission does not exist for a user with requested_email, send email + send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=portfolio) + messages.success(request, f"{requested_email} has been invited.") + else: + messages.warning(request, "User is already a member of this portfolio.") + except Exception as e: + # when exception is raised, handle and do not save the model + self._handle_exceptions(e, request, obj) + return + # Call the parent save method to save the object + super().save_model(request, obj, form, change) + + def _handle_exceptions(self, exception, request, obj): + """Handle exceptions raised during the process. + + Log warnings / errors, and message errors to the user. + """ + if isinstance(exception, EmailSendingError): + logger.warning( + "Could not sent email invitation to %s for portfolio %s (EmailSendingError)", + obj.email, + obj.portfolio, + exc_info=True, + ) + messages.error(request, "Could not send email invitation. Portfolio invitation not saved.") + elif isinstance(exception, MissingEmailError): + messages.error(request, str(exception)) + logger.error( + f"Can't send email to '{obj.email}' for portfolio '{obj.portfolio}'. " + f"No email exists for the requestor.", + exc_info=True, + ) + + else: + logger.warning("Could not send email invitation (Other Exception)", exc_info=True) + messages.error(request, "Could not send email invitation. Portfolio invitation not saved.") + + def response_add(self, request, obj, post_url_continue=None): + """ + Override response_add to handle rendering when exceptions are raised during add model. + + Normal flow on successful save_model on add is to redirect to changelist_view. + If there are errors, flow is modified to instead render change form. + """ + # Check if there are any error or warning messages in the `messages` framework + storage = get_messages(request) + has_errors = any(message.level_tag in ["error", "warning"] for message in storage) + + if has_errors: + # Re-render the change form if there are errors or warnings + # Prepare context for rendering the change form + + # Get the model form + ModelForm = self.get_form(request, obj=obj) + form = ModelForm(instance=obj) + + # Create an AdminForm instance + admin_form = AdminForm( + form, + list(self.get_fieldsets(request, obj)), + self.get_prepopulated_fields(request, obj), + self.get_readonly_fields(request, obj), + model_admin=self, + ) + media = self.media + form.media + + opts = obj._meta + change_form_context = { + **self.admin_site.each_context(request), # Add admin context + "title": f"Add {opts.verbose_name}", + "opts": opts, + "original": obj, + "save_as": self.save_as, + "has_change_permission": self.has_change_permission(request, obj), + "add": True, # Indicate this is an "Add" form + "change": False, # Indicate this is not a "Change" form + "is_popup": False, + "inline_admin_formsets": [], + "save_on_top": self.save_on_top, + "show_delete": self.has_delete_permission(request, obj), + "obj": obj, + "adminform": admin_form, # Pass the AdminForm instance + "media": media, + "errors": None, + } + return self.render_change_form( + request, + context=change_form_context, + add=True, + change=False, + obj=obj, + ) + return super().response_add(request, obj, post_url_continue) + class DomainInformationResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the diff --git a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js index a815a59a1..b3d14839e 100644 --- a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js +++ b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js @@ -629,6 +629,51 @@ export function initRejectedEmail() { }); } + +/** + * A function that handles the suborganzation and requested suborganization fields and buttons. + * - Fieldwise: Hooks to the sub_organization, suborganization_city, and suborganization_state_territory fields. + * On change, this function checks if any of these fields are not empty: + * sub_organization, suborganization_city, and suborganization_state_territory. + * If they aren't, then we show the "clear" button. If they are, then we hide it because we don't need it. + * + * - Buttonwise: Hooks to the #clear-requested-suborganization button. + * On click, this will clear the input value of sub_organization, suborganization_city, and suborganization_state_territory. +*/ +function handleSuborgFieldsAndButtons() { + const requestedSuborganizationField = document.getElementById("id_requested_suborganization"); + const suborganizationCity = document.getElementById("id_suborganization_city"); + const suborganizationStateTerritory = document.getElementById("id_suborganization_state_territory"); + const rejectButton = document.querySelector("#clear-requested-suborganization"); + + // Ensure that every variable is present before proceeding + if (!requestedSuborganizationField || !suborganizationCity || !suborganizationStateTerritory || !rejectButton) { + console.warn("handleSuborganizationSelection() => Could not find required fields.") + return; + } + + function handleRejectButtonVisibility() { + if (requestedSuborganizationField.value || suborganizationCity.value || suborganizationStateTerritory.value) { + showElement(rejectButton); + }else { + hideElement(rejectButton) + } + } + + function handleRejectButton() { + // Clear the text fields + requestedSuborganizationField.value = ""; + suborganizationCity.value = ""; + suborganizationStateTerritory.value = ""; + // Update button visibility after clearing + handleRejectButtonVisibility(); + } + rejectButton.addEventListener("click", handleRejectButton) + requestedSuborganizationField.addEventListener("blur", handleRejectButtonVisibility); + suborganizationCity.addEventListener("blur", handleRejectButtonVisibility); + suborganizationStateTerritory.addEventListener("change", handleRejectButtonVisibility); +} + /** * A function for dynamic DomainRequest fields */ @@ -636,5 +681,6 @@ export function initDynamicDomainRequestFields(){ const domainRequestPage = document.getElementById("domainrequest_form"); if (domainRequestPage) { handlePortfolioSelection(); + handleSuborgFieldsAndButtons(); } } diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js index 0e5946c23..9a60e1684 100644 --- a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js +++ b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js @@ -49,6 +49,13 @@ export function handlePortfolioSelection( const portfolioUrbanizationField = document.querySelector(".field-portfolio_urbanization"); const portfolioUrbanization = portfolioUrbanizationField.querySelector(".readonly"); const portfolioJsonUrl = document.getElementById("portfolio_json_url")?.value || null; + // These requested suborganization fields only exist on the domain request page + const rejectSuborganizationButton = document.querySelector("#clear-requested-suborganization"); + const requestedSuborganizationFieldInput = document.getElementById("id_requested_suborganization"); + const suborganizationCityInput = document.getElementById("id_suborganization_city"); + const suborganizationStateTerritoryInput = document.getElementById("id_suborganization_state_territory"); + + // Global var to track page load let isPageLoading = true; /** @@ -469,11 +476,28 @@ export function handlePortfolioSelection( if (requestedSuborganizationField) showElement(requestedSuborganizationField); if (suborganizationCity) showElement(suborganizationCity); if (suborganizationStateTerritory) showElement(suborganizationStateTerritory); + + // == LOGIC FOR THE DOMAIN REQUEST PAGE == // + // Handle rejectSuborganizationButton (display of the clear requested suborg button). + // Basically, this button should only be visible when we have data for suborg, city, and state_territory. + // The function handleSuborgFieldsAndButtons() in domain-request-form.js handles doing this same logic + // but on field input for city, state_territory, and the suborg field. + // If it doesn't exist, don't do anything. + if (rejectSuborganizationButton){ + if (requestedSuborganizationFieldInput?.value || suborganizationCityInput?.value || suborganizationStateTerritoryInput?.value) { + showElement(rejectSuborganizationButton); + }else { + hideElement(rejectSuborganizationButton); + } + } } else { // Hide suborganization request fields if suborganization is selected if (requestedSuborganizationField) hideElement(requestedSuborganizationField); if (suborganizationCity) hideElement(suborganizationCity); - if (suborganizationStateTerritory) hideElement(suborganizationStateTerritory); + if (suborganizationStateTerritory) hideElement(suborganizationStateTerritory); + + // == LOGIC FOR THE DOMAIN REQUEST PAGE == // + if (rejectSuborganizationButton) hideElement(rejectSuborganizationButton); } } diff --git a/src/registrar/assets/src/js/getgov/helpers.js b/src/registrar/assets/src/js/getgov/helpers.js index 93c25ee45..7d1449bac 100644 --- a/src/registrar/assets/src/js/getgov/helpers.js +++ b/src/registrar/assets/src/js/getgov/helpers.js @@ -1,9 +1,17 @@ export function hideElement(element) { - element.classList.add('display-none'); + if (element) { + element.classList.add('display-none'); + } else { + throw new Error('hideElement expected a passed DOM element as an argument, but none was provided.'); + } }; export function showElement(element) { - element.classList.remove('display-none'); + if (element) { + element.classList.remove('display-none'); + } else { + throw new Error('showElement expected a passed DOM element as an argument, but none was provided.'); + } }; /** diff --git a/src/registrar/assets/src/js/getgov/portfolio-member-page.js b/src/registrar/assets/src/js/getgov/portfolio-member-page.js index 02d927438..cfb83badc 100644 --- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js +++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js @@ -150,14 +150,14 @@ export function initAddNewMemberPageListeners() { document.getElementById('modalEmail').textContent = emailValue; // Get selected radio button for access level - let selectedAccess = document.querySelector('input[name="member_access_level"]:checked'); + let selectedAccess = document.querySelector('input[name="role"]: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') { + if (selectedAccess && selectedAccess.value === 'organization_admin') { populatePermissionDetails('new-member-admin-permissions'); } else { populatePermissionDetails('new-member-basic-permissions'); @@ -187,10 +187,10 @@ export function initPortfolioMemberPageRadio() { ); }else if (newMemberForm){ hookupRadioTogglerListener( - 'member_access_level', + 'role', { - 'admin': 'new-member-admin-permissions', - 'basic': 'new-member-basic-permissions' + 'organization_admin': 'new-member-admin-permissions', + 'organization_member': 'new-member-basic-permissions' } ); } diff --git a/src/registrar/assets/src/js/getgov/table-base.js b/src/registrar/assets/src/js/getgov/table-base.js index 97c256734..e1d5c11ce 100644 --- a/src/registrar/assets/src/js/getgov/table-base.js +++ b/src/registrar/assets/src/js/getgov/table-base.js @@ -143,7 +143,7 @@ export class BaseTable { this.statusCheckboxes = document.querySelectorAll(`.${this.sectionSelector} input[name="filter-status"]`); this.statusIndicator = document.getElementById(`${this.sectionSelector}__filter-indicator`); this.statusToggle = document.getElementById(`${this.sectionSelector}__usa-button--filter`); - this.noTableWrapper = document.getElementById(`${this.sectionSelector}__no-data`); + this.noDataTableWrapper = document.getElementById(`${this.sectionSelector}__no-data`); this.noSearchResultsWrapper = document.getElementById(`${this.sectionSelector}__no-search-results`); this.portfolioElement = document.getElementById('portfolio-js-value'); this.portfolioValue = this.portfolioElement ? this.portfolioElement.getAttribute('data-portfolio') : null; @@ -451,7 +451,7 @@ export class BaseTable { } // handle the display of proper messaging in the event that no members exist in the list or search returns no results - this.updateDisplay(data, this.tableWrapper, this.noTableWrapper, this.noSearchResultsWrapper, this.currentSearchTerm); + this.updateDisplay(data, this.tableWrapper, this.noDataTableWrapper, this.noSearchResultsWrapper, this.currentSearchTerm); // identify the DOM element where the list of results will be inserted into the DOM const tbody = this.tableWrapper.querySelector('tbody'); tbody.innerHTML = ''; diff --git a/src/registrar/assets/src/js/getgov/table-edit-member-domains.js b/src/registrar/assets/src/js/getgov/table-edit-member-domains.js index 95492d46f..86aa39c37 100644 --- a/src/registrar/assets/src/js/getgov/table-edit-member-domains.js +++ b/src/registrar/assets/src/js/getgov/table-edit-member-domains.js @@ -1,5 +1,6 @@ import { BaseTable } from './table-base.js'; +import { hideElement, showElement } from './helpers.js'; /** * EditMemberDomainsTable is used for PortfolioMember and PortfolioInvitedMember @@ -18,8 +19,14 @@ export class EditMemberDomainsTable extends BaseTable { this.initialDomainAssignmentsOnlyMember = []; // list of initially assigned domains which are readonly this.addedDomains = []; // list of domains added to member this.removedDomains = []; // list of domains removed from member + this.editModeContainer = document.getElementById('domain-assignments-edit-view'); + this.readonlyModeContainer = document.getElementById('domain-assignments-readonly-view'); + this.reviewButton = document.getElementById('review-domain-assignments'); + this.backButton = document.getElementById('back-to-edit-domain-assignments'); + this.saveButton = document.getElementById('save-domain-assignments'); this.initializeDomainAssignments(); this.initCancelEditDomainAssignmentButton(); + this.initEventListeners(); } getBaseUrl() { return document.getElementById("get_member_domains_json_url"); @@ -55,6 +62,14 @@ export class EditMemberDomainsTable extends BaseTable { getSearchParams(page, sortBy, order, searchTerm, status, portfolio) { let searchParams = super.getSearchParams(page, sortBy, order, searchTerm, status, portfolio); // Add checkedDomains to searchParams + let checkedDomains = this.getCheckedDomains(); + // Append updated checkedDomain IDs to searchParams + if (checkedDomains.length > 0) { + searchParams.append("checkedDomainIds", checkedDomains.join(",")); + } + return searchParams; + } + getCheckedDomains() { // Clone the initial domains to avoid mutating them let checkedDomains = [...this.initialDomainAssignments]; // Add IDs from addedDomains that are not already in checkedDomains @@ -70,11 +85,7 @@ export class EditMemberDomainsTable extends BaseTable { checkedDomains.splice(index, 1); } }); - // Append updated checkedDomain IDs to searchParams - if (checkedDomains.length > 0) { - searchParams.append("checkedDomainIds", checkedDomains.join(",")); - } - return searchParams; + return checkedDomains } addRow(dataObject, tbody, customTableOptions) { const domain = dataObject; @@ -217,7 +228,123 @@ export class EditMemberDomainsTable extends BaseTable { } }); } + + updateReadonlyDisplay() { + let totalAssignedDomains = this.getCheckedDomains().length; + + // Create unassigned domains list + const unassignedDomainsList = document.createElement('ul'); + unassignedDomainsList.classList.add('usa-list', 'usa-list--unstyled'); + this.removedDomains.forEach(removedDomain => { + const removedDomainListItem = document.createElement('li'); + removedDomainListItem.textContent = removedDomain.name; // Use textContent for security + unassignedDomainsList.appendChild(removedDomainListItem); + }); + + // Create assigned domains list + const assignedDomainsList = document.createElement('ul'); + assignedDomainsList.classList.add('usa-list', 'usa-list--unstyled'); + this.addedDomains.forEach(addedDomain => { + const addedDomainListItem = document.createElement('li'); + addedDomainListItem.textContent = addedDomain.name; // Use textContent for security + assignedDomainsList.appendChild(addedDomainListItem); + }); + + // Get the summary container + const domainAssignmentSummary = document.getElementById('domain-assignments-summary'); + // Clear existing content + domainAssignmentSummary.innerHTML = ''; + + // Append unassigned domains section + if (this.removedDomains.length) { + const unassignedHeader = document.createElement('h3'); + unassignedHeader.classList.add('header--body', 'text-primary', 'margin-bottom-1'); + unassignedHeader.textContent = 'Unassigned domains'; + domainAssignmentSummary.appendChild(unassignedHeader); + domainAssignmentSummary.appendChild(unassignedDomainsList); + } + + // Append assigned domains section + if (this.addedDomains.length) { + const assignedHeader = document.createElement('h3'); + assignedHeader.classList.add('header--body', 'text-primary', 'margin-bottom-1'); + assignedHeader.textContent = 'Assigned domains'; + domainAssignmentSummary.appendChild(assignedHeader); + domainAssignmentSummary.appendChild(assignedDomainsList); + } + + // Append total assigned domains section + const totalHeader = document.createElement('h3'); + totalHeader.classList.add('header--body', 'text-primary', 'margin-bottom-1'); + totalHeader.textContent = 'Total assigned domains'; + domainAssignmentSummary.appendChild(totalHeader); + const totalCount = document.createElement('p'); + totalCount.classList.add('margin-y-0'); + totalCount.textContent = totalAssignedDomains; + domainAssignmentSummary.appendChild(totalCount); + } + + showReadonlyMode() { + this.updateReadonlyDisplay(); + hideElement(this.editModeContainer); + showElement(this.readonlyModeContainer); + } + + showEditMode() { + hideElement(this.readonlyModeContainer); + showElement(this.editModeContainer); + } + + submitChanges() { + let memberDomainsEditForm = document.getElementById("member-domains-edit-form"); + if (memberDomainsEditForm) { + // Serialize data to send + const addedDomainIds = this.addedDomains.map(domain => domain.id); + const addedDomainsInput = document.createElement('input'); + addedDomainsInput.type = 'hidden'; + addedDomainsInput.name = 'added_domains'; // Backend will use this key to retrieve data + addedDomainsInput.value = JSON.stringify(addedDomainIds); // Stringify the array + + const removedDomainsIds = this.removedDomains.map(domain => domain.id); + const removedDomainsInput = document.createElement('input'); + removedDomainsInput.type = 'hidden'; + removedDomainsInput.name = 'removed_domains'; // Backend will use this key to retrieve data + removedDomainsInput.value = JSON.stringify(removedDomainsIds); // Stringify the array + + // Append input to the form + memberDomainsEditForm.appendChild(addedDomainsInput); + memberDomainsEditForm.appendChild(removedDomainsInput); + + memberDomainsEditForm.submit(); + } + } + + initEventListeners() { + if (this.reviewButton) { + this.reviewButton.addEventListener('click', () => { + this.showReadonlyMode(); + }); + } else { + console.warn('Missing DOM element. Expected element with id review-domain-assignments'); + } + + if (this.backButton) { + this.backButton.addEventListener('click', () => { + this.showEditMode(); + }); + } else { + console.warn('Missing DOM element. Expected element with id back-to-edit-domain-assignments'); + } + + if (this.saveButton) { + this.saveButton.addEventListener('click', () => { + this.submitChanges(); + }); + } else { + console.warn('Missing DOM element. Expected element with id save-domain-assignments'); + } + } } export function initEditMemberDomainsTable() { diff --git a/src/registrar/assets/src/js/getgov/table-member-domains.js b/src/registrar/assets/src/js/getgov/table-member-domains.js index 54e9d1212..7f89eee52 100644 --- a/src/registrar/assets/src/js/getgov/table-member-domains.js +++ b/src/registrar/assets/src/js/getgov/table-member-domains.js @@ -1,4 +1,5 @@ +import { showElement, hideElement } from './helpers.js'; import { BaseTable } from './table-base.js'; export class MemberDomainsTable extends BaseTable { @@ -24,7 +25,28 @@ export class MemberDomainsTable extends BaseTable { `; tbody.appendChild(row); } - + updateDisplay = (data, dataWrapper, noDataWrapper, noSearchResultsWrapper) => { + const { unfiltered_total, total } = data; + const searchSection = document.getElementById('edit-member-domains__search'); + if (!searchSection) console.warn('MemberDomainsTable updateDisplay expected an element with id edit-member-domains__search but none was found'); + if (unfiltered_total) { + showElement(searchSection); + if (total) { + showElement(dataWrapper); + hideElement(noSearchResultsWrapper); + hideElement(noDataWrapper); + } else { + hideElement(dataWrapper); + showElement(noSearchResultsWrapper); + hideElement(noDataWrapper); + } + } else { + hideElement(searchSection); + hideElement(dataWrapper); + hideElement(noSearchResultsWrapper); + showElement(noDataWrapper); + } + }; } export function initMemberDomainsTable() { diff --git a/src/registrar/assets/src/sass/_theme/_accordions.scss b/src/registrar/assets/src/sass/_theme/_accordions.scss index df4f686d8..762618415 100644 --- a/src/registrar/assets/src/sass/_theme/_accordions.scss +++ b/src/registrar/assets/src/sass/_theme/_accordions.scss @@ -40,7 +40,11 @@ top: 30px; } -tr:last-child .usa-accordion--more-actions .usa-accordion__content { +// Special positioning for the kabob menu popup in the last row on a given page +// This won't work on the Members table rows because that table has show-more rows +// Currently, that's not an issue since that Members table is not wrapped in the +// reponsive wrapper. +tr:last-of-type .usa-accordion--more-actions .usa-accordion__content { top: auto; bottom: -10px; right: 30px; diff --git a/src/registrar/assets/src/sass/_theme/_admin.scss b/src/registrar/assets/src/sass/_theme/_admin.scss index 58ce1e4df..5bb523cac 100644 --- a/src/registrar/assets/src/sass/_theme/_admin.scss +++ b/src/registrar/assets/src/sass/_theme/_admin.scss @@ -176,7 +176,16 @@ html[data-theme="dark"] { color: var(--primary-fg); } +// Reset the USWDS styles for alerts +@include at-media(desktop) { + .dashboard .usa-alert__body--widescreen { + padding-left: 4rem !important; + } + .dashboard .usa-alert__body--widescreen::before { + left: 1.5rem !important; + } +} #branding h1, h1, h2, h3, diff --git a/src/registrar/assets/src/sass/_theme/_alerts.scss b/src/registrar/assets/src/sass/_theme/_alerts.scss index 9579cc057..3164358b7 100644 --- a/src/registrar/assets/src/sass/_theme/_alerts.scss +++ b/src/registrar/assets/src/sass/_theme/_alerts.scss @@ -1,21 +1,18 @@ @use "uswds-core" as *; @use "base" as *; -// Fixes some font size disparities with the Figma -// for usa-alert alert elements -.usa-alert { - .usa-alert__heading.larger-font-sizing { - font-size: units(3); - } -} -.usa-alert__text.measure-none { - max-width: measure(none); -} +/*---------------- + Alert Layout +-----------------*/ // The icon was off center for some reason // Fixes that issue -@media (min-width: 64em){ +@include at-media(desktop) { + // NOTE: !important is used because _font.scss overrides this + .usa-alert__body { + max-width: $widescreen-max-width !important; + } .usa-alert--warning{ .usa-alert__body::before { left: 1rem !important; @@ -24,13 +21,29 @@ .usa-alert__body.margin-left-1 { margin-left: 0.5rem!important; } + + .usa-alert__body--widescreen::before { + left: 4rem !important; + } + .usa-alert__body--widescreen { + padding-left: 7rem!important; + } } -// NOTE: !important is used because _font.scss overrides this -.usa-alert__body--widescreen { - max-width: $widescreen-max-width !important; +/*---------------- + Alert Fonts +-----------------*/ +// Fixes some font size disparities with the Figma +// for usa-alert alert elements +.usa-alert { + .usa-alert__heading.larger-font-sizing { + font-size: 1.5rem; + } } +/*---------------- + Alert Coloring +-----------------*/ .usa-site-alert--hot-pink { .usa-alert { background-color: $hot-pink; @@ -47,3 +60,8 @@ background-color: color('base-darkest'); } } + +// Override the specificity of USWDS css to enable no max width on admin alerts +.usa-alert__body.maxw-none { + max-width: none; +} diff --git a/src/registrar/assets/src/sass/_theme/_base.scss b/src/registrar/assets/src/sass/_theme/_base.scss index 62f9f436e..d73becd75 100644 --- a/src/registrar/assets/src/sass/_theme/_base.scss +++ b/src/registrar/assets/src/sass/_theme/_base.scss @@ -2,6 +2,8 @@ @use "cisa_colors" as *; $widescreen-max-width: 1920px; +$widescreen-x-padding: 4.5rem; + $hot-pink: #FFC3F9; /* Styles for making visible to screen reader / AT users only. */ @@ -39,7 +41,8 @@ body { padding-top: units(5)!important; } -#wrapper.dashboard--grey-1 { +#wrapper.dashboard--grey-1, +.bg-gray-1 { background-color: color('gray-1'); } @@ -252,6 +255,15 @@ abbr[title] { max-width: $widescreen-max-width; } +// This is used in cases where we want to align content to widescreen margins +// but we don't want the content itself to have widescreen widths +@include at-media(desktop) { + .padding-x--widescreen { + padding-left: $widescreen-x-padding !important; + padding-right: $widescreen-x-padding !important; + } +} + .margin-right-neg-4px { margin-right: -4px; } @@ -265,4 +277,8 @@ abbr[title] { margin: 0; height: 1.5em; width: 1.5em; +} + +.maxw-fit-content { + max-width: fit-content; } \ No newline at end of file diff --git a/src/registrar/assets/src/sass/_theme/_containers.scss b/src/registrar/assets/src/sass/_theme/_containers.scss index 7473615ad..24ad480f2 100644 --- a/src/registrar/assets/src/sass/_theme/_containers.scss +++ b/src/registrar/assets/src/sass/_theme/_containers.scss @@ -6,3 +6,21 @@ .usa-identifier__container--widescreen { max-width: $widescreen-max-width !important; } + + +// NOTE: !important is used because we are overriding default +// USWDS paddings in a few locations +@include at-media(desktop) { + .grid-container--widescreen { + padding-left: $widescreen-x-padding !important; + padding-right: $widescreen-x-padding !important; + } +} + +// matches max-width to equal the max-width of .grid-container +// used to trick the eye into thinking we have left-aligned a +// regular grid-container within a widescreen (see instances +// where is_widescreen_centered is used in the html). +.max-width--grid-container { + max-width: 960px; +} \ No newline at end of file diff --git a/src/registrar/assets/src/sass/_theme/_forms.scss b/src/registrar/assets/src/sass/_theme/_forms.scss index 9158de174..4138c5878 100644 --- a/src/registrar/assets/src/sass/_theme/_forms.scss +++ b/src/registrar/assets/src/sass/_theme/_forms.scss @@ -78,3 +78,7 @@ legend.float-left-tablet + button.float-right-tablet { .read-only-value { margin-top: units(0); } + +.bg-gray-1 .usa-radio { + background: color('gray-1'); +} diff --git a/src/registrar/assets/src/sass/_theme/_header.scss b/src/registrar/assets/src/sass/_theme/_header.scss index 53eab90d8..ffb880a7b 100644 --- a/src/registrar/assets/src/sass/_theme/_header.scss +++ b/src/registrar/assets/src/sass/_theme/_header.scss @@ -110,8 +110,8 @@ } } .usa-nav__secondary { - // I don't know why USWDS has this at 2 rem, which puts it out of alignment - right: 3rem; + right: 1rem; + padding-right: $widescreen-x-padding; color: color('white'); bottom: 4.3rem; .usa-nav-link, diff --git a/src/registrar/assets/src/sass/_theme/_register-form.scss b/src/registrar/assets/src/sass/_theme/_register-form.scss index 41d2980e3..fcc5b5ae6 100644 --- a/src/registrar/assets/src/sass/_theme/_register-form.scss +++ b/src/registrar/assets/src/sass/_theme/_register-form.scss @@ -12,7 +12,7 @@ margin-top: units(1); } -// register-form-review-header is used on the summary page and +// header--body is used on the summary page and // should not be styled like the register form headers .register-form-step h3 { color: color('primary-dark'); @@ -25,15 +25,6 @@ } } -.register-form-review-header { - color: color('primary-dark'); - margin-top: units(2); - margin-bottom: 0; - font-weight: font-weight('semibold'); - // The units mixin can only get us close, so it's between - // hardcoding the value and using in markup - font-size: 16.96px; -} .register-form-step h4 { margin-bottom: 0; diff --git a/src/registrar/assets/src/sass/_theme/_typography.scss b/src/registrar/assets/src/sass/_theme/_typography.scss index 7c7639a83..db19a595b 100644 --- a/src/registrar/assets/src/sass/_theme/_typography.scss +++ b/src/registrar/assets/src/sass/_theme/_typography.scss @@ -23,6 +23,14 @@ h2 { color: color('primary-darker'); } +.header--body { + margin-top: units(2); + font-weight: font-weight('semibold'); + // The units mixin can only get us close, so it's between + // hardcoding the value and using in markup + font-size: 16.96px; +} + .h4--sm-05 { font-size: size('body', 'sm'); font-weight: normal; diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 050950c9b..0111245a1 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -251,7 +251,7 @@ TEMPLATES = [ "registrar.context_processors.org_user_status", "registrar.context_processors.add_path_to_context", "registrar.context_processors.portfolio_permissions", - "registrar.context_processors.is_widescreen_mode", + "registrar.context_processors.is_widescreen_centered", ], }, }, diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 66708c571..ef5f7f40a 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -146,7 +146,7 @@ urlpatterns = [ # ), path( "members/new-member/", - views.NewMemberView.as_view(), + views.PortfolioAddMemberView.as_view(), name="new-member", ), path( diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 7230b04c6..b3d9c3727 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -109,31 +109,21 @@ def portfolio_permissions(request): return portfolio_context -def is_widescreen_mode(request): - widescreen_paths = [] # If this list is meant to include specific paths, populate it. - portfolio_widescreen_paths = [ +def is_widescreen_centered(request): + include_paths = [ "/domains/", "/requests/", - "/request/", - "/no-organization-requests/", - "/no-organization-domains/", - "/domain-request/", + "/members/", ] - # widescreen_paths can be a bear as it trickles down sub-urls. exclude_paths gives us a way out. exclude_paths = [ "/domains/edit", + "members/new-member/", ] - # Check if the current path matches a widescreen path or the root path. - is_widescreen = any(path in request.path for path in widescreen_paths) or request.path == "/" + is_excluded = any(exclude_path in request.path for exclude_path in exclude_paths) - # Check if the user is an organization user and the path matches portfolio paths. - is_portfolio_widescreen = ( - hasattr(request.user, "is_org_user") - and request.user.is_org_user(request) - and any(path in request.path for path in portfolio_widescreen_paths) - and not any(exclude_path in request.path for exclude_path in exclude_paths) - ) + # Check if the current path matches a path in included_paths or the root path. + is_widescreen_centered = any(path in request.path for path in include_paths) or request.path == "/" # Return a dictionary with the widescreen mode status. - return {"is_widescreen_mode": is_widescreen or is_portfolio_widescreen} + return {"is_widescreen_centered": is_widescreen_centered and not is_excluded} diff --git a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py index 15265cfa8..5f9fd64ef 100644 --- a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py +++ b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py @@ -60,7 +60,10 @@ class UserPortfolioPermissionFixture: user=user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], - additional_permissions=[UserPortfolioPermissionChoices.EDIT_MEMBERS], + additional_permissions=[ + UserPortfolioPermissionChoices.EDIT_MEMBERS, + UserPortfolioPermissionChoices.EDIT_REQUESTS, + ], ) user_portfolio_permissions_to_create.append(user_portfolio_permission) else: diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 289b3da0b..3b4e3d2ab 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -17,6 +17,7 @@ from registrar.models import Contact, DomainRequest, DraftDomain, Domain, Federa from registrar.templatetags.url_helpers import public_site_url from registrar.utility.enums import ValidationReturnType from registrar.utility.constants import BranchChoices +from django.core.exceptions import ValidationError logger = logging.getLogger(__name__) @@ -78,6 +79,20 @@ class RequestingEntityForm(RegistrarForm): # Otherwise just return the suborg as normal return self.cleaned_data.get("sub_organization") + def clean_requested_suborganization(self): + name = self.cleaned_data.get("requested_suborganization") + if ( + name + and Suborganization.objects.filter( + name__iexact=name, portfolio=self.domain_request.portfolio, name__isnull=False, portfolio__isnull=False + ).exists() + ): + raise ValidationError( + "This suborganization already exists. " + "Choose a new name, or select it directly if you would like to use it." + ) + return name + def full_clean(self): """Validation logic to remove the custom suborganization value before clean is triggered. Without this override, the form will throw an 'invalid option' error.""" @@ -114,7 +129,7 @@ class RequestingEntityForm(RegistrarForm): if requesting_entity_is_suborganization == "True": if is_requesting_new_suborganization: # Validate custom suborganization fields - if not cleaned_data.get("requested_suborganization"): + if not cleaned_data.get("requested_suborganization") and "requested_suborganization" not in self.errors: self.add_error("requested_suborganization", "Enter the name of your suborganization.") if not cleaned_data.get("suborganization_city"): self.add_error("suborganization_city", "Enter the city where your suborganization is located.") diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index eaa885a85..0a8c4d623 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -12,7 +12,6 @@ from registrar.models import ( DomainInformation, Portfolio, SeniorOfficial, - User, ) from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices @@ -111,170 +110,7 @@ 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): +class BasePortfolioMemberForm(forms.ModelForm): """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. @@ -345,13 +181,18 @@ 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.""" + class Meta: + model = None + fields = ["roles", "additional_permissions"] + + def __init__(self, *args, **kwargs): + """ + Override the form's initialization. + + Map existing model values to custom form fields. + Update field descriptions. + """ super().__init__(*args, **kwargs) - if instance: - self.instance = instance - self.initial = self.map_instance_to_initial(self.instance) # Adds a

description beneath each role option self.fields["role"].descriptions = { "organization_admin": UserPortfolioRoleChoices.get_role_description( @@ -361,17 +202,15 @@ class BasePortfolioMemberForm(forms.Form): UserPortfolioRoleChoices.ORGANIZATION_MEMBER ), } - - def save(self): - """Saves self.instance by grabbing data from self.cleaned_data. - Uses map_cleaned_data_to_instance. - """ - self.instance = self.map_cleaned_data_to_instance(self.cleaned_data, self.instance) - self.instance.save() - return self.instance + # Map model instance values to custom form fields + if self.instance: + self.map_instance_to_initial() def clean(self): - """Validates form data based on selected role and its required fields.""" + """Validates form data based on selected role and its required fields. + Updates roles and additional_permissions in cleaned_data so they can be properly + mapped to the model. + """ cleaned_data = super().clean() role = cleaned_data.get("role") @@ -389,20 +228,30 @@ class BasePortfolioMemberForm(forms.Form): if cleaned_data.get("domain_request_permission_member") == "no_access": cleaned_data["domain_request_permission_member"] = None + # Handle roles + cleaned_data["roles"] = [role] + + # Handle additional_permissions + valid_fields = self.ROLE_REQUIRED_FIELDS.get(role, []) + additional_permissions = {cleaned_data.get(field) for field in valid_fields if cleaned_data.get(field)} + + # Handle EDIT permissions (should be accompanied with a view permission) + if UserPortfolioPermissionChoices.EDIT_MEMBERS in additional_permissions: + additional_permissions.add(UserPortfolioPermissionChoices.VIEW_MEMBERS) + + if UserPortfolioPermissionChoices.EDIT_REQUESTS in additional_permissions: + additional_permissions.add(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) + + # Only set unique permissions not already defined in the base role + role_permissions = UserPortfolioPermission.get_portfolio_permissions(cleaned_data["roles"], [], get_list=False) + cleaned_data["additional_permissions"] = list(additional_permissions - role_permissions) + return cleaned_data - # Explanation of how map_instance_to_initial / map_cleaned_data_to_instance work: - # map_instance_to_initial => called on init to set self.initial. - # Converts the incoming object (usually PortfolioInvitation or UserPortfolioPermission) - # into a dictionary representation for the form to use automatically. - - # map_cleaned_data_to_instance => called on save() to save the instance to the db. - # Takes the self.cleaned_data dict, and converts this dict back to the object. - - def map_instance_to_initial(self, instance): + def map_instance_to_initial(self): """ Maps self.instance to self.initial, handling roles and permissions. - Returns form data dictionary with appropriate permission levels based on user role: + Updates self.initial dictionary with appropriate permission levels based on user role: { "role": "organization_admin" or "organization_member", "member_permission_admin": permission level if admin, @@ -410,12 +259,12 @@ class BasePortfolioMemberForm(forms.Form): "domain_request_permission_member": permission level if member } """ + if self.initial is None: + self.initial = {} # Function variables - form_data = {} perms = UserPortfolioPermission.get_portfolio_permissions( - instance.roles, instance.additional_permissions, get_list=False + self.instance.roles, self.instance.additional_permissions, get_list=False ) - # Get the available options for roles, domains, and member. roles = [ UserPortfolioRoleChoices.ORGANIZATION_ADMIN, @@ -433,49 +282,62 @@ 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. - roles = instance.roles or [] + roles = self.instance.roles or [] selected_role = next((role for role in roles if role in roles), None) - form_data = {"role": selected_role} + self.initial["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 + self.initial["domain_request_permission_admin"] = selected_domain_permission + self.initial["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 + self.initial["domain_request_permission_member"] = selected_domain_permission - return form_data - def map_cleaned_data_to_instance(self, cleaned_data, instance): - """ - Maps self.cleaned_data to self.instance, setting roles and permissions. - Args: - cleaned_data (dict): Cleaned data containing role and permission choices - instance: Instance to update +class PortfolioMemberForm(BasePortfolioMemberForm): + """ + Form for updating a portfolio member. + """ - Returns: - instance: Updated instance - """ - role = cleaned_data.get("role") + class Meta: + model = UserPortfolioPermission + fields = ["roles", "additional_permissions"] - # Handle roles - instance.roles = [role] - # Handle additional_permissions - valid_fields = self.ROLE_REQUIRED_FIELDS.get(role, []) - additional_permissions = {cleaned_data.get(field) for field in valid_fields if cleaned_data.get(field)} +class PortfolioInvitedMemberForm(BasePortfolioMemberForm): + """ + Form for updating a portfolio invited member. + """ - # Handle EDIT permissions (should be accompanied with a view permission) - if UserPortfolioPermissionChoices.EDIT_MEMBERS in additional_permissions: - additional_permissions.add(UserPortfolioPermissionChoices.VIEW_MEMBERS) + class Meta: + model = PortfolioInvitation + fields = ["roles", "additional_permissions"] - if UserPortfolioPermissionChoices.EDIT_REQUESTS in additional_permissions: - additional_permissions.add(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) - # Only set unique permissions not already defined in the base role - role_permissions = UserPortfolioPermission.get_portfolio_permissions(instance.roles, [], get_list=False) - instance.additional_permissions = list(additional_permissions - role_permissions) - return instance +class PortfolioNewMemberForm(BasePortfolioMemberForm): + """ + Form for adding a portfolio invited member. + """ + + 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 = PortfolioInvitation + fields = ["portfolio", "email", "roles", "additional_permissions"] diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 44d8511b0..3d3aac769 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -12,6 +12,7 @@ from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTy from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices from auditlog.models import LogEntry +from django.core.exceptions import ValidationError from registrar.utility.waffle import flag_is_active_for_user @@ -671,6 +672,59 @@ class DomainRequest(TimeStampedModel): # Store original values for caching purposes. Used to compare them on save. self._cache_status_and_status_reasons() + def clean(self): + """ + Validates suborganization-related fields in two scenarios: + 1. New suborganization request: Prevents duplicate names within same portfolio + 2. Partial suborganization data: Enforces a all-or-nothing rule for city/state/name fields + when portfolio exists without selected suborganization + + Add new domain request validation rules here to ensure they're + enforced during both model save and form submission. + Not presently used on the domain request wizard, though. + """ + super().clean() + # Validation logic for a suborganization request + if self.is_requesting_new_suborganization(): + # Raise an error if this suborganization already exists + Suborganization = apps.get_model("registrar.Suborganization") + if ( + self.requested_suborganization + and Suborganization.objects.filter( + name__iexact=self.requested_suborganization, + portfolio=self.portfolio, + name__isnull=False, + portfolio__isnull=False, + ).exists() + ): + # Add a field-level error to requested_suborganization. + # To pass in field-specific errors, we need to embed a dict of + # field: validationerror then pass that into a validation error itself. + # This is slightly confusing, but it just adds it at that level. + msg = ( + "This suborganization already exists. " + "Choose a new name, or select it directly if you would like to use it." + ) + errors = {"requested_suborganization": ValidationError(msg)} + raise ValidationError(errors) + elif self.portfolio and not self.sub_organization: + # You cannot create a new suborganization without these fields + required_suborg_fields = { + "requested_suborganization": self.requested_suborganization, + "suborganization_city": self.suborganization_city, + "suborganization_state_territory": self.suborganization_state_territory, + } + # If at least one value is populated, enforce a all-or-nothing rule + if any(bool(value) for value in required_suborg_fields.values()): + # Find which fields are empty and throw an error on the field + errors = {} + for field_name, value in required_suborg_fields.items(): + if not value: + errors[field_name] = ValidationError( + "This field is required when creating a new suborganization.", + ) + raise ValidationError(errors) + def save(self, *args, **kwargs): """Save override for custom properties""" self.sync_organization_type() @@ -690,6 +744,18 @@ class DomainRequest(TimeStampedModel): # Update the cached values after saving self._cache_status_and_status_reasons() + def create_requested_suborganization(self): + """Creates the requested suborganization. + Adds the name, portfolio, city, and state_territory fields. + Returns the created suborganization.""" + Suborganization = apps.get_model("registrar.Suborganization") + return Suborganization.objects.create( + name=self.requested_suborganization, + portfolio=self.portfolio, + city=self.suborganization_city, + state_territory=self.suborganization_state_territory, + ) + def send_custom_status_update_email(self, status): """Helper function to send out a second status email when the status remains the same, but the reason has changed.""" @@ -784,7 +850,9 @@ class DomainRequest(TimeStampedModel): return True def delete_and_clean_up_domain(self, called_from): + # Delete the approved domain try: + # Clean up the approved domain domain_state = self.approved_domain.state # Only reject if it exists on EPP if domain_state != Domain.State.UNKNOWN: @@ -796,6 +864,39 @@ class DomainRequest(TimeStampedModel): logger.error(err) logger.error(f"Can't query an approved domain while attempting {called_from}") + # Delete the suborg as long as this is the only place it is used + self._cleanup_dangling_suborg() + + def _cleanup_dangling_suborg(self): + """Deletes the existing suborg if its only being used by the deleted record""" + # Nothing to delete, so we just smile and walk away + if self.sub_organization is None: + return + + Suborganization = apps.get_model("registrar.Suborganization") + + # Stored as so because we need to set the reference to none first, + # so we can't just use the self.sub_organization property + suborg = Suborganization.objects.get(id=self.sub_organization.id) + requests = suborg.request_sub_organization + domain_infos = suborg.information_sub_organization + + # Check if this is the only reference to the suborganization + if requests.count() != 1 or domain_infos.count() > 1: + return + + # Remove the suborganization reference from request. + self.sub_organization = None + self.save() + + # Remove the suborganization reference from domain if it exists. + if domain_infos.count() == 1: + domain_infos.update(sub_organization=None) + + # Delete the now-orphaned suborganization + logger.info(f"_cleanup_dangling_suborg() -> Deleting orphan suborganization: {suborg}") + suborg.delete() + def _send_status_update_email( self, new_status, @@ -984,6 +1085,7 @@ class DomainRequest(TimeStampedModel): if self.status == self.DomainRequestStatus.APPROVED: self.delete_and_clean_up_domain("action_needed") + elif self.status == self.DomainRequestStatus.REJECTED: self.rejection_reason = None @@ -1014,8 +1116,16 @@ class DomainRequest(TimeStampedModel): domain request into an admin on that domain. It also triggers an email notification.""" + should_save = False if self.federal_agency is None: self.federal_agency = FederalAgency.objects.filter(agency="Non-Federal Agency").first() + should_save = True + + if self.is_requesting_new_suborganization(): + self.sub_organization = self.create_requested_suborganization() + should_save = True + + if should_save: self.save() # create the domain @@ -1148,7 +1258,7 @@ class DomainRequest(TimeStampedModel): def is_requesting_new_suborganization(self) -> bool: """Determines if a user is trying to request a new suborganization using the domain request form, rather than one that already exists. - Used for the RequestingEntity page. + Used for the RequestingEntity page and on DomainInformation.create_from_da(). Returns True if a sub_organization does not exist and if requested_suborganization, suborganization_city, and suborganization_state_territory all exist. diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 25abb6748..03a01b80d 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -171,8 +171,10 @@ class UserPortfolioPermission(TimeStampedModel): # The solution to this is to only grab what is only COMMONLY "forbidden". # This will scale if we add more roles in the future. # This is thes same as applying the `&` operator across all sets for each role. - common_forbidden_perms = set.intersection( - *[set(cls.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) for role in roles] + common_forbidden_perms = ( + set.intersection(*[set(cls.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) for role in roles]) + if roles + else set() ) # Check if the users current permissions overlap with any forbidden permissions diff --git a/src/registrar/templates/401.html b/src/registrar/templates/401.html index d7c7f83ae..7698c4f82 100644 --- a/src/registrar/templates/401.html +++ b/src/registrar/templates/401.html @@ -5,8 +5,8 @@ {% block title %}{% translate "Unauthorized | " %}{% endblock %} {% block content %} -

-
+
+

{% translate "You are not authorized to view this page" %} diff --git a/src/registrar/templates/403.html b/src/registrar/templates/403.html index 999d5f98e..a04453fe9 100644 --- a/src/registrar/templates/403.html +++ b/src/registrar/templates/403.html @@ -5,8 +5,8 @@ {% block title %}{% translate "Forbidden | " %}{% endblock %} {% block content %} -
-
+
+

{% translate "You're not authorized to view this page." %} diff --git a/src/registrar/templates/404.html b/src/registrar/templates/404.html index 471575558..2bf9ecf02 100644 --- a/src/registrar/templates/404.html +++ b/src/registrar/templates/404.html @@ -5,8 +5,8 @@ {% block title %}{% translate "Page not found | " %}{% endblock %} {% block content %} -
-
+
+

{% translate "We couldn’t find that page" %} diff --git a/src/registrar/templates/500.html b/src/registrar/templates/500.html index a0663816b..fad909ddb 100644 --- a/src/registrar/templates/500.html +++ b/src/registrar/templates/500.html @@ -5,8 +5,8 @@ {% block title %}{% translate "Server error | " %}{% endblock %} {% block content %} -
-
+
+

{% translate "We're having some trouble." %} diff --git a/src/registrar/templates/admin/app_list.html b/src/registrar/templates/admin/app_list.html index 49fb59e79..aaf3dc423 100644 --- a/src/registrar/templates/admin/app_list.html +++ b/src/registrar/templates/admin/app_list.html @@ -39,7 +39,7 @@ {% for model in app.models %} {% if model.admin_url %} - {{ model.name }} + {{ model.name }} {% else %} {{ model.name }} {% endif %} diff --git a/src/registrar/templates/admin/fieldset.html b/src/registrar/templates/admin/fieldset.html index 40cd98ca8..20b76217b 100644 --- a/src/registrar/templates/admin/fieldset.html +++ b/src/registrar/templates/admin/fieldset.html @@ -61,7 +61,7 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/ {% if field.field.help_text %} {# .gov override #} {% block help_text %} -
+
{{ field.field.help_text|safe }}
{% endblock help_text %} diff --git a/src/registrar/templates/admin/transfer_user.html b/src/registrar/templates/admin/transfer_user.html index 3ba136b93..61444b173 100644 --- a/src/registrar/templates/admin/transfer_user.html +++ b/src/registrar/templates/admin/transfer_user.html @@ -43,7 +43,7 @@ {% endif %} + {% elif field.field.name == "requested_suborganization" %} + {{ field.field }} +
+ +
{% else %} {{ field.field }} {% endif %} diff --git a/src/registrar/templates/django/admin/includes/details_button.html b/src/registrar/templates/django/admin/includes/details_button.html index 73748f170..65c6d768b 100644 --- a/src/registrar/templates/django/admin/includes/details_button.html +++ b/src/registrar/templates/django/admin/includes/details_button.html @@ -1,6 +1,6 @@ {% comment %} This view provides a detail button that can be used to show/hide content {% endcomment %} -
+
Details
{% block detail_content %} diff --git a/src/registrar/templates/django/admin/includes/domain_fieldset.html b/src/registrar/templates/django/admin/includes/domain_fieldset.html index d5f5bc1af..c621deaac 100644 --- a/src/registrar/templates/django/admin/includes/domain_fieldset.html +++ b/src/registrar/templates/django/admin/includes/domain_fieldset.html @@ -11,7 +11,7 @@ {% endblock %} {% block help_text %} -
+
{% if field.field.name == "state" %}
{{ state_help_message }}
{% else %} diff --git a/src/registrar/templates/django/admin/multiple_choice_list_filter.html b/src/registrar/templates/django/admin/multiple_choice_list_filter.html index 66643f4ec..167059594 100644 --- a/src/registrar/templates/django/admin/multiple_choice_list_filter.html +++ b/src/registrar/templates/django/admin/multiple_choice_list_filter.html @@ -6,14 +6,14 @@
    {% for choice in choices %} {% if choice.reset %} - + {{ choice.display }} {% endif %} {% endfor %} {% for choice in choices %} {% if not choice.reset %} - + {% if choice.selected and choice.exclude_query_string %} {{ choice.display }}
    +
    +

    + If you add someone to a portfolio here, it will trigger an invitation email when you click "save." If you don't want to trigger an email, use the User portfolio permissions table instead. +

    +
    +
    + {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html b/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html index 1249a486c..489d67bc5 100644 --- a/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html +++ b/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html @@ -2,15 +2,13 @@ {% load custom_filters %} {% load i18n static %} -{% block field_sets %} - {% for fieldset in adminform %} - {% comment %} - This is a placeholder for now. - - Disclaimer: - When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset. - detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences. - {% endcomment %} - {% include "django/admin/includes/user_portfolio_permission_fieldset.html" with original_object=original %} - {% endfor %} -{% endblock %} \ No newline at end of file +{% block content_subtitle %} +
    +
    +

    + If you add someone to a portfolio here, it will not trigger an invitation email. To trigger an email, use the Portfolio invitations table instead. +

    +
    +
    + {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/domain_base.html b/src/registrar/templates/domain_base.html index 9f7e8d2e6..b65e9399b 100644 --- a/src/registrar/templates/domain_base.html +++ b/src/registrar/templates/domain_base.html @@ -4,10 +4,10 @@ {% block title %}{{ domain.name }} | {% endblock %} {% block content %} -
    +
    -
    -
    +
    +

    diff --git a/src/registrar/templates/domain_request_form.html b/src/registrar/templates/domain_request_form.html index 2c2716a3c..d8b020cc1 100644 --- a/src/registrar/templates/domain_request_form.html +++ b/src/registrar/templates/domain_request_form.html @@ -3,8 +3,8 @@ {% block title %}{{form_titles|get_item:steps.current}} | Request a .gov | {% endblock %} {% block content %} -

    -
    +
    +
    {% include 'domain_request_sidebar.html' %}
    diff --git a/src/registrar/templates/domain_request_intro.html b/src/registrar/templates/domain_request_intro.html index dd5b7ec6e..263201393 100644 --- a/src/registrar/templates/domain_request_intro.html +++ b/src/registrar/templates/domain_request_intro.html @@ -4,41 +4,42 @@ {% block title %} Start a request | {% endblock %} {% block content %} -
    -
    +
    +
    +
    -
    - {% csrf_token %} + + {% csrf_token %} -

    You’re about to start your .gov domain request.

    -

    You don’t have to complete the process in one session. You can save what you enter and come back to it when you’re ready.

    - {% if portfolio %} -

    We’ll use the information you provide to verify your domain request meets our guidelines.

    - {% else %} -

    We’ll use the information you provide to verify your organization’s eligibility for a .gov domain. We’ll also verify that the domain you request meets our guidelines.

    - {% endif %} -

    Time to complete the form

    -

    If you have all the information you need, - completing your domain request might take around 15 minutes.

    -

    How we’ll reach you

    -

    While reviewing your domain request, we may need to reach out with questions. We’ll also email you when we complete our review. If the contact information below is not correct, visit your profile to make updates.

    - {% include "includes/profile_information.html" with user=user%} - +

    You’re about to start your .gov domain request.

    +

    You don’t have to complete the process in one session. You can save what you enter and come back to it when you’re ready.

    + {% if portfolio %} +

    We’ll use the information you provide to verify your domain request meets our guidelines.

    + {% else %} +

    We’ll use the information you provide to verify your organization’s eligibility for a .gov domain. We’ll also verify that the domain you request meets our guidelines.

    + {% endif %} +

    Time to complete the form

    +

    If you have all the information you need, + completing your domain request might take around 15 minutes.

    +

    How we’ll reach you

    +

    While reviewing your domain request, we may need to reach out with questions. We’ll also email you when we complete our review. If the contact information below is not correct, visit your profile to make updates.

    + {% include "includes/profile_information.html" with user=user%} + -{% block form_buttons %} -
    - -
    -{% endblock %} + {% block form_buttons %} +
    + +
    + {% endblock %} -
    + -
    Paperwork Reduction Act statement (OMB control number: 1670-0049; expiration date: 10/31/2026)
    -
    +
    Paperwork Reduction Act statement (OMB control number: 1670-0049; expiration date: 10/31/2026)
    +
    {% endblock %} diff --git a/src/registrar/templates/domain_request_withdraw_confirmation.html b/src/registrar/templates/domain_request_withdraw_confirmation.html index e1a5f0c2a..525c6784a 100644 --- a/src/registrar/templates/domain_request_withdraw_confirmation.html +++ b/src/registrar/templates/domain_request_withdraw_confirmation.html @@ -8,18 +8,20 @@ {% endblock wrapperdiv %} {% block content %} -
    -
    - +
    +
    +
    + -

    Withdraw request for {{ DomainRequest.requested_domain.name }}?

    +

    Withdraw request for {{ DomainRequest.requested_domain.name }}?

    -

    If you withdraw your request, we won't review it. Once you withdraw your request, you can edit it and submit it again.

    +

    If you withdraw your request, we won't review it. Once you withdraw your request, you can edit it and submit it again.

    -

    Withdraw request - Cancel

    +

    Withdraw request + Cancel

    -
    +
    +
    {% endblock %} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 289f544ce..99ca1bfb7 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -18,7 +18,7 @@
  • {% url 'domain-dns' pk=domain.id as url %} - + DNS {% if request.path|startswith:url %} diff --git a/src/registrar/templates/emails/portfolio_invitation.txt b/src/registrar/templates/emails/portfolio_invitation.txt new file mode 100644 index 000000000..775b74c7c --- /dev/null +++ b/src/registrar/templates/emails/portfolio_invitation.txt @@ -0,0 +1,34 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +Hi. + +{{ requestor_email }} has invited you to {{ portfolio.organization_name }}. + +You can view this organization on the .gov registrar . + +---------------------------------------------------------------- + +YOU NEED A LOGIN.GOV ACCOUNT +You’ll need a Login.gov account to access this .gov organization. That account +needs to be associated with the following email address: {{ email }} + +Login.gov provides a simple and secure process for signing in to many government +services with one account. If you don’t already have one, follow these steps to +create your Login.gov account . + + +SOMETHING WRONG? +If you’re not affiliated with {{ portfolio.organization_name }} or think you received this +message in error, reply to this email. + + +THANK YOU +.Gov helps the public identify official, trusted information. Thank you for using a .gov domain. + +---------------------------------------------------------------- + +The .gov team +Contact us: +Learn about .gov + +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +{% endautoescape %} diff --git a/src/registrar/templates/emails/portfolio_invitation_subject.txt b/src/registrar/templates/emails/portfolio_invitation_subject.txt new file mode 100644 index 000000000..552bb2bec --- /dev/null +++ b/src/registrar/templates/emails/portfolio_invitation_subject.txt @@ -0,0 +1 @@ +You’ve been invited to a .gov organization \ No newline at end of file diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index b1c3775df..de4d9e712 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -5,12 +5,12 @@ {% block title %} Home | {% endblock %} {% block content %} -
    +
    {% if user.is_authenticated %} {# the entire logged in page goes here #} {% block homepage_content %} -
    +
    {% block messages %} {% include "includes/form_messages.html" %} {% endblock %} diff --git a/src/registrar/templates/includes/banner-error.html b/src/registrar/templates/includes/banner-error.html index 7b5c32ed1..10582e268 100644 --- a/src/registrar/templates/includes/banner-error.html +++ b/src/registrar/templates/includes/banner-error.html @@ -1,6 +1,6 @@
    -
    +

    Header

    diff --git a/src/registrar/templates/includes/banner-info.html b/src/registrar/templates/includes/banner-info.html index e5d54e483..cf379c50d 100644 --- a/src/registrar/templates/includes/banner-info.html +++ b/src/registrar/templates/includes/banner-info.html @@ -1,6 +1,6 @@
    -
    +

    Header

    diff --git a/src/registrar/templates/includes/banner-non-production-alert.html b/src/registrar/templates/includes/banner-non-production-alert.html index 61d4eed51..7b66d543b 100644 --- a/src/registrar/templates/includes/banner-non-production-alert.html +++ b/src/registrar/templates/includes/banner-non-production-alert.html @@ -1,6 +1,6 @@
    -
    +

    Attention: You are on a test site.

    diff --git a/src/registrar/templates/includes/banner-service-disruption.html b/src/registrar/templates/includes/banner-service-disruption.html index fc89ee65d..6ee4b976b 100644 --- a/src/registrar/templates/includes/banner-service-disruption.html +++ b/src/registrar/templates/includes/banner-service-disruption.html @@ -1,6 +1,6 @@
    -
    +

    Service disruption

    diff --git a/src/registrar/templates/includes/banner-site-alert.html b/src/registrar/templates/includes/banner-site-alert.html index 52256f46b..8dd657267 100644 --- a/src/registrar/templates/includes/banner-site-alert.html +++ b/src/registrar/templates/includes/banner-site-alert.html @@ -1,6 +1,6 @@
    -
    +

    Header here

    diff --git a/src/registrar/templates/includes/banner-system-outage.html b/src/registrar/templates/includes/banner-system-outage.html index 911fa4487..60fc4eb03 100644 --- a/src/registrar/templates/includes/banner-system-outage.html +++ b/src/registrar/templates/includes/banner-system-outage.html @@ -1,6 +1,6 @@
    -
    +

    System outage

    diff --git a/src/registrar/templates/includes/banner-warning.html b/src/registrar/templates/includes/banner-warning.html index 6838aef7b..762d0b47c 100644 --- a/src/registrar/templates/includes/banner-warning.html +++ b/src/registrar/templates/includes/banner-warning.html @@ -1,6 +1,6 @@
    -
    +

    Header

    diff --git a/src/registrar/templates/includes/domain_request_status_manage.html b/src/registrar/templates/includes/domain_request_status_manage.html deleted file mode 100644 index 2a254df4b..000000000 --- a/src/registrar/templates/includes/domain_request_status_manage.html +++ /dev/null @@ -1,236 +0,0 @@ -{% load custom_filters %} -{% load static url_helpers %} -
    -
    - {% block breadcrumb %} - {% if portfolio %} - {% url 'domain-requests' as url %} - {% else %} - {% url 'home' as url %} - {% endif %} - - {% endblock breadcrumb %} - - {% block header %} - {% if not DomainRequest.requested_domain and DomainRequest.status == DomainRequest.DomainRequestStatus.STARTED %} -

    New domain request

    - {% else %} -

    Domain request for {{ DomainRequest.requested_domain.name }}

    - {% endif %} - {% endblock header %} - - {% block status_summary %} -
    -
    -

    - - Status: - - {{ DomainRequest.get_status_display|default:"ERROR Please contact technical support/dev" }} -

    -
    -
    -
    - {% endblock status_summary %} - - {% block status_metadata %} - - {% if portfolio %} - {% if DomainRequest.creator %} -

    - Created by: {{DomainRequest.creator.email|default:DomainRequest.creator.get_formatted_name }} -

    - {% else %} -

    - No creator found: this is an error, please email help@get.gov. -

    - {% endif %} - {% endif %} - - {% with statuses=DomainRequest.DomainRequestStatus last_submitted=DomainRequest.last_submitted_date|date:"F j, Y" first_submitted=DomainRequest.first_submitted_date|date:"F j, Y" last_status_update=DomainRequest.last_status_update|date:"F j, Y" %} - {% comment %} - These are intentionally seperated this way. - There is some code repetition, but it gives us more flexibility rather than a dense reduction. - Leave it this way until we've solidified our requirements. - {% endcomment %} - {% if DomainRequest.status == statuses.STARTED %} - {% with first_started_date=DomainRequest.get_first_status_started_date|date:"F j, Y" %} -

    - {% comment %} - A newly created domain request will not have a value for last_status update. - This is because the status never really updated. - However, if this somehow goes back to started we can default to displaying that new date. - {% endcomment %} - Started on: {{last_status_update|default:first_started_date}} -

    - {% endwith %} - {% elif DomainRequest.status == statuses.SUBMITTED %} -

    - Submitted on: {{last_submitted|default:first_submitted }} -

    -

    - Last updated on: {{DomainRequest.updated_at|date:"F j, Y"}} -

    - {% elif DomainRequest.status == statuses.ACTION_NEEDED %} -

    - Submitted on: {{last_submitted|default:first_submitted }} -

    -

    - Last updated on: {{DomainRequest.updated_at|date:"F j, Y"}} -

    - {% elif DomainRequest.status == statuses.REJECTED %} -

    - Submitted on: {{last_submitted|default:first_submitted }} -

    -

    - Rejected on: {{last_status_update}} -

    - {% elif DomainRequest.status == statuses.WITHDRAWN %} -

    - Submitted on: {{last_submitted|default:first_submitted }} -

    -

    - Withdrawn on: {{last_status_update}} -

    - {% else %} - {% comment %} Shown for in_review, approved, ineligible {% endcomment %} -

    - Last updated on: {{DomainRequest.updated_at|date:"F j, Y"}} -

    - {% endif %} - {% endwith %} - {% endblock status_metadata %} - - {% block status_blurb %} - {% if DomainRequest.is_awaiting_review %} -

    {% include "includes/domain_request_awaiting_review.html" with show_withdraw_text=DomainRequest.is_withdrawable %}

    - {% endif %} - {% endblock status_blurb %} - - {% block modify_request %} - {% if DomainRequest.is_withdrawable %} -

    - Withdraw request -

    - {% endif %} - {% endblock modify_request %} -
    - -
    - {% block request_summary_header %} -

    Summary of your domain request

    - {% endblock request_summary_header%} - - {% block request_summary %} - {% with heading_level='h3' %} - {% with org_type=DomainRequest.get_generic_org_type_display %} - {% include "includes/summary_item.html" with title='Type of organization' value=org_type heading_level=heading_level %} - {% endwith %} - - {% if DomainRequest.tribe_name %} - {% include "includes/summary_item.html" with title='Tribal government' value=DomainRequest.tribe_name heading_level=heading_level %} - - {% if DomainRequest.federally_recognized_tribe %} -

    Federally-recognized tribe

    - {% endif %} - - {% if DomainRequest.state_recognized_tribe %} -

    State-recognized tribe

    - {% endif %} - - {% endif %} - - {% if DomainRequest.get_federal_type_display %} - {% include "includes/summary_item.html" with title='Federal government branch' value=DomainRequest.get_federal_type_display heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.is_election_board %} - {% with value=DomainRequest.is_election_board|yesno:"Yes,No,Incomplete" %} - {% include "includes/summary_item.html" with title='Election office' value=value heading_level=heading_level %} - {% endwith %} - {% endif %} - - {% if DomainRequest.organization_name %} - {% include "includes/summary_item.html" with title='Organization' value=DomainRequest address='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.about_your_organization %} - {% include "includes/summary_item.html" with title='About your organization' value=DomainRequest.about_your_organization heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.senior_official %} - {% include "includes/summary_item.html" with title='Senior official' value=DomainRequest.senior_official contact='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.current_websites.all %} - {% include "includes/summary_item.html" with title='Current websites' value=DomainRequest.current_websites.all list='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.requested_domain %} - {% include "includes/summary_item.html" with title='.gov domain' value=DomainRequest.requested_domain heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.alternative_domains.all %} - {% include "includes/summary_item.html" with title='Alternative domains' value=DomainRequest.alternative_domains.all list='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.purpose %} - {% include "includes/summary_item.html" with title='Purpose of your domain' value=DomainRequest.purpose heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.creator %} - {% include "includes/summary_item.html" with title='Your contact information' value=DomainRequest.creator contact='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.other_contacts.all %} - {% include "includes/summary_item.html" with title='Other employees from your organization' value=DomainRequest.other_contacts.all contact='true' list='true' heading_level=heading_level %} - {% else %} - {% include "includes/summary_item.html" with title='Other employees from your organization' value=DomainRequest.no_other_contacts_rationale heading_level=heading_level %} - {% endif %} - - {# We always show this field even if None #} - {% if DomainRequest %} -

    CISA Regional Representative

    -
      - {% if DomainRequest.cisa_representative_first_name %} - {{ DomainRequest.get_formatted_cisa_rep_name }} - {% else %} - No - {% endif %} -
    -

    Anything else

    -
      - {% if DomainRequest.anything_else %} - {{DomainRequest.anything_else}} - {% else %} - No - {% endif %} -
    - {% endif %} - {% endwith %} - {% endblock request_summary%} -
    -
    \ No newline at end of file diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index 56cdc2cec..c48e2c9a6 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -4,8 +4,8 @@ {% url 'get_domain_requests_json' as url %} -
    -
    +
    +
    {% if not portfolio %}

    Domain requests

    {% else %} @@ -13,7 +13,7 @@ {% endif %} -
    +

-
+
diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index d141dbaa6..19320a4ae 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -8,7 +8,7 @@
-
+
{% csrf_token %} diff --git a/src/registrar/templates/includes/portfolio_request_review_steps.html b/src/registrar/templates/includes/portfolio_request_review_steps.html index fcb087090..eecc5005a 100644 --- a/src/registrar/templates/includes/portfolio_request_review_steps.html +++ b/src/registrar/templates/includes/portfolio_request_review_steps.html @@ -46,7 +46,7 @@ {% endwith %} {% if domain_request.alternative_domains.all %} -

Alternative domains

+

Alternative domains

    {% for site in domain_request.alternative_domains.all %}
  • {{ site.website }}
    • diff --git a/src/registrar/templates/includes/request_review_steps.html b/src/registrar/templates/includes/request_review_steps.html index 73b71d536..dada2dffb 100644 --- a/src/registrar/templates/includes/request_review_steps.html +++ b/src/registrar/templates/includes/request_review_steps.html @@ -88,7 +88,7 @@ {% endwith %} {% if domain_request.alternative_domains.all %} -

    Alternative domains

    +

    Alternative domains

      {% for site in domain_request.alternative_domains.all %}
    • {{ site.website }}
      • @@ -132,7 +132,7 @@ {% with title=form_titles|get_item:step %} {% if domain_request.has_additional_details %} {% include "includes/summary_item.html" with title="Additional Details" value=" " heading_level=heading_level editable=is_editable edit_link=domain_request_url %} -

      CISA Regional Representative

      +

      CISA Regional Representative

        {% if domain_request.cisa_representative_first_name %}
      • {{domain_request.cisa_representative_first_name}} {{domain_request.cisa_representative_last_name}}
        • @@ -144,7 +144,7 @@ {% endif %}
      -

      Anything else

      +

      Anything else

        {% if domain_request.anything_else %} {{domain_request.anything_else}} diff --git a/src/registrar/templates/includes/request_status_manage.html b/src/registrar/templates/includes/request_status_manage.html index fc2fd8f12..b4738d0d0 100644 --- a/src/registrar/templates/includes/request_status_manage.html +++ b/src/registrar/templates/includes/request_status_manage.html @@ -1,222 +1,223 @@ {% load custom_filters %} {% load static url_helpers %} -
        -
        - {% block breadcrumb %} - {% if portfolio %} - {% url 'domain-requests' as url %} - {% else %} - {% url 'home' as url %} - {% endif %} - - {% endblock breadcrumb %} - {% block header %} - {% if not DomainRequest.requested_domain and DomainRequest.status == DomainRequest.DomainRequestStatus.STARTED %} -

        New domain request

        - {% else %} -

        Domain request for {{ DomainRequest.requested_domain.name }}

        - {% endif %} - {% endblock header %} - - {% block status_summary %} -
        -
        -

        - - Status: - - {{ DomainRequest.get_status_display|default:"ERROR Please contact technical support/dev" }} -

        -
        -
        -
        - {% endblock status_summary %} - - {% block status_metadata %} - - {% if portfolio %} - {% if DomainRequest.creator %} -

        - Created by: {{DomainRequest.creator.email|default:DomainRequest.creator.get_formatted_name }} -

        - {% else %} -

        - No creator found: this is an error, please email help@get.gov. -

        - {% endif %} - {% endif %} - - {% with statuses=DomainRequest.DomainRequestStatus last_submitted=DomainRequest.last_submitted_date|date:"F j, Y" first_submitted=DomainRequest.first_submitted_date|date:"F j, Y" last_status_update=DomainRequest.last_status_update|date:"F j, Y" %} - {% comment %} - These are intentionally seperated this way. - There is some code repetition, but it gives us more flexibility rather than a dense reduction. - Leave it this way until we've solidified our requirements. - {% endcomment %} - {% if DomainRequest.status == statuses.STARTED %} - {% with first_started_date=DomainRequest.get_first_status_started_date|date:"F j, Y" %} -

        + {% with statuses=DomainRequest.DomainRequestStatus last_submitted=DomainRequest.last_submitted_date|date:"F j, Y" first_submitted=DomainRequest.first_submitted_date|date:"F j, Y" last_status_update=DomainRequest.last_status_update|date:"F j, Y" %} {% comment %} - A newly created domain request will not have a value for last_status update. - This is because the status never really updated. - However, if this somehow goes back to started we can default to displaying that new date. + These are intentionally seperated this way. + There is some code repetition, but it gives us more flexibility rather than a dense reduction. + Leave it this way until we've solidified our requirements. {% endcomment %} - Started on: {{last_status_update|default:first_started_date}} -

        - {% endwith %} - {% elif DomainRequest.status == statuses.SUBMITTED %} -

        - Submitted on: {{last_submitted|default:first_submitted }} -

        -

        - Last updated on: {{DomainRequest.updated_at|date:"F j, Y"}} -

        - {% elif DomainRequest.status == statuses.ACTION_NEEDED %} -

        - Submitted on: {{last_submitted|default:first_submitted }} -

        -

        - Last updated on: {{DomainRequest.updated_at|date:"F j, Y"}} -

        - {% elif DomainRequest.status == statuses.REJECTED %} -

        - Submitted on: {{last_submitted|default:first_submitted }} -

        -

        - Rejected on: {{last_status_update}} -

        - {% elif DomainRequest.status == statuses.WITHDRAWN %} -

        - Submitted on: {{last_submitted|default:first_submitted }} -

        -

        - Withdrawn on: {{last_status_update}} -

        - {% else %} - {% comment %} Shown for in_review, approved, ineligible {% endcomment %} -

        - Last updated on: {{DomainRequest.updated_at|date:"F j, Y"}} -

        - {% endif %} - {% endwith %} - {% endblock status_metadata %} - - {% block status_blurb %} - {% if DomainRequest.is_awaiting_review %} -

        {% include "includes/domain_request_awaiting_review.html" with show_withdraw_text=DomainRequest.is_withdrawable %}

        - {% endif %} - {% endblock status_blurb %} - - {% block modify_request %} - {% if DomainRequest.is_withdrawable %} -

        - Withdraw request + {% if DomainRequest.status == statuses.STARTED %} + {% with first_started_date=DomainRequest.get_first_status_started_date|date:"F j, Y" %} +

        + {% comment %} + A newly created domain request will not have a value for last_status update. + This is because the status never really updated. + However, if this somehow goes back to started we can default to displaying that new date. + {% endcomment %} + Started on: {{last_status_update|default:first_started_date}}

        - {% endif %} - {% endblock modify_request %} -
        - -
        - {% block request_summary_header %} -

        Summary of your domain request

        - {% endblock request_summary_header%} - - {% block request_summary %} - {% if portfolio %} - {% include "includes/portfolio_request_review_steps.html" with is_editable=False domain_request=DomainRequest %} - {% else %} - {% with heading_level='h3' %} - {% with org_type=DomainRequest.get_generic_org_type_display %} - {% include "includes/summary_item.html" with title='Type of organization' value=org_type heading_level=heading_level %} {% endwith %} - - {% if DomainRequest.tribe_name %} - {% include "includes/summary_item.html" with title='Tribal government' value=DomainRequest.tribe_name heading_level=heading_level %} - - {% if DomainRequest.federally_recognized_tribe %} -

        Federally-recognized tribe

        - {% endif %} - - {% if DomainRequest.state_recognized_tribe %} -

        State-recognized tribe

        - {% endif %} - - {% endif %} - - {% if DomainRequest.get_federal_type_display %} - {% include "includes/summary_item.html" with title='Federal government branch' value=DomainRequest.get_federal_type_display heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.is_election_board %} - {% with value=DomainRequest.is_election_board|yesno:"Yes,No,Incomplete" %} - {% include "includes/summary_item.html" with title='Election office' value=value heading_level=heading_level %} - {% endwith %} - {% endif %} - - {% if DomainRequest.organization_name %} - {% include "includes/summary_item.html" with title='Organization' value=DomainRequest address='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.about_your_organization %} - {% include "includes/summary_item.html" with title='About your organization' value=DomainRequest.about_your_organization heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.senior_official %} - {% include "includes/summary_item.html" with title='Senior official' value=DomainRequest.senior_official contact='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.current_websites.all %} - {% include "includes/summary_item.html" with title='Current websites' value=DomainRequest.current_websites.all list='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.requested_domain %} - {% include "includes/summary_item.html" with title='.gov domain' value=DomainRequest.requested_domain heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.alternative_domains.all %} - {% include "includes/summary_item.html" with title='Alternative domains' value=DomainRequest.alternative_domains.all list='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.purpose %} - {% include "includes/summary_item.html" with title='Purpose of your domain' value=DomainRequest.purpose heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.creator %} - {% include "includes/summary_item.html" with title='Your contact information' value=DomainRequest.creator contact='true' heading_level=heading_level %} - {% endif %} - - {% if DomainRequest.other_contacts.all %} - {% include "includes/summary_item.html" with title='Other employees from your organization' value=DomainRequest.other_contacts.all contact='true' list='true' heading_level=heading_level %} + {% elif DomainRequest.status == statuses.SUBMITTED %} +

        + Submitted on: {{last_submitted|default:first_submitted }} +

        +

        + Last updated on: {{DomainRequest.updated_at|date:"F j, Y"}} +

        + {% elif DomainRequest.status == statuses.ACTION_NEEDED %} +

        + Submitted on: {{last_submitted|default:first_submitted }} +

        +

        + Last updated on: {{DomainRequest.updated_at|date:"F j, Y"}} +

        + {% elif DomainRequest.status == statuses.REJECTED %} +

        + Submitted on: {{last_submitted|default:first_submitted }} +

        +

        + Rejected on: {{last_status_update}} +

        + {% elif DomainRequest.status == statuses.WITHDRAWN %} +

        + Submitted on: {{last_submitted|default:first_submitted }} +

        +

        + Withdrawn on: {{last_status_update}} +

        {% else %} - {% include "includes/summary_item.html" with title='Other employees from your organization' value=DomainRequest.no_other_contacts_rationale heading_level=heading_level %} + {% comment %} Shown for in_review, approved, ineligible {% endcomment %} +

        + Last updated on: {{DomainRequest.updated_at|date:"F j, Y"}} +

        {% endif %} + {% endwith %} + {% endblock status_metadata %} + + {% block status_blurb %} + {% if DomainRequest.is_awaiting_review %} +

        {% include "includes/domain_request_awaiting_review.html" with show_withdraw_text=DomainRequest.is_withdrawable %}

        + {% endif %} + {% endblock status_blurb %} + + {% block modify_request %} + {% if DomainRequest.is_withdrawable %} +

        + Withdraw request +

        + {% endif %} + {% endblock modify_request %} +
        + +
        + {% block request_summary_header %} +

        Summary of your domain request

        + {% endblock request_summary_header%} + + {% block request_summary %} + {% if portfolio %} + {% include "includes/portfolio_request_review_steps.html" with is_editable=False domain_request=DomainRequest %} + {% else %} + {% with heading_level='h3' %} + {% with org_type=DomainRequest.get_generic_org_type_display %} + {% include "includes/summary_item.html" with title='Type of organization' value=org_type heading_level=heading_level %} + {% endwith %} + + {% if DomainRequest.tribe_name %} + {% include "includes/summary_item.html" with title='Tribal government' value=DomainRequest.tribe_name heading_level=heading_level %} + + {% if DomainRequest.federally_recognized_tribe %} +

        Federally-recognized tribe

        + {% endif %} + + {% if DomainRequest.state_recognized_tribe %} +

        State-recognized tribe

        + {% endif %} + + {% endif %} + + {% if DomainRequest.get_federal_type_display %} + {% include "includes/summary_item.html" with title='Federal government branch' value=DomainRequest.get_federal_type_display heading_level=heading_level %} + {% endif %} + + {% if DomainRequest.is_election_board %} + {% with value=DomainRequest.is_election_board|yesno:"Yes,No,Incomplete" %} + {% include "includes/summary_item.html" with title='Election office' value=value heading_level=heading_level %} + {% endwith %} + {% endif %} + + {% if DomainRequest.organization_name %} + {% include "includes/summary_item.html" with title='Organization' value=DomainRequest address='true' heading_level=heading_level %} + {% endif %} + + {% if DomainRequest.about_your_organization %} + {% include "includes/summary_item.html" with title='About your organization' value=DomainRequest.about_your_organization heading_level=heading_level %} + {% endif %} + + {% if DomainRequest.senior_official %} + {% include "includes/summary_item.html" with title='Senior official' value=DomainRequest.senior_official contact='true' heading_level=heading_level %} + {% endif %} + + {% if DomainRequest.current_websites.all %} + {% include "includes/summary_item.html" with title='Current websites' value=DomainRequest.current_websites.all list='true' heading_level=heading_level %} + {% endif %} + + {% if DomainRequest.requested_domain %} + {% include "includes/summary_item.html" with title='.gov domain' value=DomainRequest.requested_domain heading_level=heading_level %} + {% endif %} + + {% if DomainRequest.alternative_domains.all %} + {% include "includes/summary_item.html" with title='Alternative domains' value=DomainRequest.alternative_domains.all list='true' heading_level=heading_level %} + {% endif %} + + {% if DomainRequest.purpose %} + {% include "includes/summary_item.html" with title='Purpose of your domain' value=DomainRequest.purpose heading_level=heading_level %} + {% endif %} + + {% if DomainRequest.creator %} + {% include "includes/summary_item.html" with title='Your contact information' value=DomainRequest.creator contact='true' heading_level=heading_level %} + {% endif %} + + {% if DomainRequest.other_contacts.all %} + {% include "includes/summary_item.html" with title='Other employees from your organization' value=DomainRequest.other_contacts.all contact='true' list='true' heading_level=heading_level %} + {% else %} + {% include "includes/summary_item.html" with title='Other employees from your organization' value=DomainRequest.no_other_contacts_rationale heading_level=heading_level %} + {% endif %} {# We always show this field even if None #} {% if DomainRequest %} -

        CISA Regional Representative

        +

        CISA Regional Representative

          {% if DomainRequest.cisa_representative_first_name %} {{ DomainRequest.get_formatted_cisa_rep_name }} @@ -224,7 +225,7 @@ No {% endif %}
        -

        Anything else

        +

        Anything else

          {% if DomainRequest.anything_else %} {{DomainRequest.anything_else}} @@ -236,5 +237,6 @@ {% endwith %} {% endif %} {% endblock request_summary%} +
\ No newline at end of file diff --git a/src/registrar/templates/includes/required_fields.html b/src/registrar/templates/includes/required_fields.html index be0395979..a9d860f24 100644 --- a/src/registrar/templates/includes/required_fields.html +++ b/src/registrar/templates/includes/required_fields.html @@ -1,3 +1,3 @@ -

+

Required fields are marked with an asterisk (*).

diff --git a/src/registrar/templates/includes/senior_official.html b/src/registrar/templates/includes/senior_official.html index 19b443fcd..52b4ab6eb 100644 --- a/src/registrar/templates/includes/senior_official.html +++ b/src/registrar/templates/includes/senior_official.html @@ -1,5 +1,9 @@ {% load static field_helpers url_helpers %} + +{% block messages %} +{% include "includes/form_messages.html" %} +{% endblock messages%} {% if can_edit %} {% include "includes/form_errors.html" with form=form %} {% endif %} diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index 15cc0f67f..bbdfc8dee 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -22,7 +22,7 @@

{% endif %} {% if sub_header_text %} -

{{ sub_header_text }}

+

{{ sub_header_text }}

{% endif %} {% if permissions %} {% include "includes/member_permissions.html" with permissions=value %} diff --git a/src/registrar/templates/portfolio_base.html b/src/registrar/templates/portfolio_base.html index c2a60b7ba..9f43c7251 100644 --- a/src/registrar/templates/portfolio_base.html +++ b/src/registrar/templates/portfolio_base.html @@ -4,26 +4,26 @@
{% block content %} -
+
{% if user.is_authenticated %} {# the entire logged in page goes here #} -
- {% block messages %} - {% include "includes/form_messages.html" %} - {% endblock %} +
+
{% block portfolio_content %}{% endblock %} -
- {% else %} {# not user.is_authenticated #} - {# the entire logged out page goes here #} - -

- Sign in -

- - {% endif %} +
+ {% else %} {# not user.is_authenticated #} + {# the entire logged out page goes here #} + +

+ Sign in +

+ + {% endif %} +
+
{% endblock content%} diff --git a/src/registrar/templates/portfolio_domains.html b/src/registrar/templates/portfolio_domains.html index 4fd99ce8e..55d807e78 100644 --- a/src/registrar/templates/portfolio_domains.html +++ b/src/registrar/templates/portfolio_domains.html @@ -9,9 +9,11 @@ {% endblock %} {% block portfolio_content %} + + {% block messages %} {% include "includes/form_messages.html" %} -{% endblock %} +{% endblock messages%}

Domains

diff --git a/src/registrar/templates/portfolio_member.html b/src/registrar/templates/portfolio_member.html index f492dbd2f..3fa4fed14 100644 --- a/src/registrar/templates/portfolio_member.html +++ b/src/registrar/templates/portfolio_member.html @@ -8,7 +8,12 @@ Organization member {% load static %} {% block portfolio_content %} -
+
+ + {% include "includes/form_errors.html" with form=form %} + {% block messages %} + {% include "includes/form_messages.html" %} + {% endblock messages%} {% url 'members' as url %}

- {% with group_classes="usa-form-editable usa-form-editable--no-border padding-top-0" %} + {% with group_classes="usa-form-editable usa-form-editable--no-border bg-gray-1 padding-top-0" %} {% input_with_errors form.member_permission_admin %} {% endwith %}
- -
-

Basic member permissions

-

Member permissions available for basic-level acccess.

+ +
+

Basic member permissions

+

Member permissions available for basic-level acccess.

Organization domain requests

- {% with group_classes="usa-form-editable usa-form-editable--no-border padding-top-0" %} + {% with group_classes="usa-form-editable usa-form-editable--no-border bg-gray-1 padding-top-0" %} {% input_with_errors form.domain_request_permission_member %} {% endwith %}
- -
- - Cancel - - -
- + +
+ + Cancel + + +
+ +
{% endblock portfolio_content%} diff --git a/src/registrar/templates/portfolio_members.html b/src/registrar/templates/portfolio_members.html index 332d1c16c..720d60e59 100644 --- a/src/registrar/templates/portfolio_members.html +++ b/src/registrar/templates/portfolio_members.html @@ -10,6 +10,11 @@ {% block portfolio_content %} + +{% block messages %} + {% include "includes/form_messages.html" %} +{% endblock messages%} +
diff --git a/src/registrar/templates/portfolio_no_domains.html b/src/registrar/templates/portfolio_no_domains.html index 995f391a2..d0abd9e9a 100644 --- a/src/registrar/templates/portfolio_no_domains.html +++ b/src/registrar/templates/portfolio_no_domains.html @@ -4,8 +4,13 @@ {% block title %} Domains | {% endblock %} + +{% block messages %} + {% include "includes/form_messages.html" %} +{% endblock messages%} + {% block portfolio_content %} -
+

Domains

diff --git a/src/registrar/templates/portfolio_no_requests.html b/src/registrar/templates/portfolio_no_requests.html index a51a034a8..5d443d6f7 100644 --- a/src/registrar/templates/portfolio_no_requests.html +++ b/src/registrar/templates/portfolio_no_requests.html @@ -4,7 +4,13 @@ {% block title %} Domain Requests | {% endblock %} + +{% block messages %} + {% include "includes/form_messages.html" %} +{% endblock messages%} + {% block portfolio_content %} +

Domain requests

@@ -27,4 +33,5 @@ {% endif %}
+
{% endblock %} diff --git a/src/registrar/templates/portfolio_organization.html b/src/registrar/templates/portfolio_organization.html index ce32555a5..55064d902 100644 --- a/src/registrar/templates/portfolio_organization.html +++ b/src/registrar/templates/portfolio_organization.html @@ -18,12 +18,11 @@
- - {% block messages %} - {% include "includes/form_messages.html" %} - {% endblock %} - + {% include "includes/form_errors.html" with form=form %} + {% block messages %} + {% include "includes/form_messages.html" %} + {% endblock messages%}

Organization

diff --git a/src/registrar/templates/portfolio_requests.html b/src/registrar/templates/portfolio_requests.html index 467141077..58fbde10c 100644 --- a/src/registrar/templates/portfolio_requests.html +++ b/src/registrar/templates/portfolio_requests.html @@ -9,9 +9,11 @@ {% endblock %} {% block portfolio_content %} + + {% block messages %} {% include "includes/form_messages.html" %} -{% endblock %} +{% endblock messages%}

Domain requests

@@ -19,7 +21,7 @@ {% if has_edit_request_portfolio_permission %}
-

Domain requests can only be modified by the person who created the request.

+

Domain requests can only be modified by the person who created the request.

diff --git a/src/registrar/templates/portfolio_senior_official.html b/src/registrar/templates/portfolio_senior_official.html index 631db0206..d521c015b 100644 --- a/src/registrar/templates/portfolio_senior_official.html +++ b/src/registrar/templates/portfolio_senior_official.html @@ -6,9 +6,6 @@ {% load static %} {% block portfolio_content %} -{% block messages %} - {% include "includes/form_messages.html" %} -{% endblock %}
diff --git a/src/registrar/templates/profile.html b/src/registrar/templates/profile.html index 7d365d9c1..1d91a2597 100644 --- a/src/registrar/templates/profile.html +++ b/src/registrar/templates/profile.html @@ -11,90 +11,92 @@ Edit your User Profile | {% endblock %} {% block content %} -
-
- {% if messages %} - {% for message in messages %} -
-
- {{ message }} -
-
- {% endfor %} - {% endif %} - {% include "includes/form_errors.html" with form=form %} +
+
+
+ {% if messages %} + {% for message in messages %} +
+
+ {{ message }} +
+
+ {% endfor %} + {% endif %} + {% include "includes/form_errors.html" with form=form %} - {% if show_back_button %} - - -

- {{ profile_back_button_text }} -

- - {% endif %} - - {% if show_confirmation_modal %} - -
-
-
-

- Add contact information -

-
-

- .Gov domain registrants must maintain accurate contact information in the .gov registrar. - Before you can manage your domain, we need you to add your contact information. -

-
-
- +

+
+

+ .Gov domain registrants must maintain accurate contact information in the .gov registrar. + Before you can manage your domain, we need you to add your contact information. +

+
+
+
    +
  • + +
    • +
+
+
+
- - + {% endif %} + + + + {% endblock content %} + + {% block content_bottom %} + {% include "includes/profile_form.html" with form=form %} - {% endif %} - - - -{% endblock content %} - -{% block content_bottom %} - {% include "includes/profile_form.html" with form=form %} - +
{% endblock content_bottom %} diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index af4345a14..8eca0108e 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1034,6 +1034,10 @@ def completed_domain_request( # noqa action_needed_reason=None, portfolio=None, organization_name=None, + sub_organization=None, + requested_suborganization=None, + suborganization_city=None, + suborganization_state_territory=None, ): """A completed domain request.""" if not user: @@ -1098,6 +1102,18 @@ def completed_domain_request( # noqa if portfolio: domain_request_kwargs["portfolio"] = portfolio + if sub_organization: + domain_request_kwargs["sub_organization"] = sub_organization + + if requested_suborganization: + domain_request_kwargs["requested_suborganization"] = requested_suborganization + + if suborganization_city: + domain_request_kwargs["suborganization_city"] = suborganization_city + + if suborganization_state_territory: + domain_request_kwargs["suborganization_state_territory"] = suborganization_state_territory + domain_request, _ = DomainRequest.objects.get_or_create(**domain_request_kwargs) if has_other_contacts: diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a259e5bef..dceb3a79e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2,6 +2,8 @@ from datetime import datetime from django.utils import timezone from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite +from registrar.utility.email import EmailSendingError +from registrar.utility.errors import MissingEmailError from waffle.testutils import override_flag from django_webtest import WebTest # type: ignore from api.tests.common import less_console_noise_decorator @@ -277,6 +279,29 @@ class TestUserPortfolioPermissionAdmin(TestCase): # Should return the forbidden permissions for member role self.assertEqual(member_only_permissions, set(member_forbidden)) + @less_console_noise_decorator + def test_has_change_form_description(self): + """Tests if this model has a model description on the change form view""" + self.client.force_login(self.superuser) + + user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + user=self.superuser, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + response = self.client.get( + "/admin/registrar/userportfoliopermission/{}/change/".format(user_portfolio_permission.pk), + follow=True, + ) + + # Make sure that the page is loaded correctly + self.assertEqual(response.status_code, 200) + + # Test for a description snippet + self.assertContains( + response, + "If you add someone to a portfolio here, it will not trigger an invitation email.", + ) + class TestPortfolioInvitationAdmin(TestCase): """Tests for the PortfolioInvitationAdmin class as super user @@ -432,6 +457,30 @@ class TestPortfolioInvitationAdmin(TestCase): ) self.assertContains(response, "Show more") + @less_console_noise_decorator + def test_has_change_form_description(self): + """Tests if this model has a model description on the change form view""" + self.client.force_login(self.superuser) + + invitation, _ = PortfolioInvitation.objects.get_or_create( + email=self.superuser.email, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + response = self.client.get( + "/admin/registrar/portfolioinvitation/{}/change/".format(invitation.pk), + follow=True, + ) + + # Make sure that the page is loaded correctly + self.assertEqual(response.status_code, 200) + + # Test for a description snippet + self.assertContains( + response, + "If you add someone to a portfolio here, it will trigger an invitation email when you click", + ) + + @less_console_noise_decorator def test_get_filters(self): """Ensures that our filters are displaying correctly""" with less_console_noise(): @@ -456,6 +505,176 @@ class TestPortfolioInvitationAdmin(TestCase): self.assertContains(response, invited_html, count=1) self.assertContains(response, retrieved_html, count=1) + @less_console_noise_decorator + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") # Mock the `messages.warning` call + def test_save_sends_email(self, mock_messages_warning, mock_send_email): + """On save_model, an email is NOT sent if an invitation already exists.""" + self.client.force_login(self.superuser) + + # Create an instance of the admin class + admin_instance = PortfolioInvitationAdmin(PortfolioInvitation, admin_site=None) + + # Create a PortfolioInvitation instance + portfolio_invitation = PortfolioInvitation( + email="james.gordon@gotham.gov", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + # Create a request object + request = self.factory.post("/admin/registrar/PortfolioInvitation/add/") + request.user = self.superuser + + # Call the save_model method + admin_instance.save_model(request, portfolio_invitation, None, None) + + # Assert that send_portfolio_invitation_email is not called + mock_send_email.assert_called() + + # Get the arguments passed to send_portfolio_invitation_email + _, called_kwargs = mock_send_email.call_args + + # Assert the email content + self.assertEqual(called_kwargs["email"], "james.gordon@gotham.gov") + self.assertEqual(called_kwargs["requestor"], self.superuser) + self.assertEqual(called_kwargs["portfolio"], self.portfolio) + + # Assert that a warning message was triggered + mock_messages_warning.assert_called_once_with(request, "james.gordon@gotham.gov has been invited.") + + @less_console_noise_decorator + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.warning") # Mock the `messages.warning` call + def test_save_does_not_send_email_if_requested_user_exists(self, mock_messages_warning, mock_send_email): + """On save_model, an email is NOT sent if an the requested email belongs to an existing user. + It also throws a warning.""" + self.client.force_login(self.superuser) + + # Create an instance of the admin class + admin_instance = PortfolioInvitationAdmin(PortfolioInvitation, admin_site=None) + + # Mock the UserPortfolioPermission query to simulate the invitation already existing + existing_user = create_user() + UserPortfolioPermission.objects.create(user=existing_user, portfolio=self.portfolio) + + # Create a PortfolioInvitation instance + portfolio_invitation = PortfolioInvitation( + email=existing_user.email, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + # Create a request object + request = self.factory.post("/admin/registrar/PortfolioInvitation/add/") + request.user = self.superuser + + # Call the save_model method + admin_instance.save_model(request, portfolio_invitation, None, None) + + # Assert that send_portfolio_invitation_email is not called + mock_send_email.assert_not_called() + + # Assert that a warning message was triggered + mock_messages_warning.assert_called_once_with(request, "User is already a member of this portfolio.") + + @less_console_noise_decorator + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.error") # Mock the `messages.error` call + def test_save_exception_email_sending_error(self, mock_messages_error, mock_send_email): + """Handle EmailSendingError correctly when sending the portfolio invitation fails.""" + self.client.force_login(self.superuser) + + # Mock the email sending function to raise EmailSendingError + mock_send_email.side_effect = EmailSendingError("Email service unavailable") + + # Create an instance of the admin class + admin_instance = PortfolioInvitationAdmin(PortfolioInvitation, admin_site=None) + + # Create a PortfolioInvitation instance + portfolio_invitation = PortfolioInvitation( + email="james.gordon@gotham.gov", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + # Create a request object + request = self.factory.post("/admin/registrar/PortfolioInvitation/add/") + request.user = self.superuser + + # Call the save_model method + admin_instance.save_model(request, portfolio_invitation, None, None) + + # Assert that messages.error was called with the correct message + mock_messages_error.assert_called_once_with( + request, "Could not send email invitation. Portfolio invitation not saved." + ) + + @less_console_noise_decorator + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.error") # Mock the `messages.error` call + def test_save_exception_missing_email_error(self, mock_messages_error, mock_send_email): + """Handle MissingEmailError correctly when no email exists for the requestor.""" + self.client.force_login(self.superuser) + + # Mock the email sending function to raise MissingEmailError + mock_send_email.side_effect = MissingEmailError() + + # Create an instance of the admin class + admin_instance = PortfolioInvitationAdmin(PortfolioInvitation, admin_site=None) + + # Create a PortfolioInvitation instance + portfolio_invitation = PortfolioInvitation( + email="james.gordon@gotham.gov", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + # Create a request object + request = self.factory.post("/admin/registrar/PortfolioInvitation/add/") + request.user = self.superuser + + # Call the save_model method + admin_instance.save_model(request, portfolio_invitation, None, None) + + # Assert that messages.error was called with the correct message + mock_messages_error.assert_called_once_with( + request, + "Can't send invitation email. No email is associated with your user account.", + ) + + @less_console_noise_decorator + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.error") # Mock the `messages.error` call + def test_save_exception_generic_error(self, mock_messages_error, mock_send_email): + """Handle generic exceptions correctly during portfolio invitation.""" + self.client.force_login(self.superuser) + + # Mock the email sending function to raise a generic exception + mock_send_email.side_effect = Exception("Unexpected error") + + # Create an instance of the admin class + admin_instance = PortfolioInvitationAdmin(PortfolioInvitation, admin_site=None) + + # Create a PortfolioInvitation instance + portfolio_invitation = PortfolioInvitation( + email="james.gordon@gotham.gov", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + # Create a request object + request = self.factory.post("/admin/registrar/PortfolioInvitation/add/") + request.user = self.superuser + + # Call the save_model method + admin_instance.save_model(request, portfolio_invitation, None, None) + + # Assert that messages.error was called with the correct message + mock_messages_error.assert_called_once_with( + request, "Could not send email invitation. Portfolio invitation not saved." + ) + class TestHostAdmin(TestCase): """Tests for the HostAdmin class as super user diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 439f4fab0..efb1331df 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -1,5 +1,7 @@ from datetime import datetime +from django.forms import ValidationError from django.utils import timezone +from waffle.testutils import override_flag import re from django.test import RequestFactory, Client, TestCase, override_settings from django.contrib.admin.sites import AdminSite @@ -24,6 +26,7 @@ from registrar.models import ( SeniorOfficial, Portfolio, AllowedEmail, + Suborganization, ) from .common import ( MockSESClient, @@ -82,6 +85,7 @@ class TestDomainRequestAdmin(MockEppLib): Contact.objects.all().delete() Website.objects.all().delete() SeniorOfficial.objects.all().delete() + Suborganization.objects.all().delete() Portfolio.objects.all().delete() self.mock_client.EMAILS_SENT.clear() @@ -91,6 +95,83 @@ class TestDomainRequestAdmin(MockEppLib): User.objects.all().delete() AllowedEmail.objects.all().delete() + @override_flag("organization_feature", active=True) + @less_console_noise_decorator + def test_clean_validates_duplicate_suborganization(self): + """Tests that clean() prevents duplicate suborganization names within the same portfolio""" + # Create a portfolio and existing suborganization + portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser) + + # Create an existing suborganization + Suborganization.objects.create(name="Existing Suborg", portfolio=portfolio) + + # Create a domain request trying to use the same suborganization name + # (intentionally lowercase) + domain_request = completed_domain_request( + name="test1234.gov", + portfolio=portfolio, + requested_suborganization="existing suborg", + suborganization_city="Rome", + suborganization_state_territory=DomainRequest.StateTerritoryChoices.OHIO, + ) + + # Assert that the validation error is raised + with self.assertRaises(ValidationError) as err: + domain_request.clean() + + self.assertIn("This suborganization already exists", str(err.exception)) + + # Test that a different name is allowed. Should not raise a error. + domain_request.requested_suborganization = "New Suborg" + domain_request.clean() + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_clean_validates_partial_suborganization_fields(self): + """Tests that clean() enforces all-or-nothing rule for suborganization fields""" + portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser) + + # Create domain request with only city filled out + domain_request = completed_domain_request( + name="test1234.gov", + portfolio=portfolio, + suborganization_city="Test City", + ) + + # Assert validation error is raised with correct missing fields + with self.assertRaises(ValidationError) as err: + domain_request.clean() + + error_dict = err.exception.error_dict + expected_missing = ["requested_suborganization", "suborganization_state_territory"] + + # Verify correct fields are flagged as required + self.assertEqual(sorted(error_dict.keys()), sorted(expected_missing)) + + # Verify error message + for field in expected_missing: + self.assertEqual( + str(error_dict[field][0].message), "This field is required when creating a new suborganization." + ) + + # When all data is passed in, this should validate correctly + domain_request.requested_suborganization = "Complete Suborg" + domain_request.suborganization_state_territory = DomainRequest.StateTerritoryChoices.OHIO + # Assert that no ValidationError is raised + try: + domain_request.clean() + except ValidationError as e: + self.fail(f"ValidationError was raised unexpectedly: {e}") + + # Also ensure that no validation error is raised if nothing is passed in at all + domain_request.suborganization_city = None + domain_request.requested_suborganization = None + domain_request.suborganization_state_territory = None + try: + domain_request.clean() + except ValidationError as e: + self.fail(f"ValidationError was raised unexpectedly: {e}") + @less_console_noise_decorator def test_domain_request_senior_official_is_alphabetically_sorted(self): """Tests if the senior offical dropdown is alphanetically sorted in the django admin display""" diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 12d9af8ac..a2960deac 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -18,7 +18,17 @@ from registrar.forms.domain_request_wizard import ( AboutYourOrganizationForm, ) from registrar.forms.domain import ContactForm -from registrar.tests.common import MockEppLib +from registrar.forms.portfolio import ( + PortfolioInvitedMemberForm, + PortfolioMemberForm, + PortfolioNewMemberForm, +) +from registrar.models.portfolio import Portfolio +from registrar.models.portfolio_invitation import PortfolioInvitation +from registrar.models.user import User +from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from registrar.tests.common import MockEppLib, create_user from django.contrib.auth import get_user_model @@ -408,3 +418,196 @@ class TestContactForm(TestCase): def test_contact_form_email_invalid2(self): form = ContactForm(data={"email": "@"}) self.assertEqual(form.errors["email"], ["Enter a valid email address."]) + + +class TestBasePortfolioMemberForms(TestCase): + """We test on the child forms instead of BasePortfolioMemberForm because the base form + is a model form with no model bound.""" + + def setUp(self): + super().setUp() + self.user = create_user() + self.portfolio, _ = Portfolio.objects.get_or_create( + creator_id=self.user.id, organization_name="Hotel California" + ) + + def tearDown(self): + super().tearDown() + Portfolio.objects.all().delete() + UserPortfolioPermission.objects.all().delete() + PortfolioInvitation.objects.all().delete() + User.objects.all().delete() + + def _assert_form_is_valid(self, form_class, data, instance=None): + if instance is not None: + form = form_class(data=data, instance=instance) + else: + print("no instance") + form = form_class(data=data) + self.assertTrue(form.is_valid(), f"Form {form_class.__name__} failed validation with data: {data}") + return form + + def _assert_form_has_error(self, form_class, data, field_name): + form = form_class(data=data) + self.assertFalse(form.is_valid()) + self.assertIn(field_name, form.errors) + + def _assert_initial_data(self, form_class, instance, expected_initial_data): + """Helper to check if the instance data is correctly mapped to the initial form values.""" + form = form_class(instance=instance) + for field, expected_value in expected_initial_data.items(): + self.assertEqual(form.initial[field], expected_value) + + def _assert_permission_mapping(self, form_class, data, expected_permissions): + """Helper to check if permissions are correctly handled and mapped.""" + form = self._assert_form_is_valid(form_class, data) + cleaned_data = form.cleaned_data + for permission in expected_permissions: + self.assertIn(permission, cleaned_data["additional_permissions"]) + + def test_required_field_for_admin(self): + """Test that required fields are validated for an admin role.""" + data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value, + "domain_request_permission_admin": "", # Simulate missing field + "member_permission_admin": "", # Simulate missing field + } + + # Check required fields for all forms + self._assert_form_has_error(PortfolioMemberForm, data, "domain_request_permission_admin") + self._assert_form_has_error(PortfolioMemberForm, data, "member_permission_admin") + + self._assert_form_has_error(PortfolioInvitedMemberForm, data, "domain_request_permission_admin") + self._assert_form_has_error(PortfolioInvitedMemberForm, data, "member_permission_admin") + + self._assert_form_has_error(PortfolioNewMemberForm, data, "domain_request_permission_admin") + self._assert_form_has_error(PortfolioNewMemberForm, data, "member_permission_admin") + + def test_required_field_for_member(self): + """Test that required fields are validated for a member role.""" + data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": "", # Simulate missing field + } + + # Check required fields for all forms + self._assert_form_has_error(PortfolioMemberForm, data, "domain_request_permission_member") + self._assert_form_has_error(PortfolioInvitedMemberForm, data, "domain_request_permission_member") + self._assert_form_has_error(PortfolioNewMemberForm, data, "domain_request_permission_member") + + def test_clean_validates_required_fields_for_role(self): + """Test that the `clean` method validates the correct fields for each role. + + For PortfolioMemberForm and PortfolioInvitedMemberForm, we pass an object as the instance to the form. + For UserPortfolioPermissionChoices, we add a portfolio and an email to the POST data. + + These things are handled in the views.""" + + user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + portfolio=self.portfolio, user=self.user + ) + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create(portfolio=self.portfolio, email="hi@ho") + + data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value, + "domain_request_permission_admin": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "member_permission_admin": UserPortfolioPermissionChoices.EDIT_MEMBERS.value, + } + + # Check form validity for all forms + form = self._assert_form_is_valid(PortfolioMemberForm, data, user_portfolio_permission) + cleaned_data = form.cleaned_data + self.assertEqual(cleaned_data["roles"], [UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value]) + self.assertEqual(cleaned_data["additional_permissions"], [UserPortfolioPermissionChoices.EDIT_MEMBERS]) + + form = self._assert_form_is_valid(PortfolioInvitedMemberForm, data, portfolio_invitation) + cleaned_data = form.cleaned_data + self.assertEqual(cleaned_data["roles"], [UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value]) + self.assertEqual(cleaned_data["additional_permissions"], [UserPortfolioPermissionChoices.EDIT_MEMBERS]) + + data = { + "email": "hi@ho.com", + "portfolio": self.portfolio.id, + "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value, + "domain_request_permission_admin": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "member_permission_admin": UserPortfolioPermissionChoices.EDIT_MEMBERS.value, + } + + form = self._assert_form_is_valid(PortfolioNewMemberForm, data) + cleaned_data = form.cleaned_data + self.assertEqual(cleaned_data["roles"], [UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value]) + self.assertEqual(cleaned_data["additional_permissions"], [UserPortfolioPermissionChoices.EDIT_MEMBERS]) + + def test_clean_member_permission_edgecase(self): + """Test that the clean method correctly handles the special "no_access" value for members. + We'll need to add a portfolio, which in the app is handled by the view post.""" + + user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + portfolio=self.portfolio, user=self.user + ) + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create(portfolio=self.portfolio, email="hi@ho") + + data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": "no_access", # Simulate no access permission + } + + form = self._assert_form_is_valid(PortfolioMemberForm, data, user_portfolio_permission) + cleaned_data = form.cleaned_data + self.assertEqual(cleaned_data["domain_request_permission_member"], None) + + form = self._assert_form_is_valid(PortfolioInvitedMemberForm, data, portfolio_invitation) + cleaned_data = form.cleaned_data + self.assertEqual(cleaned_data["domain_request_permission_member"], None) + + def test_map_instance_to_initial_admin_role(self): + """Test that instance data is correctly mapped to the initial form values for an admin role.""" + user_portfolio_permission = UserPortfolioPermission( + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[UserPortfolioPermissionChoices.VIEW_MEMBERS], + ) + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create( + portfolio=self.portfolio, + email="hi@ho", + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[UserPortfolioPermissionChoices.VIEW_MEMBERS], + ) + + expected_initial_data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + "domain_request_permission_admin": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + "member_permission_admin": UserPortfolioPermissionChoices.VIEW_MEMBERS, + } + self._assert_initial_data(PortfolioMemberForm, user_portfolio_permission, expected_initial_data) + self._assert_initial_data(PortfolioInvitedMemberForm, portfolio_invitation, expected_initial_data) + + def test_map_instance_to_initial_member_role(self): + """Test that instance data is correctly mapped to the initial form values for a member role.""" + user_portfolio_permission = UserPortfolioPermission( + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS], + ) + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create( + portfolio=self.portfolio, + email="hi@ho", + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS], + ) + expected_initial_data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + } + self._assert_initial_data(PortfolioMemberForm, user_portfolio_permission, expected_initial_data) + self._assert_initial_data(PortfolioInvitedMemberForm, portfolio_invitation, expected_initial_data) + + def test_invalid_data_for_admin(self): + """Test invalid form submission for an admin role with missing permissions.""" + data = { + "email": "hi@ho.com", + "portfolio": self.portfolio.id, + "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value, + "domain_request_permission_admin": "", # Missing field + "member_permission_admin": "", # Missing field + } + self._assert_form_has_error(PortfolioMemberForm, data, "domain_request_permission_admin") + self._assert_form_has_error(PortfolioInvitedMemberForm, data, "member_permission_admin") diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index da474224c..983a12b3c 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -15,6 +15,7 @@ from registrar.models import ( FederalAgency, AllowedEmail, Portfolio, + Suborganization, ) import boto3_mocking @@ -23,6 +24,8 @@ from registrar.utility.errors import FSMDomainRequestError from .common import ( MockSESClient, + create_user, + create_superuser, less_console_noise, completed_domain_request, set_domain_request_investigators, @@ -1070,3 +1073,142 @@ class TestDomainRequest(TestCase): ) self.assertEqual(domain_request2.generic_org_type, domain_request2.converted_generic_org_type) self.assertEqual(domain_request2.federal_agency, domain_request2.converted_federal_agency) + + +class TestDomainRequestSuborganization(TestCase): + """Tests for the suborganization fields on domain requests""" + + def setUp(self): + super().setUp() + self.user = create_user() + self.superuser = create_superuser() + + def tearDown(self): + super().tearDown() + DomainInformation.objects.all().delete() + DomainRequest.objects.all().delete() + Domain.objects.all().delete() + Suborganization.objects.all().delete() + Portfolio.objects.all().delete() + + @less_console_noise_decorator + def test_approve_creates_requested_suborganization(self): + """Test that approving a domain request with a requested suborganization creates it""" + portfolio = Portfolio.objects.create(organization_name="Test Org", creator=self.user) + + domain_request = completed_domain_request( + name="test.gov", + portfolio=portfolio, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + requested_suborganization="Boom", + suborganization_city="Explody town", + suborganization_state_territory=DomainRequest.StateTerritoryChoices.OHIO, + ) + domain_request.investigator = self.superuser + domain_request.save() + + domain_request.approve() + + created_suborg = Suborganization.objects.filter( + name="Boom", + city="Explody town", + state_territory=DomainRequest.StateTerritoryChoices.OHIO, + portfolio=portfolio, + ).first() + + self.assertIsNotNone(created_suborg) + self.assertEqual(domain_request.sub_organization, created_suborg) + + @less_console_noise_decorator + def test_approve_without_requested_suborganization_makes_no_changes(self): + """Test that approving without a requested suborganization doesn't create one""" + portfolio = Portfolio.objects.create(organization_name="Test Org", creator=self.user) + + domain_request = completed_domain_request( + name="test.gov", + portfolio=portfolio, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + ) + domain_request.investigator = self.superuser + domain_request.save() + + initial_suborg_count = Suborganization.objects.count() + domain_request.approve() + + self.assertEqual(Suborganization.objects.count(), initial_suborg_count) + self.assertIsNone(domain_request.sub_organization) + + @less_console_noise_decorator + def test_approve_with_existing_suborganization_makes_no_changes(self): + """Test that approving with an existing suborganization doesn't create a new one""" + portfolio = Portfolio.objects.create(organization_name="Test Org", creator=self.user) + existing_suborg = Suborganization.objects.create(name="Existing Division", portfolio=portfolio) + + domain_request = completed_domain_request( + name="test.gov", + portfolio=portfolio, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + sub_organization=existing_suborg, + ) + domain_request.investigator = self.superuser + domain_request.save() + + initial_suborg_count = Suborganization.objects.count() + domain_request.approve() + + self.assertEqual(Suborganization.objects.count(), initial_suborg_count) + self.assertEqual(domain_request.sub_organization, existing_suborg) + + @less_console_noise_decorator + def test_cleanup_dangling_suborg_with_single_reference(self): + """Test that a suborganization is deleted when it's only referenced once""" + portfolio = Portfolio.objects.create(organization_name="Test Org", creator=self.user) + suborg = Suborganization.objects.create(name="Test Division", portfolio=portfolio) + + domain_request = completed_domain_request( + name="test.gov", + portfolio=portfolio, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + sub_organization=suborg, + ) + domain_request.approve() + + # set it back to in review + domain_request.in_review() + domain_request.refresh_from_db() + + # Verify the suborganization was deleted + self.assertFalse(Suborganization.objects.filter(id=suborg.id).exists()) + self.assertIsNone(domain_request.sub_organization) + + @less_console_noise_decorator + def test_cleanup_dangling_suborg_with_multiple_references(self): + """Test that a suborganization is preserved when it has multiple references""" + portfolio = Portfolio.objects.create(organization_name="Test Org", creator=self.user) + suborg = Suborganization.objects.create(name="Test Division", portfolio=portfolio) + + # Create two domain requests using the same suborganization + domain_request1 = completed_domain_request( + name="test1.gov", + portfolio=portfolio, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + sub_organization=suborg, + ) + domain_request2 = completed_domain_request( + name="test2.gov", + portfolio=portfolio, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + sub_organization=suborg, + ) + + domain_request1.approve() + domain_request2.approve() + + # set one back to in review + domain_request1.in_review() + domain_request1.refresh_from_db() + + # Verify the suborganization still exists + self.assertTrue(Suborganization.objects.filter(id=suborg.id).exists()) + self.assertEqual(domain_request1.sub_organization, suborg) + self.assertEqual(domain_request2.sub_organization, suborg) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index ba237e1e7..aedfc41c2 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -4,6 +4,8 @@ from unittest.mock import MagicMock, ANY, patch from django.conf import settings from django.urls import reverse from django.contrib.auth import get_user_model +from registrar.models.portfolio_invitation import PortfolioInvitation +from registrar.utility.email import EmailSendingError from waffle.testutils import override_flag from api.tests.common import less_console_noise_decorator from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices @@ -548,6 +550,7 @@ class TestDomainManagers(TestDomainOverview): self.portfolio = Portfolio.objects.create(creator=self.user, organization_name="Ice Cream") # Add the portfolio to the domain_information object self.domain_information.portfolio = self.portfolio + self.domain_information.save() # Add portfolio perms to the user object self.portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( user=self.user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] @@ -560,6 +563,7 @@ class TestDomainManagers(TestDomainOverview): def tearDown(self): """Ensure that the user has its original permissions""" + PortfolioInvitation.objects.all().delete() super().tearDown() @less_console_noise_decorator @@ -592,7 +596,7 @@ class TestDomainManagers(TestDomainOverview): @less_console_noise_decorator def test_domain_user_add_form(self): """Adding an existing user works.""" - other_user, _ = get_user_model().objects.get_or_create(email="mayor@igorville.gov") + get_user_model().objects.get_or_create(email="mayor@igorville.gov") add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -615,6 +619,148 @@ class TestDomainManagers(TestDomainOverview): success_page = success_result.follow() self.assertContains(success_page, "mayor@igorville.gov") + @boto3_mocking.patching + @override_flag("organization_feature", active=True) + @less_console_noise_decorator + @patch("registrar.views.domain.send_portfolio_invitation_email") + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_user_add_form_sends_portfolio_invitation(self, mock_send_domain_email, mock_send_portfolio_email): + """Adding an existing user works and sends portfolio invitation when + user is not member of portfolio.""" + get_user_model().objects.get_or_create(email="mayor@igorville.gov") + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + + add_page.form["email"] = "mayor@igorville.gov" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + success_result = add_page.form.submit() + + self.assertEqual(success_result.status_code, 302) + self.assertEqual( + success_result["Location"], + reverse("domain-users", kwargs={"pk": self.domain.id}), + ) + + # Verify that the invitation emails were sent + mock_send_portfolio_email.assert_called_once_with( + email="mayor@igorville.gov", requestor=self.user, portfolio=self.portfolio + ) + mock_send_domain_email.assert_called_once() + call_args = mock_send_domain_email.call_args.kwargs + self.assertEqual(call_args["email"], "mayor@igorville.gov") + self.assertEqual(call_args["requestor"], self.user) + self.assertEqual(call_args["domain"], self.domain) + self.assertIsNone(call_args.get("is_member_of_different_org")) + + # Assert that the PortfolioInvitation is created + portfolio_invitation = PortfolioInvitation.objects.filter( + email="mayor@igorville.gov", portfolio=self.portfolio + ).first() + self.assertIsNotNone(portfolio_invitation, "Portfolio invitation should be created.") + self.assertEqual(portfolio_invitation.email, "mayor@igorville.gov") + self.assertEqual(portfolio_invitation.portfolio, self.portfolio) + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + success_page = success_result.follow() + self.assertContains(success_page, "mayor@igorville.gov") + + @boto3_mocking.patching + @override_flag("organization_feature", active=True) + @less_console_noise_decorator + @patch("registrar.views.domain.send_portfolio_invitation_email") + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_user_add_form_doesnt_send_portfolio_invitation_if_already_member( + self, mock_send_domain_email, mock_send_portfolio_email + ): + """Adding an existing user works and sends portfolio invitation when + user is not member of portfolio.""" + other_user, _ = get_user_model().objects.get_or_create(email="mayor@igorville.gov") + UserPortfolioPermission.objects.get_or_create( + user=other_user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + + add_page.form["email"] = "mayor@igorville.gov" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + success_result = add_page.form.submit() + + self.assertEqual(success_result.status_code, 302) + self.assertEqual( + success_result["Location"], + reverse("domain-users", kwargs={"pk": self.domain.id}), + ) + + # Verify that the invitation emails were sent + mock_send_portfolio_email.assert_not_called() + mock_send_domain_email.assert_called_once() + call_args = mock_send_domain_email.call_args.kwargs + self.assertEqual(call_args["email"], "mayor@igorville.gov") + self.assertEqual(call_args["requestor"], self.user) + self.assertEqual(call_args["domain"], self.domain) + self.assertIsNone(call_args.get("is_member_of_different_org")) + + # Assert that no PortfolioInvitation is created + portfolio_invitation_exists = PortfolioInvitation.objects.filter( + email="mayor@igorville.gov", portfolio=self.portfolio + ).exists() + self.assertFalse( + portfolio_invitation_exists, "Portfolio invitation should not be created when the user is already a member." + ) + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + success_page = success_result.follow() + self.assertContains(success_page, "mayor@igorville.gov") + + @boto3_mocking.patching + @override_flag("organization_feature", active=True) + @less_console_noise_decorator + @patch("registrar.views.domain.send_portfolio_invitation_email") + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_user_add_form_sends_portfolio_invitation_raises_email_sending_error( + self, mock_send_domain_email, mock_send_portfolio_email + ): + """Adding an existing user works and attempts to send portfolio invitation when + user is not member of portfolio and send raises an error.""" + mock_send_portfolio_email.side_effect = EmailSendingError("Failed to send email.") + get_user_model().objects.get_or_create(email="mayor@igorville.gov") + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + + add_page.form["email"] = "mayor@igorville.gov" + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + success_result = add_page.form.submit() + + self.assertEqual(success_result.status_code, 302) + self.assertEqual( + success_result["Location"], + reverse("domain-users", kwargs={"pk": self.domain.id}), + ) + + # Verify that the invitation emails were sent + mock_send_portfolio_email.assert_called_once_with( + email="mayor@igorville.gov", requestor=self.user, portfolio=self.portfolio + ) + mock_send_domain_email.assert_not_called() + + # Assert that no PortfolioInvitation is created + portfolio_invitation_exists = PortfolioInvitation.objects.filter( + email="mayor@igorville.gov", portfolio=self.portfolio + ).exists() + self.assertFalse( + portfolio_invitation_exists, "Portfolio invitation should not be created when email fails to send." + ) + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + success_page = success_result.follow() + self.assertContains(success_page, "Could not send email invitation.") + @boto3_mocking.patching @less_console_noise_decorator def test_domain_invitation_created(self): @@ -827,39 +973,20 @@ class TestDomainManagers(TestDomainOverview): self.assertNotIn("Last", email_content) self.assertNotIn("First Last", email_content) - @boto3_mocking.patching @less_console_noise_decorator - def test_domain_invitation_email_displays_error_non_existent(self): - """Inviting a non existent user sends them an email, with email as the name.""" - # make sure there is no user with this email - email_address = "mayor@igorville.gov" - User.objects.filter(email=email_address).delete() - - # Give the user who is sending the email an invalid email address - self.user.email = "" - self.user.save() - + def test_domain_invitation_email_validation_blocks_bad_email(self): + """Inviting a bad email blocks at validation.""" + email_address = "mayor" self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) - mock_client = MagicMock() - mock_error_message = MagicMock() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with patch("django.contrib.messages.error") as mock_error_message: - add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - add_page.form["email"] = email_address - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - add_page.form.submit().follow() + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + add_page.form["email"] = email_address + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + response = add_page.form.submit() - expected_message_content = "Can't send invitation email. No email is associated with your account." + self.assertContains(response, "Enter an email address in the required format, like name@example.com.") - # Grab the message content - returned_error_message = mock_error_message.call_args[0][1] - - # Check that the message content is what we expect - self.assertEqual(expected_message_content, returned_error_message) - - @boto3_mocking.patching @less_console_noise_decorator def test_domain_invitation_email_displays_error(self): """When the requesting user has no email, an error is displayed""" @@ -870,28 +997,25 @@ class TestDomainManagers(TestDomainOverview): # Give the user who is sending the email an invalid email address self.user.email = "" + self.user.is_staff = False self.user.save() self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) - mock_client = MagicMock() + with patch("django.contrib.messages.error") as mock_error: + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + add_page.form["email"] = email_address + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + add_page.form.submit() - mock_error_message = MagicMock() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with patch("django.contrib.messages.error") as mock_error_message: - add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - add_page.form["email"] = email_address - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - add_page.form.submit().follow() + expected_message_content = "Can't send invitation email. No email is associated with your user account." - expected_message_content = "Can't send invitation email. No email is associated with your account." - - # Grab the message content - returned_error_message = mock_error_message.call_args[0][1] - - # Check that the message content is what we expect - self.assertEqual(expected_message_content, returned_error_message) + # Assert that the error message was called with the correct argument + mock_error.assert_called_once_with( + ANY, + expected_message_content, + ) @less_console_noise_decorator def test_domain_invitation_cancel(self): diff --git a/src/registrar/tests/test_views_member_domains_json.py b/src/registrar/tests/test_views_member_domains_json.py index c9f1e38cc..091ad6151 100644 --- a/src/registrar/tests/test_views_member_domains_json.py +++ b/src/registrar/tests/test_views_member_domains_json.py @@ -94,6 +94,12 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): DomainInvitation.objects.create( email=cls.invited_member_email, domain=cls.domain2, status=DomainInvitation.DomainInvitationStatus.INVITED ) + DomainInvitation.objects.create( + email=cls.invited_member_email, domain=cls.domain3, status=DomainInvitation.DomainInvitationStatus.CANCELED + ) + DomainInvitation.objects.create( + email=cls.invited_member_email, domain=cls.domain4, status=DomainInvitation.DomainInvitationStatus.RETRIEVED + ) @classmethod def tearDownClass(cls): @@ -138,7 +144,8 @@ class GetPortfolioMemberDomainsJsonTest(TestWithUser, WebTest): @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) def test_get_portfolio_invitedmember_domains_json_authenticated(self): - """Test that portfolio invitedmember's domains are returned properly for an authenticated user.""" + """Test that portfolio invitedmember's domains are returned properly for an authenticated user. + CANCELED and RETRIEVED invites should be ignored.""" response = self.app.get( reverse("get_member_domains_json"), params={"portfolio": self.portfolio.id, "email": self.invited_member_email, "member_only": "true"}, diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py index d7b9f3a9f..8082a1a30 100644 --- a/src/registrar/tests/test_views_members_json.py +++ b/src/registrar/tests/test_views_members_json.py @@ -157,7 +157,7 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): @override_flag("organization_members", active=True) def test_get_portfolio_invited_json_authenticated(self): """Test that portfolio invitees are returned properly for an authenticated user.""" - """Also tests that reposnse is 200 when no domains""" + """Also tests that response is 200 when no domains""" UserPortfolioPermission.objects.create( user=self.user, portfolio=self.portfolio, @@ -258,13 +258,14 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): role=UserDomainRole.Roles.MANAGER, ) - # create domain for which user is manager and domain not in portfolio + # create another domain in the portfolio domain2 = Domain.objects.create( - name="somedomain2.com", + name="thissecondpermtestsmultipleperms@lets.notbreak", ) DomainInformation.objects.create( creator=self.user, domain=domain2, + portfolio=self.portfolio, ) UserDomainRole.objects.create( user=self.user, @@ -272,6 +273,20 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): role=UserDomainRole.Roles.MANAGER, ) + # create domain for which user is manager and domain not in portfolio + domain3 = Domain.objects.create( + name="somedomain3.com", + ) + DomainInformation.objects.create( + creator=self.user, + domain=domain3, + ) + UserDomainRole.objects.create( + user=self.user, + domain=domain3, + role=UserDomainRole.Roles.MANAGER, + ) + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) self.assertEqual(response.status_code, 200) data = response.json @@ -279,7 +294,8 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): # Check if the domain appears in the response JSON and that domain2 does not domain_names = [domain_name for member in data["members"] for domain_name in member.get("domain_names", [])] self.assertIn("somedomain1.com", domain_names) - self.assertNotIn("somedomain2.com", domain_names) + self.assertIn("thissecondpermtestsmultipleperms@lets.notbreak", domain_names) + self.assertNotIn("somedomain3.com", domain_names) @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -318,19 +334,33 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): domain=domain, ) - # create a domain not in the portfolio + # create another domain in the portfolio domain2 = Domain.objects.create( - name="somedomain2.com", + name="thissecondinvitetestsasubqueryinjson@lets.notbreak", ) DomainInformation.objects.create( creator=self.user, domain=domain2, + portfolio=self.portfolio, ) DomainInvitation.objects.create( email=self.email6, domain=domain2, ) + # create a domain not in the portfolio + domain3 = Domain.objects.create( + name="somedomain3.com", + ) + DomainInformation.objects.create( + creator=self.user, + domain=domain3, + ) + DomainInvitation.objects.create( + email=self.email6, + domain=domain3, + ) + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) self.assertEqual(response.status_code, 200) data = response.json @@ -338,7 +368,8 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): # Check if the domain appears in the response JSON and domain2 does not domain_names = [domain_name for member in data["members"] for domain_name in member.get("domain_names", [])] self.assertIn("somedomain1.com", domain_names) - self.assertNotIn("somedomain2.com", domain_names) + self.assertIn("thissecondinvitetestsasubqueryinjson@lets.notbreak", domain_names) + self.assertNotIn("somedomain3.com", domain_names) @less_console_noise_decorator @override_flag("organization_feature", active=True) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 01383ae77..9bc97874d 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -14,17 +14,21 @@ from registrar.models import ( Suborganization, AllowedEmail, ) +from registrar.models.domain_invitation import DomainInvitation from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user_group import UserGroup from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.tests.test_views import TestWithUser +from registrar.utility.email import EmailSendingError +from registrar.utility.errors import MissingEmailError from .common import MockSESClient, completed_domain_request, create_test_user, create_user from waffle.testutils import override_flag from django.contrib.sessions.middleware import SessionMiddleware import boto3_mocking # type: ignore from django.test import Client import logging +import json logger = logging.getLogger(__name__) @@ -1927,7 +1931,7 @@ class TestPortfolioMemberDomainsView(TestWithUser, WebTest): cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Test Portfolio") # Assign permissions to the user making requests - UserPortfolioPermission.objects.create( + cls.portfolio_permission = UserPortfolioPermission.objects.create( user=cls.user, portfolio=cls.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], @@ -2106,11 +2110,22 @@ class TestPortfolioMemberDomainsEditView(TestPortfolioMemberDomainsView): @classmethod def setUpClass(cls): super().setUpClass() + cls.url = reverse("member-domains-edit", kwargs={"pk": cls.portfolio_permission.pk}) @classmethod def tearDownClass(cls): super().tearDownClass() + def setUp(self): + super().setUp() + names = ["1.gov", "2.gov", "3.gov"] + Domain.objects.bulk_create([Domain(name=name) for name in names]) + + def tearDown(self): + super().tearDown() + UserDomainRole.objects.all().delete() + Domain.objects.all().delete() + @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @@ -2162,16 +2177,140 @@ class TestPortfolioMemberDomainsEditView(TestPortfolioMemberDomainsView): # Make sure the response is not found self.assertEqual(response.status_code, 404) + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_valid_added_domains(self): + """Test that domains can be successfully added.""" + self.client.force_login(self.user) + + data = { + "added_domains": json.dumps([1, 2, 3]), # Mock domain IDs + } + response = self.client.post(self.url, data) + + # Check that the UserDomainRole objects were created + self.assertEqual(UserDomainRole.objects.filter(user=self.user, role=UserDomainRole.Roles.MANAGER).count(), 3) + + # Check for a success message and a redirect + self.assertRedirects(response, reverse("member-domains", kwargs={"pk": self.portfolio_permission.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual(str(messages[0]), "The domain assignment changes have been saved.") + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_valid_removed_domains(self): + """Test that domains can be successfully removed.""" + self.client.force_login(self.user) + + # Create some UserDomainRole objects + domains = [1, 2, 3] + UserDomainRole.objects.bulk_create([UserDomainRole(domain_id=domain, user=self.user) for domain in domains]) + + data = { + "removed_domains": json.dumps([1, 2]), + } + response = self.client.post(self.url, data) + + # Check that the UserDomainRole objects were deleted + self.assertEqual(UserDomainRole.objects.filter(user=self.user).count(), 1) + self.assertEqual(UserDomainRole.objects.filter(domain_id=3, user=self.user).count(), 1) + + # Check for a success message and a redirect + self.assertRedirects(response, reverse("member-domains", kwargs={"pk": self.portfolio_permission.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual(str(messages[0]), "The domain assignment changes have been saved.") + + UserDomainRole.objects.all().delete() + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_invalid_added_domains_data(self): + """Test that an error is returned for invalid added domains data.""" + self.client.force_login(self.user) + + data = { + "added_domains": "json-statham", + } + response = self.client.post(self.url, data) + + # Check that no UserDomainRole objects were created + self.assertEqual(UserDomainRole.objects.filter(user=self.user).count(), 0) + + # Check for an error message and a redirect + self.assertRedirects(response, reverse("member-domains", kwargs={"pk": self.portfolio_permission.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual( + str(messages[0]), "Invalid data for added domains. If the issue persists, please contact help@get.gov." + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_invalid_removed_domains_data(self): + """Test that an error is returned for invalid removed domains data.""" + self.client.force_login(self.user) + + data = { + "removed_domains": "not-a-json", + } + response = self.client.post(self.url, data) + + # Check that no UserDomainRole objects were deleted + self.assertEqual(UserDomainRole.objects.filter(user=self.user).count(), 0) + + # Check for an error message and a redirect + self.assertRedirects(response, reverse("member-domains", kwargs={"pk": self.portfolio_permission.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual( + str(messages[0]), "Invalid data for removed domains. If the issue persists, please contact help@get.gov." + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_no_changes(self): + """Test that no changes message is displayed when no changes are made.""" + self.client.force_login(self.user) + + response = self.client.post(self.url, {}) + + # Check that no UserDomainRole objects were created or deleted + self.assertEqual(UserDomainRole.objects.filter(user=self.user).count(), 0) + + # Check for an info message and a redirect + self.assertRedirects(response, reverse("member-domains", kwargs={"pk": self.portfolio_permission.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual(str(messages[0]), "No changes detected.") + class TestPortfolioInvitedMemberEditDomainsView(TestPortfolioInvitedMemberDomainsView): @classmethod def setUpClass(cls): super().setUpClass() + cls.url = reverse("invitedmember-domains-edit", kwargs={"pk": cls.invitation.pk}) @classmethod def tearDownClass(cls): super().tearDownClass() + def setUp(self): + super().setUp() + names = ["1.gov", "2.gov", "3.gov"] + Domain.objects.bulk_create([Domain(name=name) for name in names]) + + def tearDown(self): + super().tearDown() + Domain.objects.all().delete() + DomainInvitation.objects.all().delete() + @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @@ -2222,6 +2361,175 @@ class TestPortfolioInvitedMemberEditDomainsView(TestPortfolioInvitedMemberDomain # Make sure the response is not found self.assertEqual(response.status_code, 404) + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_valid_added_domains(self): + """Test adding new domains successfully.""" + self.client.force_login(self.user) + + data = { + "added_domains": json.dumps([1, 2, 3]), # Mock domain IDs + } + response = self.client.post(self.url, data) + + # Check that the DomainInvitation objects were created + self.assertEqual( + DomainInvitation.objects.filter( + email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + ).count(), + 3, + ) + + # Check for a success message and a redirect + self.assertRedirects(response, reverse("invitedmember-domains", kwargs={"pk": self.invitation.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual(str(messages[0]), "The domain assignment changes have been saved.") + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_existing_and_new_added_domains(self): + """Test updating existing and adding new invitations.""" + self.client.force_login(self.user) + + # Create existing invitations + DomainInvitation.objects.bulk_create( + [ + DomainInvitation( + domain_id=1, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.CANCELED + ), + DomainInvitation( + domain_id=2, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + ), + ] + ) + + data = { + "added_domains": json.dumps([1, 2, 3]), + } + response = self.client.post(self.url, data) + + # Check that status for domain_id=1 was updated to INVITED + self.assertEqual( + DomainInvitation.objects.get(domain_id=1, email="invited@example.com").status, + DomainInvitation.DomainInvitationStatus.INVITED, + ) + + # Check that domain_id=3 was created as INVITED + self.assertTrue( + DomainInvitation.objects.filter( + domain_id=3, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + ).exists() + ) + + # Check for a success message and a redirect + self.assertRedirects(response, reverse("invitedmember-domains", kwargs={"pk": self.invitation.pk})) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_valid_removed_domains(self): + """Test removing domains successfully.""" + self.client.force_login(self.user) + + # Create existing invitations + DomainInvitation.objects.bulk_create( + [ + DomainInvitation( + domain_id=1, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + ), + DomainInvitation( + domain_id=2, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + ), + ] + ) + + data = { + "removed_domains": json.dumps([1]), + } + response = self.client.post(self.url, data) + + # Check that the status for domain_id=1 was updated to CANCELED + self.assertEqual( + DomainInvitation.objects.get(domain_id=1, email="invited@example.com").status, + DomainInvitation.DomainInvitationStatus.CANCELED, + ) + + # Check that domain_id=2 remains INVITED + self.assertEqual( + DomainInvitation.objects.get(domain_id=2, email="invited@example.com").status, + DomainInvitation.DomainInvitationStatus.INVITED, + ) + + # Check for a success message and a redirect + self.assertRedirects(response, reverse("invitedmember-domains", kwargs={"pk": self.invitation.pk})) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_invalid_added_domains_data(self): + """Test handling of invalid JSON for added domains.""" + self.client.force_login(self.user) + + data = { + "added_domains": "not-a-json", + } + response = self.client.post(self.url, data) + + # Check that no DomainInvitation objects were created + self.assertEqual(DomainInvitation.objects.count(), 0) + + # Check for an error message and a redirect + self.assertRedirects(response, reverse("invitedmember-domains", kwargs={"pk": self.invitation.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual( + str(messages[0]), "Invalid data for added domains. If the issue persists, please contact help@get.gov." + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_invalid_removed_domains_data(self): + """Test handling of invalid JSON for removed domains.""" + self.client.force_login(self.user) + + data = { + "removed_domains": "json-sudeikis", + } + response = self.client.post(self.url, data) + + # Check that no DomainInvitation objects were updated + self.assertEqual(DomainInvitation.objects.count(), 0) + + # Check for an error message and a redirect + self.assertRedirects(response, reverse("invitedmember-domains", kwargs={"pk": self.invitation.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual( + str(messages[0]), "Invalid data for removed domains. If the issue persists, please contact help@get.gov." + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_post_with_no_changes(self): + """Test the case where no changes are made.""" + self.client.force_login(self.user) + + response = self.client.post(self.url, {}) + + # Check that no DomainInvitation objects were created or updated + self.assertEqual(DomainInvitation.objects.count(), 0) + + # Check for an info message and a redirect + self.assertRedirects(response, reverse("invitedmember-domains", kwargs={"pk": self.invitation.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual(str(messages[0]), "No changes detected.") + class TestRequestingEntity(WebTest): """The requesting entity page is a domain request form that only exists @@ -2268,6 +2576,46 @@ class TestRequestingEntity(WebTest): User.objects.all().delete() super().tearDown() + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_requests", active=True) + def test_form_validates_duplicate_suborganization(self): + """Tests that form validation prevents duplicate suborganization names within the same portfolio""" + # Create an existing suborganization + suborganization = Suborganization.objects.create(name="Existing Suborg", portfolio=self.portfolio) + + # Start the domain request process + response = self.app.get(reverse("domain-request:start")) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + + # Navigate past the intro page + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + form = response.forms[0] + response = form.submit().follow() + + # Fill out the requesting entity form + form = response.forms[0] + form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = "True" + form["portfolio_requesting_entity-is_requesting_new_suborganization"] = "True" + form["portfolio_requesting_entity-requested_suborganization"] = suborganization.name.lower() + form["portfolio_requesting_entity-suborganization_city"] = "Eggnog" + form["portfolio_requesting_entity-suborganization_state_territory"] = DomainRequest.StateTerritoryChoices.OHIO + + # Submit form and verify error + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + response = form.submit() + self.assertContains(response, "This suborganization already exists") + + # Test that a different name is allowed + form["portfolio_requesting_entity-requested_suborganization"] = "New Suborg" + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + response = form.submit().follow() + + # Verify successful submission by checking we're on the next page + self.assertContains(response, "Current websites") + @override_flag("organization_feature", active=True) @override_flag("organization_requests", active=True) @less_console_noise_decorator @@ -2531,7 +2879,9 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): ], ) - cls.new_member_email = "new_user@example.com" + cls.new_member_email = "davekenn4242@gmail.com" + + AllowedEmail.objects.get_or_create(email=cls.new_member_email) # Assign permissions to the user making requests UserPortfolioPermission.objects.create( @@ -2550,8 +2900,10 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): UserPortfolioPermission.objects.all().delete() Portfolio.objects.all().delete() User.objects.all().delete() + AllowedEmail.objects.all().delete() super().tearDownClass() + @boto3_mocking.patching @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @@ -2563,30 +2915,240 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): session_id = self.client.session.session_key self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # Simulate submission of member invite for new user - final_response = self.client.post( - reverse("new-member"), - { - "member_access_level": "basic", - "basic_org_domain_request_permissions": "view_only", - "email": self.new_member_email, - }, - ) + mock_client_class = MagicMock() + mock_client = mock_client_class.return_value - # Ensure the final submission is successful - self.assertEqual(final_response.status_code, 302) # redirects after success + with boto3_mocking.clients.handler_for("sesv2", mock_client_class): + # Simulate submission of member invite for new user + final_response = self.client.post( + reverse("new-member"), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "email": self.new_member_email, + }, + ) - # Validate Database Changes - portfolio_invite = PortfolioInvitation.objects.filter( - email=self.new_member_email, portfolio=self.portfolio - ).first() - self.assertIsNotNone(portfolio_invite) - self.assertEqual(portfolio_invite.email, self.new_member_email) + # Ensure the final submission is successful + self.assertEqual(final_response.status_code, 302) # Redirects + + # Validate Database Changes + portfolio_invite = PortfolioInvitation.objects.filter( + email=self.new_member_email, portfolio=self.portfolio + ).first() + self.assertIsNotNone(portfolio_invite) + self.assertEqual(portfolio_invite.email, self.new_member_email) + + # Check that an email was sent + self.assertTrue(mock_client.send_email.called) + + @boto3_mocking.patching + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_member_invite_for_new_users_initial_ajax_call_passes(self): + """Tests the member invitation flow for new users.""" + self.client.force_login(self.user) + + # Simulate a session to ensure continuity + session_id = self.client.session.session_key + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + mock_client_class = MagicMock() + mock_client = mock_client_class.return_value + + with boto3_mocking.clients.handler_for("sesv2", mock_client_class): + # Simulate submission of member invite for new user + final_response = self.client.post( + reverse("new-member"), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "email": self.new_member_email, + }, + HTTP_X_REQUESTED_WITH="XMLHttpRequest", + ) + + # Ensure the prep ajax submission is successful + self.assertEqual(final_response.status_code, 200) + + # Check that the response is a JSON response with is_valid + json_response = final_response.json() + self.assertIn("is_valid", json_response) + self.assertTrue(json_response["is_valid"]) + + # assert that portfolio invitation is not created + self.assertFalse( + PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(), + "Portfolio invitation should not be created when an Exception occurs.", + ) + + # Check that an email was not sent + self.assertFalse(mock_client.send_email.called) @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) - def test_member_invite_for_previously_invited_member(self): + @patch("registrar.views.portfolios.send_portfolio_invitation_email") + def test_member_invite_for_previously_invited_member_initial_ajax_call_fails(self, mock_send_email): + """Tests the initial ajax call in the member invitation flow for existing portfolio member.""" + self.client.force_login(self.user) + + # Simulate a session to ensure continuity + session_id = self.client.session.session_key + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + invite_count_before = PortfolioInvitation.objects.count() + + # Simulate submission of member invite for user who has already been invited + response = self.client.post( + reverse("new-member"), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "email": self.invited_member_email, + }, + HTTP_X_REQUESTED_WITH="XMLHttpRequest", + ) + self.assertEqual(response.status_code, 200) + + # Check that the response is a JSON response with is_valid == False + json_response = response.json() + self.assertIn("is_valid", json_response) + self.assertFalse(json_response["is_valid"]) + + # Validate Database has not changed + invite_count_after = PortfolioInvitation.objects.count() + self.assertEqual(invite_count_after, invite_count_before) + + # assert that send_portfolio_invitation_email is not called + mock_send_email.assert_not_called() + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + @patch("registrar.views.portfolios.send_portfolio_invitation_email") + def test_submit_new_member_raises_email_sending_error(self, mock_send_email): + """Test when adding a new member and email_send method raises EmailSendingError.""" + mock_send_email.side_effect = EmailSendingError("Failed to send email.") + + self.client.force_login(self.user) + + # Simulate a session to ensure continuity + session_id = self.client.session.session_key + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + form_data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "email": self.new_member_email, + } + + # Act + with patch("django.contrib.messages.warning") as mock_warning: + response = self.client.post(reverse("new-member"), data=form_data) + + # Assert + # assert that the send_portfolio_invitation_email called + mock_send_email.assert_called_once_with( + email=self.new_member_email, requestor=self.user, portfolio=self.portfolio + ) + # assert that response is a redirect to reverse("members") + self.assertRedirects(response, reverse("members")) + # assert that messages contains message, "Could not send email invitation" + mock_warning.assert_called_once_with(response.wsgi_request, "Could not send email invitation.") + # assert that portfolio invitation is not created + self.assertFalse( + PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(), + "Portfolio invitation should not be created when an EmailSendingError occurs.", + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + @patch("registrar.views.portfolios.send_portfolio_invitation_email") + def test_submit_new_member_raises_missing_email_error(self, mock_send_email): + """Test when adding a new member and email_send method raises MissingEmailError.""" + mock_send_email.side_effect = MissingEmailError() + + self.client.force_login(self.user) + + # Simulate a session to ensure continuity + session_id = self.client.session.session_key + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + form_data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "email": self.new_member_email, + } + + # Act + with patch("django.contrib.messages.error") as mock_error: + response = self.client.post(reverse("new-member"), data=form_data) + + # Assert + # assert that the send_portfolio_invitation_email called + mock_send_email.assert_called_once_with( + email=self.new_member_email, requestor=self.user, portfolio=self.portfolio + ) + # assert that response is a redirect to reverse("members") + self.assertRedirects(response, reverse("members")) + # assert that messages contains message, "Could not send email invitation" + mock_error.assert_called_once_with( + response.wsgi_request, + "Can't send invitation email. No email is associated with your user account.", + ) + # assert that portfolio invitation is not created + self.assertFalse( + PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(), + "Portfolio invitation should not be created when a MissingEmailError occurs.", + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + @patch("registrar.views.portfolios.send_portfolio_invitation_email") + def test_submit_new_member_raises_exception(self, mock_send_email): + """Test when adding a new member and email_send method raises Exception.""" + mock_send_email.side_effect = Exception("Generic exception") + + self.client.force_login(self.user) + + # Simulate a session to ensure continuity + session_id = self.client.session.session_key + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + form_data = { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "email": self.new_member_email, + } + + # Act + with patch("django.contrib.messages.warning") as mock_warning: + response = self.client.post(reverse("new-member"), data=form_data) + + # Assert + # assert that the send_portfolio_invitation_email called + mock_send_email.assert_called_once_with( + email=self.new_member_email, requestor=self.user, portfolio=self.portfolio + ) + # assert that response is a redirect to reverse("members") + self.assertRedirects(response, reverse("members")) + # assert that messages contains message, "Could not send email invitation" + mock_warning.assert_called_once_with(response.wsgi_request, "Could not send email invitation.") + # assert that portfolio invitation is not created + self.assertFalse( + PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(), + "Portfolio invitation should not be created when an Exception occurs.", + ) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + @patch("registrar.views.portfolios.send_portfolio_invitation_email") + def test_member_invite_for_previously_invited_member(self, mock_send_email): """Tests the member invitation flow for existing portfolio member.""" self.client.force_login(self.user) @@ -2600,23 +3162,35 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): response = self.client.post( reverse("new-member"), { - "member_access_level": "basic", - "basic_org_domain_request_permissions": "view_only", + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "email": self.invited_member_email, }, ) - self.assertEqual(response.status_code, 302) # Redirects + self.assertEqual(response.status_code, 200) - # TODO: verify messages + # verify messages + self.assertContains( + response, + ( + "This user is already assigned to a portfolio invitation. " + "Based on current waffle flag settings, users cannot be assigned " + "to multiple portfolios." + ), + ) # Validate Database has not changed invite_count_after = PortfolioInvitation.objects.count() self.assertEqual(invite_count_after, invite_count_before) + # assert that send_portfolio_invitation_email is not called + mock_send_email.assert_not_called() + @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) - def test_member_invite_for_existing_member(self): + @patch("registrar.views.portfolios.send_portfolio_invitation_email") + def test_member_invite_for_existing_member(self, mock_send_email): """Tests the member invitation flow for existing portfolio member.""" self.client.force_login(self.user) @@ -2630,19 +3204,30 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): response = self.client.post( reverse("new-member"), { - "member_access_level": "basic", - "basic_org_domain_request_permissions": "view_only", + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "email": self.user.email, }, ) - self.assertEqual(response.status_code, 302) # Redirects + self.assertEqual(response.status_code, 200) - # TODO: verify messages + # Verify messages + self.assertContains( + response, + ( + "This user is already assigned to a portfolio. " + "Based on current waffle flag settings, users cannot be " + "assigned to multiple portfolios." + ), + ) # Validate Database has not changed invite_count_after = PortfolioInvitation.objects.count() self.assertEqual(invite_count_after, invite_count_before) + # assert that send_portfolio_invitation_email is not called + mock_send_email.assert_not_called() + class TestEditPortfolioMemberView(WebTest): """Tests for the edit member page on portfolios""" @@ -2783,7 +3368,13 @@ class TestEditPortfolioMemberView(WebTest): @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) def test_admin_removing_own_admin_role(self): - """Tests an admin removing their own admin role redirects to home.""" + """Tests an admin removing their own admin role redirects to home. + + Removing the admin role will remove both view and edit members permissions. + Note: The user can remove the edit members permissions but as long as they + stay in admin role, they will at least still have view members permissions. + """ + self.client.force_login(self.user) # Get the user's admin permission diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index af2fadeb9..7addf041d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -417,7 +417,7 @@ class MemberExport(BaseExport): # Adding a order_by increases output predictability. # Doesn't matter as much for normal use, but makes tests easier. # We should also just be ordering by default anyway. - members = permissions.union(invitations).order_by("email_display") + members = permissions.union(invitations).order_by("email_display", "member_display", "first_name", "last_name") return convert_queryset_to_dict(members, is_model=False) @classmethod diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py new file mode 100644 index 000000000..7171b8902 --- /dev/null +++ b/src/registrar/utility/email_invitations.py @@ -0,0 +1,114 @@ +from django.conf import settings +from registrar.models import DomainInvitation +from registrar.utility.errors import ( + AlreadyDomainInvitedError, + AlreadyDomainManagerError, + MissingEmailError, + OutsideOrgMemberError, +) +from registrar.utility.waffle import flag_is_active_for_user +from registrar.utility.email import send_templated_email +import logging + +logger = logging.getLogger(__name__) + + +def send_domain_invitation_email(email: str, requestor, domain, is_member_of_different_org): + """ + Sends a domain invitation email to the specified address. + + Raises exceptions for validation or email-sending issues. + + Args: + email (str): Email address of the recipient. + requestor (User): The user initiating the invitation. + domain (Domain): The domain object for which the invitation is being sent. + is_member_of_different_org (bool): if an email belongs to a different org + + Raises: + MissingEmailError: If the requestor has no email associated with their account. + AlreadyDomainManagerError: If the email corresponds to an existing domain manager. + AlreadyDomainInvitedError: If an invitation has already been sent. + OutsideOrgMemberError: If the requested_user is part of a different organization. + EmailSendingError: If there is an error while sending the email. + """ + # Default email address for staff + requestor_email = settings.DEFAULT_FROM_EMAIL + + # Check if the requestor is staff and has an email + if not requestor.is_staff: + if not requestor.email or requestor.email.strip() == "": + raise MissingEmailError + else: + requestor_email = requestor.email + + # Check if the recipient is part of a different organization + # COMMENT: this does not account for multiple_portfolios flag being active + if ( + flag_is_active_for_user(requestor, "organization_feature") + and not flag_is_active_for_user(requestor, "multiple_portfolios") + and is_member_of_different_org + ): + raise OutsideOrgMemberError + + # Check for an existing invitation + try: + invite = DomainInvitation.objects.get(email=email, domain=domain) + if invite.status == DomainInvitation.DomainInvitationStatus.RETRIEVED: + raise AlreadyDomainManagerError(email) + elif invite.status == DomainInvitation.DomainInvitationStatus.CANCELED: + invite.update_cancellation_status() + invite.save() + else: + raise AlreadyDomainInvitedError(email) + except DomainInvitation.DoesNotExist: + pass + + # Send the email + send_templated_email( + "emails/domain_invitation.txt", + "emails/domain_invitation_subject.txt", + to_address=email, + context={ + "domain": domain, + "requestor_email": requestor_email, + }, + ) + + +def send_portfolio_invitation_email(email: str, requestor, portfolio): + """ + Sends a portfolio member invitation email to the specified address. + + Raises exceptions for validation or email-sending issues. + + Args: + email (str): Email address of the recipient + requestor (User): The user initiating the invitation. + portfolio (Portfolio): The portfolio object for which the invitation is being sent. + + Raises: + MissingEmailError: If the requestor has no email associated with their account. + EmailSendingError: If there is an error while sending the email. + """ + + # Default email address for staff + requestor_email = settings.DEFAULT_FROM_EMAIL + + # Check if the requestor is staff and has an email + if not requestor.is_staff: + if not requestor.email or requestor.email.strip() == "": + raise MissingEmailError + else: + requestor_email = requestor.email + + send_templated_email( + "emails/portfolio_invitation.txt", + "emails/portfolio_invitation_subject.txt", + to_address=email, + context={ + "portfolio": portfolio, + "requestor_email": requestor_email, + "email": email, + }, + ) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index e70f06d1e..039fb3696 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -23,6 +23,33 @@ class InvalidDomainError(ValueError): pass +class InvitationError(Exception): + """Base exception for invitation-related errors.""" + + pass + + +class AlreadyDomainManagerError(InvitationError): + """Raised when the user is already a manager for the domain.""" + + def __init__(self, email): + super().__init__(f"{email} is already a manager for this domain.") + + +class AlreadyDomainInvitedError(InvitationError): + """Raised when the user has already been invited to the domain.""" + + def __init__(self, email): + super().__init__(f"{email} has already been invited to this domain.") + + +class MissingEmailError(InvitationError): + """Raised when the requestor has no email associated with their account.""" + + def __init__(self): + super().__init__("Can't send invitation email. No email is associated with your user account.") + + class OutsideOrgMemberError(ValueError): """ Error raised when an org member tries adding a user from a different .gov org. diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 45f1e6ad2..f544a20f7 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -25,14 +25,17 @@ from registrar.models import ( PortfolioInvitation, User, UserDomainRole, - UserPortfolioPermission, PublicContact, ) +from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.utility.enums import DefaultEmail from registrar.utility.errors import ( + AlreadyDomainInvitedError, + AlreadyDomainManagerError, GenericError, GenericErrorCodes, + MissingEmailError, NameserverError, NameserverErrorCodes as nsErrorCodes, DsDataError, @@ -63,6 +66,7 @@ from epplibwrapper import ( ) from ..utility.email import send_templated_email, EmailSendingError +from ..utility.email_invitations import send_domain_invitation_email, send_portfolio_invitation_email from .utility import DomainPermissionView, DomainInvitationPermissionCancelView from django import forms @@ -1100,7 +1104,10 @@ class DomainUsersView(DomainBaseView): # If any of the PortfolioInvitations have the ORGANIZATION_ADMIN role, set the flag to True for portfolio_invitation in portfolio_invitations: - if UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_invitation.roles: + if ( + portfolio_invitation.roles + and UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_invitation.roles + ): has_admin_flag = True break # Once we find one match, no need to check further @@ -1142,170 +1149,172 @@ class DomainAddUserView(DomainFormBaseView): def get_success_url(self): return reverse("domain-users", kwargs={"pk": self.object.pk}) - def _domain_abs_url(self): - """Get an absolute URL for this domain.""" - return self.request.build_absolute_uri(reverse("domain", kwargs={"pk": self.object.id})) + def _get_org_membership(self, requestor_org, requested_email, requested_user): + """ + Verifies if an email belongs to a different organization as a member or invited member. + Verifies if an email belongs to this organization as a member or invited member. + User does not belong to any org can be deduced from the tuple returned. - def _is_member_of_different_org(self, email, requestor, requested_user): - """Verifies if an email belongs to a different organization as a member or invited member.""" - # Check if user is a already member of a different organization than the requestor's org - requestor_org = UserPortfolioPermission.objects.filter(user=requestor).first().portfolio - existing_org_permission = UserPortfolioPermission.objects.filter(user=requested_user).first() - existing_org_invitation = PortfolioInvitation.objects.filter(email=email).first() - - return (existing_org_permission and existing_org_permission.portfolio != requestor_org) or ( - existing_org_invitation and existing_org_invitation.portfolio != requestor_org - ) - - def _check_invite_status(self, invite, email): - """Check if invitation status is canceled or retrieved, and gives the appropiate response""" - if invite.status == DomainInvitation.DomainInvitationStatus.RETRIEVED: - messages.warning( - self.request, - f"{email} is already a manager for this domain.", - ) - return False - elif invite.status == DomainInvitation.DomainInvitationStatus.CANCELED: - invite.update_cancellation_status() - invite.save() - return True - else: - # else if it has been sent but not accepted - messages.warning(self.request, f"{email} has already been invited to this domain") - return False - - def _send_domain_invitation_email(self, email: str, requestor: User, requested_user=None, add_success=True): - """Performs the sending of the domain invitation email, - does not make a domain information object - 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 + Returns a tuple (member_of_a_different_org, member_of_this_org). """ - # Set a default email address to send to for staff - requestor_email = settings.DEFAULT_FROM_EMAIL + # COMMENT: this code does not take into account multiple portfolios flag - # 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, - ) + # COMMENT: shouldn't this code be based on the organization of the domain, not the org + # of the requestor? requestor could have multiple portfolios + + # Check for existing permissions or invitations for the requested user + existing_org_permission = UserPortfolioPermission.objects.filter(user=requested_user).first() + existing_org_invitation = PortfolioInvitation.objects.filter(email=requested_email).first() + + # Determine membership in a different organization + member_of_a_different_org = ( + existing_org_permission and existing_org_permission.portfolio != requestor_org + ) or (existing_org_invitation and existing_org_invitation.portfolio != requestor_org) + + # Determine membership in the same organization + member_of_this_org = (existing_org_permission and existing_org_permission.portfolio == requestor_org) or ( + existing_org_invitation and existing_org_invitation.portfolio == requestor_org + ) + + return member_of_a_different_org, member_of_this_org + + def form_valid(self, form): + """Add the specified user to this domain.""" + requested_email = form.cleaned_data["email"] + requestor = self.request.user + + # Look up a user with that email + requested_user = self._get_requested_user(requested_email) + # NOTE: This does not account for multiple portfolios flag being set to True + domain_org = self.object.domain_info.portfolio + + # requestor can only send portfolio invitations if they are staff or if they are a member + # of the domain's portfolio + requestor_can_update_portfolio = ( + UserPortfolioPermission.objects.filter(user=requestor, portfolio=domain_org).first() is not None + or requestor.is_staff + ) + + member_of_a_different_org, member_of_this_org = self._get_org_membership( + domain_org, requested_email, requested_user + ) + + # determine portfolio of the domain (code currently is looking at requestor's portfolio) + # if requested_email/user is not member or invited member of this portfolio + # COMMENT: this code does not take into account multiple portfolios flag + # send portfolio invitation email + # create portfolio invitation + # create message to view + if ( + flag_is_active_for_user(requestor, "organization_feature") + and not flag_is_active_for_user(requestor, "multiple_portfolios") + and domain_org is not None + and requestor_can_update_portfolio + and not member_of_this_org + ): + try: + send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=domain_org) + PortfolioInvitation.objects.get_or_create(email=requested_email, portfolio=domain_org) + messages.success(self.request, f"{requested_email} has been invited to the organization: {domain_org}") + except Exception as e: + self._handle_portfolio_exceptions(e, requested_email, domain_org) + # If that first invite does not succeed take an early exit + return redirect(self.get_success_url()) + + try: + if requested_user is None: + self._handle_new_user_invitation(requested_email, requestor, member_of_a_different_org) + else: + self._handle_existing_user(requested_email, requestor, requested_user, member_of_a_different_org) + except Exception as e: + self._handle_exceptions(e, requested_email) + + return redirect(self.get_success_url()) + + def _get_requested_user(self, email): + """Retrieve a user by email or return None if the user doesn't exist.""" + try: + return User.objects.get(email=email) + except User.DoesNotExist: return None - # Check is user is a member or invited member of a different org from this domain's org - if flag_is_active_for_user(requestor, "organization_feature") and self._is_member_of_different_org( - email, requestor, requested_user - ): - add_success = False - raise OutsideOrgMemberError + def _handle_new_user_invitation(self, email, requestor, member_of_different_org): + """Handle invitation for a new user who does not exist in the system.""" + send_domain_invitation_email( + email=email, + requestor=requestor, + domain=self.object, + is_member_of_different_org=member_of_different_org, + ) + DomainInvitation.objects.get_or_create(email=email, domain=self.object) + messages.success(self.request, f"{email} has been invited to the domain: {self.object}") - # Check to see if an invite has already been sent - try: - invite = DomainInvitation.objects.get(email=email, domain=self.object) - # check if the invite has already been accepted or has a canceled invite - add_success = self._check_invite_status(invite, email) - except Exception: - logger.error("An error occured") + def _handle_existing_user(self, email, requestor, requested_user, member_of_different_org): + """Handle adding an existing user to the domain.""" + send_domain_invitation_email( + email=email, + requestor=requestor, + domain=self.object, + is_member_of_different_org=member_of_different_org, + ) + UserDomainRole.objects.create( + user=requested_user, + domain=self.object, + role=UserDomainRole.Roles.MANAGER, + ) + messages.success(self.request, f"Added user {email}.") - try: - send_templated_email( - "emails/domain_invitation.txt", - "emails/domain_invitation_subject.txt", - to_address=email, - context={ - "domain_url": self._domain_abs_url(), - "domain": self.object, - "requestor_email": requestor_email, - }, - ) - except EmailSendingError as exc: - logger.warn( - "Could not sent email invitation to %s for domain %s", + def _handle_exceptions(self, exception, email): + """Handle exceptions raised during the process.""" + if isinstance(exception, EmailSendingError): + logger.warning( + "Could not send email invitation to %s for domain %s (EmailSendingError)", email, self.object, exc_info=True, ) - logger.info(exc) - raise EmailSendingError("Could not send email invitation.") from exc - else: - if add_success: - messages.success(self.request, f"{email} has been invited to this domain.") - - def _make_invitation(self, email_address: str, requestor: User): - """Make a Domain invitation for this email and redirect with a message.""" - try: - self._send_domain_invitation_email(email=email_address, requestor=requestor) - except EmailSendingError: messages.warning(self.request, "Could not send email invitation.") + elif isinstance(exception, OutsideOrgMemberError): + logger.warning( + "Could not send email. Can not invite member of a .gov organization to a different organization.", + self.object, + exc_info=True, + ) + messages.error( + self.request, + f"{email} is already a member of another .gov organization.", + ) + elif isinstance(exception, AlreadyDomainManagerError): + messages.warning(self.request, str(exception)) + elif isinstance(exception, AlreadyDomainInvitedError): + messages.warning(self.request, str(exception)) + elif isinstance(exception, MissingEmailError): + messages.error(self.request, str(exception)) + logger.error( + f"Can't send email to '{email}' on domain '{self.object}'. No email exists for the requestor.", + exc_info=True, + ) + elif isinstance(exception, IntegrityError): + messages.warning(self.request, f"{email} is already a manager for this domain") else: - # (NOTE: only create a domainInvitation if the e-mail sends correctly) - DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) - return redirect(self.get_success_url()) + logger.warning("Could not send email invitation (Other Exception)", exc_info=True) + messages.warning(self.request, "Could not send email invitation.") - def form_valid(self, form): - """Add the specified user on this domain. - Throws EmailSendingError.""" - requested_email = form.cleaned_data["email"] - requestor = self.request.user - email_success = False - # look up a user with that email - try: - requested_user = User.objects.get(email=requested_email) - except User.DoesNotExist: - # no matching user, go make an invitation - email_success = True - return self._make_invitation(requested_email, requestor) + def _handle_portfolio_exceptions(self, exception, email, portfolio): + """Handle exceptions raised during the process.""" + if isinstance(exception, EmailSendingError): + logger.warning("Could not send email invitation (EmailSendingError)", exc_info=True) + messages.warning(self.request, "Could not send email invitation.") + elif isinstance(exception, MissingEmailError): + messages.error(self.request, str(exception)) + logger.error( + f"Can't send email to '{email}' for portfolio '{portfolio}'. No email exists for the requestor.", + exc_info=True, + ) else: - # if user already exists then just send an email - try: - self._send_domain_invitation_email( - requested_email, requestor, requested_user=requested_user, add_success=False - ) - email_success = True - except EmailSendingError: - logger.warn( - "Could not send email invitation (EmailSendingError)", - self.object, - exc_info=True, - ) - messages.warning(self.request, "Could not send email invitation.") - email_success = True - except OutsideOrgMemberError: - logger.warn( - "Could not send email. Can not invite member of a .gov organization to a different organization.", - self.object, - exc_info=True, - ) - messages.error( - self.request, - f"{requested_email} is already a member of another .gov organization.", - ) - 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.") - if email_success: - try: - UserDomainRole.objects.create( - user=requested_user, - domain=self.object, - role=UserDomainRole.Roles.MANAGER, - ) - messages.success(self.request, f"Added user {requested_email}.") - except IntegrityError: - messages.warning(self.request, f"{requested_email} is already a manager for this domain") - - return redirect(self.get_success_url()) + logger.warning("Could not send email invitation (Other Exception)", exc_info=True) + messages.warning(self.request, "Could not send email invitation.") class DomainInvitationCancelView(SuccessMessageMixin, DomainInvitationPermissionCancelView): diff --git a/src/registrar/views/member_domains_json.py b/src/registrar/views/member_domains_json.py index 125059692..3d24336bb 100644 --- a/src/registrar/views/member_domains_json.py +++ b/src/registrar/views/member_domains_json.py @@ -90,7 +90,9 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): domain_info_ids = DomainInformation.objects.filter(portfolio=portfolio).values_list( "domain_id", flat=True ) - domain_invitations = DomainInvitation.objects.filter(email=email).values_list("domain_id", flat=True) + domain_invitations = DomainInvitation.objects.filter( + email=email, status=DomainInvitation.DomainInvitationStatus.INVITED + ).values_list("domain_id", flat=True) return domain_info_ids.intersection(domain_invitations) else: domain_infos = DomainInformation.objects.filter(portfolio=portfolio) diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index b5c608eab..11e58e112 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -12,6 +12,7 @@ from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.views.utility.mixins import PortfolioMembersPermission from registrar.models.utility.orm_helper import ArrayRemoveNull +from django.contrib.postgres.aggregates import StringAgg class PortfolioMembersJson(PortfolioMembersPermission, View): @@ -119,11 +120,22 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): def initial_invitations_search(self, portfolio): """Perform initial invitations search and get related DomainInvitation data based on the email.""" - # Get DomainInvitation query for matching email and for the portfolio - domain_invitations = DomainInvitation.objects.filter( - email=OuterRef("email"), # Check if email matches the OuterRef("email") - domain__domain_info__portfolio=portfolio, # Check if the domain's portfolio matches the given portfolio - ).annotate(domain_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField())) + + # Subquery to get concatenated domain information for each email + domain_invitations = ( + DomainInvitation.objects.filter(email=OuterRef("email"), domain__domain_info__portfolio=portfolio) + .annotate( + concatenated_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField()) + ) + .values("concatenated_info") + ) + + concatenated_domain_info = ( + domain_invitations.values("email") + .annotate(domain_info=StringAgg("concatenated_info", delimiter=", ")) + .values("domain_info") + ) + # PortfolioInvitation query invitations = PortfolioInvitation.objects.filter(portfolio=portfolio) invitations = invitations.annotate( @@ -136,7 +148,12 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): # Use ArrayRemove to return an empty list when no domain invitations are found domain_info=ArrayRemoveNull( ArrayAgg( - Subquery(domain_invitations.values("domain_info")), + # We've pre-concatenated the domain infos to limit the subquery to return a single virtual 'row', + # otherwise we'll trigger a "more than one row returned by a subquery used as an expression" + # when an email matches multiple domain invitations. + # We'll take care when processing the list of one single concatenated items item + # in serialize_members. + Subquery(concatenated_domain_info), distinct=True, ) ), @@ -153,6 +170,7 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): "domain_info", "type", ) + return invitations def apply_search_term(self, queryset, request): @@ -190,10 +208,19 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in (item.get("roles") or []) action_url = reverse(item["type"], kwargs={"pk": item["id"]}) + item_type = item.get("type", "") + + # Ensure domain_info is properly processed for invites - + # we need to un-concatenate the subquery + domain_info_list = item.get("domain_info", []) + if item_type == "invitedmember" and isinstance(domain_info_list, list) and domain_info_list: + # Split the first item in the list if it exists + domain_info_list = domain_info_list[0].split(", ") + # Serialize member data member_json = { "id": item.get("id", ""), # id is id of UserPortfolioPermission or PortfolioInvitation - "type": item.get("type", ""), # source is member or invitedmember + "type": item_type, # source is member or invitedmember "name": " ".join(filter(None, [item.get("first_name", ""), item.get("last_name", "")])), "email": item.get("email_display", ""), "member_display": item.get("member_display", ""), @@ -203,9 +230,9 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): ), # split domain_info array values into ids to form urls, and names "domain_urls": [ - reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) for domain_info in item.get("domain_info") + reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) for domain_info in domain_info_list ], - "domain_names": [domain_info.split(":")[1] for domain_info in item.get("domain_info")], + "domain_names": [domain_info.split(":")[1] for domain_info in domain_info_list], "is_admin": is_admin, "last_active": item.get("last_active"), "action_url": action_url, diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 8e1df48f3..751e28d85 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -1,5 +1,5 @@ +import json import logging -from django.conf import settings from django.http import Http404, JsonResponse from django.shortcuts import get_object_or_404, redirect, render @@ -8,10 +8,15 @@ from django.utils.safestring import mark_safe from django.contrib import messages from registrar.forms import portfolio as portfolioForms from registrar.models import Portfolio, User +from registrar.models.domain_invitation import DomainInvitation from registrar.models.portfolio_invitation import PortfolioInvitation +from registrar.models.user_domain_role import UserDomainRole 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.utility.email_invitations import send_portfolio_invitation_email +from registrar.utility.errors import MissingEmailError +from registrar.utility.enums import DefaultUserValues from registrar.views.utility.mixins import PortfolioMemberPermission from registrar.views.utility.permission_views import ( PortfolioDomainRequestsPermissionView, @@ -26,6 +31,7 @@ from registrar.views.utility.permission_views import ( ) from django.views.generic import View from django.views.generic.edit import FormMixin +from django.db import IntegrityError logger = logging.getLogger(__name__) @@ -145,7 +151,7 @@ class PortfolioMemberDeleteView(PortfolioMemberPermission, View): class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): template_name = "portfolio_member_permissions.html" - form_class = portfolioForms.BasePortfolioMemberForm + form_class = portfolioForms.PortfolioMemberForm def get(self, request, pk): portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) @@ -164,13 +170,14 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): def post(self, request, pk): portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + user_initially_is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_permission.roles user = portfolio_permission.user form = self.form_class(request.POST, instance=portfolio_permission) if form.is_valid(): # Check if user is removing their own admin or edit role removing_admin_role_on_self = ( request.user == user - and UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_permission.roles + and user_initially_is_admin and UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in form.cleaned_data.get("role", []) ) form.save() @@ -222,6 +229,86 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V }, ) + def post(self, request, pk): + """ + Handles adding and removing domains for a portfolio member. + """ + added_domains = request.POST.get("added_domains") + removed_domains = request.POST.get("removed_domains") + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + member = portfolio_permission.user + + added_domain_ids = self._parse_domain_ids(added_domains, "added domains") + if added_domain_ids is None: + return redirect(reverse("member-domains", kwargs={"pk": pk})) + + removed_domain_ids = self._parse_domain_ids(removed_domains, "removed domains") + if removed_domain_ids is None: + return redirect(reverse("member-domains", kwargs={"pk": pk})) + + if added_domain_ids or removed_domain_ids: + try: + self._process_added_domains(added_domain_ids, member) + self._process_removed_domains(removed_domain_ids, member) + messages.success(request, "The domain assignment changes have been saved.") + return redirect(reverse("member-domains", kwargs={"pk": pk})) + except IntegrityError: + messages.error( + request, + "A database error occurred while saving changes. If the issue persists, " + f"please contact {DefaultUserValues.HELP_EMAIL}.", + ) + logger.error("A database error occurred while saving changes.") + return redirect(reverse("member-domains-edit", kwargs={"pk": pk})) + except Exception as e: + messages.error( + request, + "An unexpected error occurred: {str(e)}. If the issue persists, " + f"please contact {DefaultUserValues.HELP_EMAIL}.", + ) + logger.error(f"An unexpected error occurred: {str(e)}") + return redirect(reverse("member-domains-edit", kwargs={"pk": pk})) + else: + messages.info(request, "No changes detected.") + return redirect(reverse("member-domains", kwargs={"pk": pk})) + + def _parse_domain_ids(self, domain_data, domain_type): + """ + Parses the domain IDs from the request and handles JSON errors. + """ + try: + return json.loads(domain_data) if domain_data else [] + except json.JSONDecodeError: + messages.error( + self.request, + f"Invalid data for {domain_type}. If the issue persists, " + f"please contact {DefaultUserValues.HELP_EMAIL}.", + ) + logger.error(f"Invalid data for {domain_type}") + return None + + def _process_added_domains(self, added_domain_ids, member): + """ + Processes added domains by bulk creating UserDomainRole instances. + """ + if added_domain_ids: + # Bulk create UserDomainRole instances for added domains + UserDomainRole.objects.bulk_create( + [ + UserDomainRole(domain_id=domain_id, user=member, role=UserDomainRole.Roles.MANAGER) + for domain_id in added_domain_ids + ], + ignore_conflicts=True, # Avoid duplicate entries + ) + + def _process_removed_domains(self, removed_domain_ids, member): + """ + Processes removed domains by deleting corresponding UserDomainRole instances. + """ + if removed_domain_ids: + # Delete UserDomainRole instances for removed domains + UserDomainRole.objects.filter(domain_id__in=removed_domain_ids, user=member).delete() + class PortfolioInvitedMemberView(PortfolioMemberPermissionView, View): @@ -284,7 +371,7 @@ class PortfolioInvitedMemberDeleteView(PortfolioMemberPermission, View): class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View): template_name = "portfolio_member_permissions.html" - form_class = portfolioForms.BasePortfolioMemberForm + form_class = portfolioForms.PortfolioInvitedMemberForm def get(self, request, pk): portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) @@ -348,6 +435,106 @@ class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermission }, ) + def post(self, request, pk): + """ + Handles adding and removing domains for a portfolio invitee. + """ + added_domains = request.POST.get("added_domains") + removed_domains = request.POST.get("removed_domains") + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + email = portfolio_invitation.email + + added_domain_ids = self._parse_domain_ids(added_domains, "added domains") + if added_domain_ids is None: + return redirect(reverse("invitedmember-domains", kwargs={"pk": pk})) + + removed_domain_ids = self._parse_domain_ids(removed_domains, "removed domains") + if removed_domain_ids is None: + return redirect(reverse("invitedmember-domains", kwargs={"pk": pk})) + + if added_domain_ids or removed_domain_ids: + try: + self._process_added_domains(added_domain_ids, email) + self._process_removed_domains(removed_domain_ids, email) + messages.success(request, "The domain assignment changes have been saved.") + return redirect(reverse("invitedmember-domains", kwargs={"pk": pk})) + except IntegrityError: + messages.error( + request, + "A database error occurred while saving changes. If the issue persists, " + f"please contact {DefaultUserValues.HELP_EMAIL}.", + ) + logger.error("A database error occurred while saving changes.") + return redirect(reverse("invitedmember-domains-edit", kwargs={"pk": pk})) + except Exception as e: + messages.error( + request, + "An unexpected error occurred: {str(e)}. If the issue persists, " + f"please contact {DefaultUserValues.HELP_EMAIL}.", + ) + logger.error(f"An unexpected error occurred: {str(e)}.") + return redirect(reverse("invitedmember-domains-edit", kwargs={"pk": pk})) + else: + messages.info(request, "No changes detected.") + return redirect(reverse("invitedmember-domains", kwargs={"pk": pk})) + + def _parse_domain_ids(self, domain_data, domain_type): + """ + Parses the domain IDs from the request and handles JSON errors. + """ + try: + return json.loads(domain_data) if domain_data else [] + except json.JSONDecodeError: + messages.error( + self.request, + f"Invalid data for {domain_type}. If the issue persists, " + f"please contact {DefaultUserValues.HELP_EMAIL}.", + ) + logger.error(f"Invalid data for {domain_type}.") + return None + + def _process_added_domains(self, added_domain_ids, email): + """ + Processes added domain invitations by updating existing invitations + or creating new ones. + """ + if not added_domain_ids: + return + + # Update existing invitations from CANCELED to INVITED + existing_invitations = DomainInvitation.objects.filter(domain_id__in=added_domain_ids, email=email) + existing_invitations.update(status=DomainInvitation.DomainInvitationStatus.INVITED) + + # Determine which domains need new invitations + existing_domain_ids = existing_invitations.values_list("domain_id", flat=True) + new_domain_ids = set(added_domain_ids) - set(existing_domain_ids) + + # Bulk create new invitations + DomainInvitation.objects.bulk_create( + [ + DomainInvitation( + domain_id=domain_id, + email=email, + status=DomainInvitation.DomainInvitationStatus.INVITED, + ) + for domain_id in new_domain_ids + ] + ) + + def _process_removed_domains(self, removed_domain_ids, email): + """ + Processes removed domain invitations by updating their status to CANCELED. + """ + if not removed_domain_ids: + return + + # Update invitations from INVITED to CANCELED + DomainInvitation.objects.filter( + domain_id__in=removed_domain_ids, + email=email, + status=DomainInvitation.DomainInvitationStatus.INVITED, + ).update(status=DomainInvitation.DomainInvitationStatus.CANCELED) + class PortfolioNoDomainsView(NoPortfolioDomainsPermissionView, View): """Some users have access to the underlying portfolio, but not any domains. @@ -509,34 +696,27 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View): return render(request, "portfolio_members.html") -class NewMemberView(PortfolioMembersPermissionView, FormMixin): +class PortfolioAddMemberView(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): - """Include the instance in the form kwargs.""" - kwargs = super().get_form_kwargs() - kwargs["instance"] = self.get_object() - return kwargs + form_class = portfolioForms.PortfolioNewMemberForm def get(self, request, *args, **kwargs): """Handle GET requests to display the form.""" - self.object = self.get_object() + self.object = None # No existing PortfolioInvitation instance form = self.get_form() return self.render_to_response(self.get_context_data(form=form)) def post(self, request, *args, **kwargs): """Handle POST requests to process form submission.""" - self.object = self.get_object() - form = self.get_form() + self.object = None # For a new invitation, there's no existing model instance + + # portfolio not submitted with form, so override the value + data = request.POST.copy() + if not data.get("portfolio"): + data["portfolio"] = self.request.session.get("portfolio").id + # Pass the modified data to the form + form = portfolioForms.PortfolioNewMemberForm(data) if form.is_valid(): return self.form_valid(form) @@ -553,7 +733,7 @@ class NewMemberView(PortfolioMembersPermissionView, FormMixin): return super().form_invalid(form) # Handle non-AJAX requests normally def form_valid(self, form): - + super().form_valid(form) if self.is_ajax(): return JsonResponse({"is_valid": True}) # Return a JSON response else: @@ -563,108 +743,42 @@ class NewMemberView(PortfolioMembersPermissionView, FormMixin): """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.""" + """Add the specified user as a member for this portfolio.""" requested_email = form.cleaned_data["email"] requestor = self.request.user + portfolio = form.cleaned_data["portfolio"] 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.") + 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) + form.save() + 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 _handle_exceptions(self, exception, portfolio, email): + """Handle exceptions raised during the process.""" + if isinstance(exception, EmailSendingError): + logger.warning( + "Could not sent email invitation to %s for portfolio %s (EmailSendingError)", + email, + portfolio, + exc_info=True, + ) + messages.warning(self.request, "Could not send email invitation.") + elif isinstance(exception, MissingEmailError): + messages.error(self.request, str(exception)) + logger.error( + f"Can't send email to '{email}' for portfolio '{portfolio}'. No email exists for the requestor.", + exc_info=True, + ) + else: + logger.warning("Could not send email invitation (Other Exception)", exc_info=True) + messages.warning(self.request, "Could not send email invitation.")