diff --git a/docs/developer/registry-access.md b/docs/developer/registry-access.md index c7737d5bc..50caa4823 100644 --- a/docs/developer/registry-access.md +++ b/docs/developer/registry-access.md @@ -103,3 +103,31 @@ response = registry._client.transport.receive() ``` This is helpful for debugging situations where epplib is not correctly or fully parsing the XML returned from the registry. + +### Adding in a expiring soon domain +The below scenario is if you are NOT in org model mode (`organization_feature` waffle flag is off). + +1. Go to the `staging` sandbox and to `/admin` +2. Go to Domains and find a domain that is actually expired by sorting the Expiration Date column +3. Click into the domain to check the expiration date +4. Click into Manage Domain to double check the expiration date as well +5. Now hold onto that domain name, and save it for the command below + +6. In a terminal, run these commands: +``` +cf ssh getgov- +/tmp/lifecycle/shell +./manage.py shell +from registrar.models import Domain, DomainInvitation +from registrar.models import User +user = User.objects.filter(first_name="") +domain = Domain.objects.get_or_create(name="") +``` + +7. Go back to `/admin` and create Domain Information for that domain you just added in via the terminal +8. Go to Domain to find it +9. Click Manage Domain +10. Add yourself as domain manager +11. Go to the Registrar page and you should now see the expiring domain + +If you want to be in the org model mode, turn the `organization_feature` waffle flag on, and add that domain via Django Admin to a portfolio to be able to view it. \ No newline at end of file diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 0863aa0b7..499c0840f 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -918,3 +918,38 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi - Parameters #1-#2: Either `--agency_name` or `--branch` must be specified. Not both. - Parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them, you must specify at least one to run this script. + + +## Patch suborganizations +This script deletes some duplicate suborganization data that exists in our database (one-time use). +It works in two ways: +1. If the only name difference between two suborg records is extra spaces or a capitalization difference, +then we delete all duplicate records of this type. +2. If the suborg name is one we manually specify to delete via the script. + +Before it deletes records, it goes through each DomainInformation and DomainRequest object and updates the reference to "sub_organization" to match the non-duplicative record. + +### Running on sandboxes + +#### Step 1: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 2: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-za` + +#### Step 3: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 4: Upload your csv to the desired sandbox +[Follow these steps](#use-scp-to-transfer-data-to-sandboxes) to upload the federal_cio csv to a sandbox of your choice. + +#### Step 5: Running the script +To create a specific portfolio: +```./manage.py patch_suborganizations``` + +### Running locally + +#### Step 1: Running the script +```docker-compose exec app ./manage.py patch_suborganizations``` diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 849cb6100..e89147b11 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -14,6 +14,7 @@ from django.db.models import ( from django.db.models.functions import Concat, Coalesce from django.http import HttpResponseRedirect from registrar.models.federal_agency import FederalAgency +from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.utility.admin_helpers import ( AutocompleteSelectWithPlaceholder, get_action_needed_reason_default_email, @@ -27,8 +28,12 @@ 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 registrar.utility.email_invitations import send_domain_invitation_email, send_portfolio_invitation_email +from registrar.views.utility.invitation_helper import ( + get_org_membership, + get_requested_user, + handle_invitation_exceptions, +) from waffle.decorators import flag_is_active from django.contrib import admin, messages from django.contrib.auth.admin import UserAdmin as BaseUserAdmin @@ -41,7 +46,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, MissingEmailError +from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes 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 @@ -1362,6 +1367,8 @@ class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): autocomplete_fields = ["user", "domain"] + change_form_template = "django/admin/user_domain_role_change_form.html" + # Fixes a bug where non-superusers are redirected to the main page def delete_view(self, request, object_id, extra_context=None): """Custom delete_view implementation that specifies redirect behaviour""" @@ -1389,169 +1396,9 @@ class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().changeform_view(request, object_id, form_url, extra_context=extra_context) -class DomainInvitationAdmin(ListHeaderAdmin): - """Custom domain invitation admin class.""" - - class Meta: - model = models.DomainInvitation - fields = "__all__" - - _meta = Meta() - - # Columns - list_display = [ - "email", - "domain", - "status", - ] - - # Search - search_fields = [ - "email", - "domain__name", - ] - - # Filters - list_filter = ("status",) - - search_help_text = "Search by email or domain." - - # Mark the FSM field 'status' as readonly - # to allow admin users to create Domain Invitations - # without triggering the FSM Transition Not Allowed - # error. - readonly_fields = ["status"] - - autocomplete_fields = ["domain"] - - change_form_template = "django/admin/email_clipboard_change_form.html" - - # Select domain invitations to change -> Domain invitations - def changelist_view(self, request, extra_context=None): - if extra_context is None: - extra_context = {} - extra_context["tabtitle"] = "Domain invitations" - # 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. - - On creation of a new domain invitation, attempt to retrieve the invitation, - which will be successful if a single User exists for that email; otherwise, will - just continue to create the invitation. - """ - if not change and User.objects.filter(email=obj.email).count() == 1: - # Domain Invitation creation for an existing User - obj.retrieve() - # Call the parent save method to save the object - super().save_model(request, obj, form, change) - - -class PortfolioInvitationAdmin(ListHeaderAdmin): - """Custom portfolio invitation admin class.""" - - form = PortfolioInvitationAdminForm - - class Meta: - model = models.PortfolioInvitation - fields = "__all__" - - _meta = Meta() - - # Columns - list_display = [ - "email", - "portfolio", - "roles", - "additional_permissions", - "status", - ] - - # Search - search_fields = [ - "email", - "portfolio__name", - ] - - # Filters - list_filter = ("status",) - - search_help_text = "Search by email or portfolio." - - # Mark the FSM field 'status' as readonly - # to allow admin users to create Domain Invitations - # without triggering the FSM Transition Not Allowed - # error. - readonly_fields = ["status"] - - autocomplete_fields = ["portfolio"] - - 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): - if extra_context is None: - extra_context = {} - extra_context["tabtitle"] = "Portfolio invitations" - # 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.") +class BaseInvitationAdmin(ListHeaderAdmin): + """Base class for admin classes which will customize save_model and send email invitations + on model adds, and require custom handling of forms and form errors.""" def response_add(self, request, obj, post_url_continue=None): """ @@ -1560,8 +1407,9 @@ class PortfolioInvitationAdmin(ListHeaderAdmin): 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 + # store current messages from request so that they are preserved throughout the method storage = get_messages(request) + # Check if there are any error or warning messages in the `messages` framework has_errors = any(message.level_tag in ["error", "warning"] for message in storage) if has_errors: @@ -1608,7 +1456,206 @@ class PortfolioInvitationAdmin(ListHeaderAdmin): change=False, obj=obj, ) - return super().response_add(request, obj, post_url_continue) + + response = super().response_add(request, obj, post_url_continue) + + # Re-add all messages from storage after `super().response_add` + # as super().response_add resets the success messages in request + for message in storage: + messages.add_message(request, message.level, message.message) + + return response + + +class DomainInvitationAdmin(BaseInvitationAdmin): + """Custom domain invitation admin class.""" + + class Meta: + model = models.DomainInvitation + fields = "__all__" + + _meta = Meta() + + # Columns + list_display = [ + "email", + "domain", + "status", + ] + + # Search + search_fields = [ + "email", + "domain__name", + ] + + # Filters + list_filter = ("status",) + + search_help_text = "Search by email or domain." + + # Mark the FSM field 'status' as readonly + # to allow admin users to create Domain Invitations + # without triggering the FSM Transition Not Allowed + # error. + readonly_fields = ["status"] + + autocomplete_fields = ["domain"] + + change_form_template = "django/admin/domain_invitation_change_form.html" + + # Select domain invitations to change -> Domain invitations + def changelist_view(self, request, extra_context=None): + if extra_context is None: + extra_context = {} + extra_context["tabtitle"] = "Domain invitations" + # 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. + + On creation of a new domain invitation, attempt to retrieve the invitation, + which will be successful if a single User exists for that email; otherwise, will + just continue to create the invitation. + """ + if not change: + domain = obj.domain + domain_org = getattr(domain.domain_info, "portfolio", None) + requested_email = obj.email + # Look up a user with that email + requested_user = get_requested_user(requested_email) + requestor = request.user + + member_of_a_different_org, member_of_this_org = get_org_membership( + domain_org, requested_email, requested_user + ) + + try: + if ( + flag_is_active(request, "organization_feature") + and not flag_is_active(request, "multiple_portfolios") + and domain_org is not None + and not member_of_this_org + and not member_of_a_different_org + ): + send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=domain_org) + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create( + email=requested_email, + portfolio=domain_org, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + # if user exists for email, immediately retrieve portfolio invitation upon creation + if requested_user is not None: + portfolio_invitation.retrieve() + portfolio_invitation.save() + messages.success(request, f"{requested_email} has been invited to the organization: {domain_org}") + + send_domain_invitation_email( + email=requested_email, + requestor=requestor, + domains=domain, + is_member_of_different_org=member_of_a_different_org, + requested_user=requested_user, + ) + if requested_user is not None: + # Domain Invitation creation for an existing User + obj.retrieve() + # Call the parent save method to save the object + super().save_model(request, obj, form, change) + messages.success(request, f"{requested_email} has been invited to the domain: {domain}") + except Exception as e: + handle_invitation_exceptions(request, e, requested_email) + return + else: + # Call the parent save method to save the object + super().save_model(request, obj, form, change) + + +class PortfolioInvitationAdmin(BaseInvitationAdmin): + """Custom portfolio invitation admin class.""" + + form = PortfolioInvitationAdminForm + + class Meta: + model = models.PortfolioInvitation + fields = "__all__" + + _meta = Meta() + + # Columns + list_display = [ + "email", + "portfolio", + "roles", + "additional_permissions", + "status", + ] + + # Search + search_fields = [ + "email", + "portfolio__organization_name", + ] + + # Filters + list_filter = ("status",) + + search_help_text = "Search by email or portfolio." + + # Mark the FSM field 'status' as readonly + # to allow admin users to create Domain Invitations + # without triggering the FSM Transition Not Allowed + # error. + readonly_fields = ["status"] + + autocomplete_fields = ["portfolio"] + + 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): + if extra_context is None: + extra_context = {} + extra_context["tabtitle"] = "Portfolio invitations" + # 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 + # Look up a user with that email + requested_user = get_requested_user(requested_email) + + 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) + # if user exists for email, immediately retrieve portfolio invitation upon creation + if requested_user is not None: + obj.retrieve() + 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 + handle_invitation_exceptions(request, e, requested_email) + return + # Call the parent save method to save the object + super().save_model(request, obj, form, change) class DomainInformationResource(resources.ModelResource): @@ -2782,7 +2829,9 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): try: # Retrieve and order audit log entries by timestamp in descending order - audit_log_entries = LogEntry.objects.filter(object_id=object_id).order_by("-timestamp") + audit_log_entries = LogEntry.objects.filter( + object_id=object_id, content_type__model="domainrequest" + ).order_by("-timestamp") # Process each log entry to filter based on the change criteria for log_entry in audit_log_entries: diff --git a/src/registrar/assets/js/uswds-edited.js b/src/registrar/assets/js/uswds-edited.js index f59417b41..9d4dd2e51 100644 --- a/src/registrar/assets/js/uswds-edited.js +++ b/src/registrar/assets/js/uswds-edited.js @@ -29,6 +29,7 @@ * - tooltip dynamic content updated to include nested element (for better sizing control) * - modal exposed to window to be accessible in other js files * - fixed bug in createHeaderButton which added newlines to header button tooltips + * - modified combobox to handle error class */ if ("document" in window.self) { @@ -1213,6 +1214,11 @@ const enhanceComboBox = _comboBoxEl => { input.setAttribute("class", INPUT_CLASS); input.setAttribute("type", "text"); input.setAttribute("role", "combobox"); + // DOTGOV - handle error class for combobox + // Check if 'usa-input--error' exists in selectEl and add it to input if true + if (selectEl.classList.contains('usa-input--error')) { + input.classList.add('usa-input--error'); + } additionalAttributes.forEach(attr => Object.keys(attr).forEach(key => { const value = Sanitizer.escapeHTML`${attr[key]}`; input.setAttribute(key, value); diff --git a/src/registrar/assets/src/js/getgov/combobox.js b/src/registrar/assets/src/js/getgov/combobox.js deleted file mode 100644 index 36b7aa0ad..000000000 --- a/src/registrar/assets/src/js/getgov/combobox.js +++ /dev/null @@ -1,113 +0,0 @@ -import { hideElement, showElement } from './helpers.js'; - -export function loadInitialValuesForComboBoxes() { - var overrideDefaultClearButton = true; - var isTyping = false; - - document.addEventListener('DOMContentLoaded', (event) => { - handleAllComboBoxElements(); - }); - - function handleAllComboBoxElements() { - const comboBoxElements = document.querySelectorAll(".usa-combo-box"); - comboBoxElements.forEach(comboBox => { - const input = comboBox.querySelector("input"); - const select = comboBox.querySelector("select"); - if (!input || !select) { - console.warn("No combobox element found"); - return; - } - // Set the initial value of the combobox - let initialValue = select.getAttribute("data-default-value"); - let clearInputButton = comboBox.querySelector(".usa-combo-box__clear-input"); - if (!clearInputButton) { - console.warn("No clear element found"); - return; - } - - // Override the default clear button behavior such that it no longer clears the input, - // it just resets to the data-initial-value. - // Due to the nature of how uswds works, this is slightly hacky. - // Use a MutationObserver to watch for changes in the dropdown list - const dropdownList = comboBox.querySelector(`#${input.id}--list`); - const observer = new MutationObserver(function(mutations) { - mutations.forEach(function(mutation) { - if (mutation.type === "childList") { - addBlankOption(clearInputButton, dropdownList, initialValue); - } - }); - }); - - // Configure the observer to watch for changes in the dropdown list - const config = { childList: true, subtree: true }; - observer.observe(dropdownList, config); - - // Input event listener to detect typing - input.addEventListener("input", () => { - isTyping = true; - }); - - // Blur event listener to reset typing state - input.addEventListener("blur", () => { - isTyping = false; - }); - - // Hide the reset button when there is nothing to reset. - // Do this once on init, then everytime a change occurs. - updateClearButtonVisibility(select, initialValue, clearInputButton) - select.addEventListener("change", () => { - updateClearButtonVisibility(select, initialValue, clearInputButton) - }); - - // Change the default input behaviour - have it reset to the data default instead - clearInputButton.addEventListener("click", (e) => { - if (overrideDefaultClearButton && initialValue) { - e.preventDefault(); - e.stopPropagation(); - input.click(); - // Find the dropdown option with the desired value - const dropdownOptions = document.querySelectorAll(".usa-combo-box__list-option"); - if (dropdownOptions) { - dropdownOptions.forEach(option => { - if (option.getAttribute("data-value") === initialValue) { - // Simulate a click event on the dropdown option - option.click(); - } - }); - } - } - }); - }); - } - - function updateClearButtonVisibility(select, initialValue, clearInputButton) { - if (select.value === initialValue) { - hideElement(clearInputButton); - }else { - showElement(clearInputButton) - } - } - - function addBlankOption(clearInputButton, dropdownList, initialValue) { - if (dropdownList && !dropdownList.querySelector('[data-value=""]') && !isTyping) { - const blankOption = document.createElement("li"); - blankOption.setAttribute("role", "option"); - blankOption.setAttribute("data-value", ""); - blankOption.classList.add("usa-combo-box__list-option"); - if (!initialValue){ - blankOption.classList.add("usa-combo-box__list-option--selected") - } - blankOption.textContent = "⎯"; - - dropdownList.insertBefore(blankOption, dropdownList.firstChild); - blankOption.addEventListener("click", (e) => { - e.preventDefault(); - e.stopPropagation(); - overrideDefaultClearButton = false; - // Trigger the default clear behavior - clearInputButton.click(); - overrideDefaultClearButton = true; - }); - } - } -} diff --git a/src/registrar/assets/src/js/getgov/helpers-uswds.js b/src/registrar/assets/src/js/getgov/helpers-uswds.js index 129d578b6..eec7b0818 100644 --- a/src/registrar/assets/src/js/getgov/helpers-uswds.js +++ b/src/registrar/assets/src/js/getgov/helpers-uswds.js @@ -4,7 +4,7 @@ * accessible directly in getgov.min.js * */ -export function initializeTooltips() { +export function uswdsInitializeTooltips() { function checkTooltip() { // Check that the tooltip library is loaded, and if not, wait and retry if (window.tooltip && typeof window.tooltip.init === 'function') { diff --git a/src/registrar/assets/src/js/getgov/main.js b/src/registrar/assets/src/js/getgov/main.js index 6ff402aa4..a077da929 100644 --- a/src/registrar/assets/src/js/getgov/main.js +++ b/src/registrar/assets/src/js/getgov/main.js @@ -3,7 +3,6 @@ import { initDomainValidators } from './domain-validators.js'; import { initFormsetsForms, triggerModalOnDsDataForm, nameserversFormListener } from './formset-forms.js'; import { initializeUrbanizationToggle } from './urbanization.js'; import { userProfileListener, finishUserSetupListener } from './user-profile.js'; -import { loadInitialValuesForComboBoxes } from './combobox.js'; import { handleRequestingEntityFieldset } from './requesting-entity.js'; import { initDomainsTable } from './table-domains.js'; import { initDomainRequestsTable } from './table-domain-requests.js'; @@ -31,8 +30,6 @@ initializeUrbanizationToggle(); userProfileListener(); finishUserSetupListener(); -loadInitialValuesForComboBoxes(); - handleRequestingEntityFieldset(); initDomainsTable(); diff --git a/src/registrar/assets/src/js/getgov/requesting-entity.js b/src/registrar/assets/src/js/getgov/requesting-entity.js index 3bcdcd35c..833eab2f8 100644 --- a/src/registrar/assets/src/js/getgov/requesting-entity.js +++ b/src/registrar/assets/src/js/getgov/requesting-entity.js @@ -9,15 +9,15 @@ export function handleRequestingEntityFieldset() { const formPrefix = "portfolio_requesting_entity"; const radioFieldset = document.getElementById(`id_${formPrefix}-requesting_entity_is_suborganization__fieldset`); const radios = radioFieldset?.querySelectorAll(`input[name="${formPrefix}-requesting_entity_is_suborganization"]`); - const select = document.getElementById(`id_${formPrefix}-sub_organization`); - const selectParent = select?.parentElement; + const input = document.getElementById(`id_${formPrefix}-sub_organization`); + const inputGrandParent = input?.parentElement?.parentElement; + const select = input?.previousElementSibling; const suborgContainer = document.getElementById("suborganization-container"); const suborgDetailsContainer = document.getElementById("suborganization-container__details"); const suborgAddtlInstruction = document.getElementById("suborganization-addtl-instruction"); - const subOrgCreateNewOption = document.getElementById("option-to-add-suborg")?.value; // Make sure all crucial page elements exist before proceeding. // This more or less ensures that we are on the Requesting Entity page, and not elsewhere. - if (!radios || !select || !selectParent || !suborgContainer || !suborgDetailsContainer) return; + if (!radios || !input || !select || !inputGrandParent || !suborgContainer || !suborgDetailsContainer) return; // requestingSuborganization: This just broadly determines if they're requesting a suborg at all // requestingNewSuborganization: This variable determines if the user is trying to *create* a new suborganization or not. @@ -27,8 +27,8 @@ export function handleRequestingEntityFieldset() { function toggleSuborganization(radio=null) { if (radio != null) requestingSuborganization = radio?.checked && radio.value === "True"; requestingSuborganization ? showElement(suborgContainer) : hideElement(suborgContainer); - if (select.options.length == 2) { // --Select-- and other are the only options - hideElement(selectParent); // Hide the select drop down and indicate requesting new suborg + if (select.options.length == 1) { // other is the only option + hideElement(inputGrandParent); // Hide the combo box and indicate requesting new suborg hideElement(suborgAddtlInstruction); // Hide additional instruction related to the list requestingNewSuborganization.value = "True"; } else { @@ -37,11 +37,6 @@ export function handleRequestingEntityFieldset() { requestingNewSuborganization.value === "True" ? showElement(suborgDetailsContainer) : hideElement(suborgDetailsContainer); } - // Add fake "other" option to sub_organization select - if (select && !Array.from(select.options).some(option => option.value === "other")) { - select.add(new Option(subOrgCreateNewOption, "other")); - } - if (requestingNewSuborganization.value === "True") { select.value = "other"; } diff --git a/src/registrar/assets/src/js/getgov/table-base.js b/src/registrar/assets/src/js/getgov/table-base.js index e1d5c11ce..338d5d98c 100644 --- a/src/registrar/assets/src/js/getgov/table-base.js +++ b/src/registrar/assets/src/js/getgov/table-base.js @@ -375,6 +375,13 @@ export class BaseTable { */ loadModals(page, total, unfiltered_total) {} + /** + * Loads tooltips + sets up event listeners + * "Activates" the tooltips after the DOM updates + * Utilizes "uswdsInitializeTooltips" + */ + initializeTooltips() {} + /** * Allows us to customize the table display based on specific conditions and a user's permissions * Dynamically manages the visibility set up of columns, adding/removing headers @@ -382,7 +389,7 @@ export class BaseTable { * for a member, they will also see the kebab column) * @param {Object} dataObjects - Data which contains info on domain requests or a user's permission * Currently returns a dictionary of either: - * - "needsAdditionalColumn": If a new column should be displayed + * - "hasAdditionalActions": If additional elements need to be added to the Action column * - "UserPortfolioPermissionChoices": A user's portfolio permission choices */ customizeTable(dataObjects){ return {}; } @@ -406,7 +413,7 @@ export class BaseTable { * Returns either: data.members, data.domains or data.domain_requests * @param {Object} dataObject - The data used to populate the row content * @param {HTMLElement} tbody - The table body to which the new row is appended to - * @param {Object} customTableOptions - Additional options for customizing row appearance (ie needsAdditionalColumn) + * @param {Object} customTableOptions - Additional options for customizing row appearance (ie hasAdditionalActions) */ addRow(dataObject, tbody, customTableOptions) { throw new Error('addRow must be defined'); @@ -471,6 +478,7 @@ export class BaseTable { this.initCheckboxListeners(); this.loadModals(data.page, data.total, data.unfiltered_total); + this.initializeTooltips(); // Do not scroll on first page load if (scroll) diff --git a/src/registrar/assets/src/js/getgov/table-domain-requests.js b/src/registrar/assets/src/js/getgov/table-domain-requests.js index 51e4ea12b..f667a96b5 100644 --- a/src/registrar/assets/src/js/getgov/table-domain-requests.js +++ b/src/registrar/assets/src/js/getgov/table-domain-requests.js @@ -52,26 +52,8 @@ export class DomainRequestsTable extends BaseTable { // Manage "export as CSV" visibility for domain requests this.toggleExportButton(data.domain_requests); - let needsDeleteColumn = data.domain_requests.some(request => request.is_deletable); - - // Remove existing delete th and td if they exist - let existingDeleteTh = document.querySelector('.delete-header'); - if (!needsDeleteColumn) { - if (existingDeleteTh) - existingDeleteTh.remove(); - } else { - if (!existingDeleteTh) { - const delheader = document.createElement('th'); - delheader.setAttribute('scope', 'col'); - delheader.setAttribute('role', 'columnheader'); - delheader.setAttribute('class', 'delete-header width-5'); - delheader.innerHTML = ` - Delete Action`; - let tableHeaderRow = this.tableWrapper.querySelector('thead tr'); - tableHeaderRow.appendChild(delheader); - } - } - return { 'needsAdditionalColumn': needsDeleteColumn }; + let isDeletable = data.domain_requests.some(request => request.is_deletable); + return { 'hasAdditionalActions': isDeletable }; } addRow(dataObject, tbody, customTableOptions) { @@ -88,6 +70,7 @@ export class DomainRequestsTable extends BaseTable { Domain request cannot be deleted now. Edit the request for more information.`; let markupCreatorRow = ''; + if (this.portfolioValue) { markupCreatorRow = ` @@ -98,7 +81,7 @@ export class DomainRequestsTable extends BaseTable { } if (request.is_deletable) { - // 1st path: Just a modal trigger in any screen size for non-org users + // 1st path (non-org): Just a modal trigger in any screen size for non-org users modalTrigger = ` ${request.status} - - - - ${actionLabel} ${request.requested_domain ? request.requested_domain : 'New domain request'} - + +
+ + + ${actionLabel} ${request.requested_domain ? request.requested_domain : 'New domain request'} + + ${customTableOptions.hasAdditionalActions ? modalTrigger : ''} +
- ${customTableOptions.needsAdditionalColumn ? ''+modalTrigger+'' : ''} `; tbody.appendChild(row); if (request.is_deletable) DomainRequestsTable.addDomainRequestsModal(request.requested_domain, request.id, request.created_at, tbody); diff --git a/src/registrar/assets/src/js/getgov/table-domains.js b/src/registrar/assets/src/js/getgov/table-domains.js index a6373a5c2..3102484cf 100644 --- a/src/registrar/assets/src/js/getgov/table-domains.js +++ b/src/registrar/assets/src/js/getgov/table-domains.js @@ -1,4 +1,5 @@ import { BaseTable } from './table-base.js'; +import { uswdsInitializeTooltips } from './helpers-uswds.js'; export class DomainsTable extends BaseTable { @@ -55,7 +56,7 @@ export class DomainsTable extends BaseTable { ${markupForSuborganizationRow} - + ${member.name} - ${customTableOptions.needsAdditionalColumn ? ''+kebabHTML+'' : ''} + ${customTableOptions.hasAdditionalActions ? ''+kebabHTML+'' : ''} `; tbody.appendChild(row); if (domainsHTML || permissionsHTML) { @@ -137,7 +137,7 @@ export class MembersTable extends BaseTable { } // This easter egg is only for fixtures that dont have names as we are displaying their emails // All prod users will have emails linked to their account - if (customTableOptions.needsAdditionalColumn) MembersTable.addMemberDeleteModal(num_domains, member.email || "Samwise Gamgee", member_delete_url, unique_id, row); + if (customTableOptions.hasAdditionalActions) MembersTable.addMemberDeleteModal(num_domains, member.email || "Samwise Gamgee", member_delete_url, unique_id, row); } /** diff --git a/src/registrar/assets/src/sass/_theme/_base.scss b/src/registrar/assets/src/sass/_theme/_base.scss index d73becd75..97f5710e9 100644 --- a/src/registrar/assets/src/sass/_theme/_base.scss +++ b/src/registrar/assets/src/sass/_theme/_base.scss @@ -281,4 +281,8 @@ abbr[title] { .maxw-fit-content { max-width: fit-content; +} + +.width-quarter { + width: 25%; } \ No newline at end of file diff --git a/src/registrar/assets/src/sass/_theme/_tooltips.scss b/src/registrar/assets/src/sass/_theme/_tooltips.scss index 58beb8ae6..65bfbb483 100644 --- a/src/registrar/assets/src/sass/_theme/_tooltips.scss +++ b/src/registrar/assets/src/sass/_theme/_tooltips.scss @@ -66,9 +66,9 @@ text-align: center; font-size: inherit; //inherit tooltip fontsize of .93rem max-width: fit-content; + display: block; @include at-media('desktop') { width: 70vw; } - display: block; } } \ No newline at end of file diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index ef5f7f40a..beb38e104 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -345,6 +345,11 @@ urlpatterns = [ views.DomainSecurityEmailView.as_view(), name="domain-security-email", ), + path( + "domain//renewal", + views.DomainRenewalView.as_view(), + name="domain-renewal", + ), path( "domain//users/add", views.DomainAddUserView.as_view(), diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index 121e2b3f7..13725f109 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -10,6 +10,7 @@ from .domain import ( DomainDsdataFormset, DomainDsdataForm, DomainSuborganizationForm, + DomainRenewalForm, ) from .portfolio import ( PortfolioOrgAddressForm, diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index b43d91a58..05eb90db3 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -4,6 +4,7 @@ import logging from django import forms from django.core.validators import MinValueValidator, MaxValueValidator, RegexValidator, MaxLengthValidator from django.forms import formset_factory +from registrar.forms.utility.combobox import ComboboxWidget from registrar.models import DomainRequest, FederalAgency from phonenumber_field.widgets import RegionalPhoneNumberWidget from registrar.models.suborganization import Suborganization @@ -161,9 +162,10 @@ class DomainSuborganizationForm(forms.ModelForm): """Form for updating the suborganization""" sub_organization = forms.ModelChoiceField( + label="Suborganization name", queryset=Suborganization.objects.none(), required=False, - widget=forms.Select(), + widget=ComboboxWidget, ) class Meta: @@ -178,20 +180,6 @@ class DomainSuborganizationForm(forms.ModelForm): portfolio = self.instance.portfolio if self.instance else None self.fields["sub_organization"].queryset = Suborganization.objects.filter(portfolio=portfolio) - # Set initial value - if self.instance and self.instance.sub_organization: - self.fields["sub_organization"].initial = self.instance.sub_organization - - # Set custom form label - self.fields["sub_organization"].label = "Suborganization name" - - # Use the combobox rather than the regular select widget - self.fields["sub_organization"].widget.template_name = "django/forms/widgets/combobox.html" - - # Set data-default-value attribute - if self.instance and self.instance.sub_organization: - self.fields["sub_organization"].widget.attrs["data-default-value"] = self.instance.sub_organization.pk - class BaseNameserverFormset(forms.BaseFormSet): def clean(self): @@ -456,6 +444,13 @@ class DomainSecurityEmailForm(forms.Form): class DomainOrgNameAddressForm(forms.ModelForm): """Form for updating the organization name and mailing address.""" + # for federal agencies we also want to know the top-level agency. + federal_agency = forms.ModelChoiceField( + label="Federal agency", + required=False, + queryset=FederalAgency.objects.all(), + widget=ComboboxWidget, + ) zipcode = forms.CharField( label="Zip code", validators=[ @@ -469,6 +464,16 @@ class DomainOrgNameAddressForm(forms.ModelForm): }, ) + state_territory = forms.ChoiceField( + label="State, territory, or military post", + required=True, + choices=DomainInformation.StateTerritoryChoices.choices, + error_messages={ + "required": ("Select the state, territory, or military post where your organization is located.") + }, + widget=ComboboxWidget(attrs={"required": True}), + ) + class Meta: model = DomainInformation fields = [ @@ -486,25 +491,12 @@ class DomainOrgNameAddressForm(forms.ModelForm): "organization_name": {"required": "Enter the name of your organization."}, "address_line1": {"required": "Enter the street address of your organization."}, "city": {"required": "Enter the city where your organization is located."}, - "state_territory": { - "required": "Select the state, territory, or military post where your organization is located." - }, } widgets = { - # We need to set the required attributed for State/territory - # because for this fields we are creating an individual - # instance of the Select. For the other fields we use the for loop to set - # the class's required attribute to true. "organization_name": forms.TextInput, "address_line1": forms.TextInput, "address_line2": forms.TextInput, "city": forms.TextInput, - "state_territory": forms.Select( - attrs={ - "required": True, - }, - choices=DomainInformation.StateTerritoryChoices.choices, - ), "urbanization": forms.TextInput, } @@ -661,3 +653,15 @@ DomainDsdataFormset = formset_factory( extra=0, can_delete=True, ) + + +class DomainRenewalForm(forms.Form): + """Form making sure domain renewal ack is checked""" + + is_policy_acknowledged = forms.BooleanField( + required=True, + label="I have read and agree to the requirements for operating a .gov domain.", + error_messages={ + "required": "Check the box if you read and agree to the requirements for operating a .gov domain." + }, + ) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index ccdbb17ba..7c9dcb180 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -7,6 +7,7 @@ from django import forms from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe +from registrar.forms.utility.combobox import ComboboxWidget from registrar.forms.utility.wizard_form_helper import ( RegistrarForm, RegistrarFormSet, @@ -43,7 +44,7 @@ class RequestingEntityForm(RegistrarForm): label="Suborganization name", required=False, queryset=Suborganization.objects.none(), - empty_label="--Select--", + widget=ComboboxWidget, ) requested_suborganization = forms.CharField( label="Requested suborganization", @@ -56,22 +57,44 @@ class RequestingEntityForm(RegistrarForm): suborganization_state_territory = forms.ChoiceField( label="State, territory, or military post", required=False, - choices=[("", "--Select--")] + DomainRequest.StateTerritoryChoices.choices, + choices=DomainRequest.StateTerritoryChoices.choices, + widget=ComboboxWidget, ) def __init__(self, *args, **kwargs): - """Override of init to add the suborganization queryset""" + """Override of init to add the suborganization queryset and 'other' option""" super().__init__(*args, **kwargs) if self.domain_request.portfolio: - self.fields["sub_organization"].queryset = Suborganization.objects.filter( - portfolio=self.domain_request.portfolio - ) + # Fetch the queryset for the portfolio + queryset = Suborganization.objects.filter(portfolio=self.domain_request.portfolio) + # set the queryset appropriately so that post can validate against queryset + self.fields["sub_organization"].queryset = queryset + + # Modify the choices to include "other" so that form can display options properly + self.fields["sub_organization"].choices = [(obj.id, str(obj)) for obj in queryset] + [ + ("other", "Other (enter your suborganization manually)") + ] + + @classmethod + def from_database(cls, obj: DomainRequest | Contact | None): + """Returns a dict of form field values gotten from `obj`. + Overrides RegistrarForm method in order to set sub_organization to 'other' + on GETs of the RequestingEntityForm.""" + if obj is None: + return {} + # get the domain request as a dict, per usual method + domain_request_dict = {name: getattr(obj, name) for name in cls.declared_fields.keys()} # type: ignore + + # set sub_organization to 'other' if is_requesting_new_suborganization is True + if isinstance(obj, DomainRequest) and obj.is_requesting_new_suborganization(): + domain_request_dict["sub_organization"] = "other" + + return domain_request_dict def clean_sub_organization(self): """On suborganization clean, set the suborganization value to None if the user is requesting a custom suborganization (as it doesn't exist yet)""" - # If it's a new suborganization, return None (equivalent to selecting nothing) if self.cleaned_data.get("is_requesting_new_suborganization"): return None @@ -94,41 +117,60 @@ class RequestingEntityForm(RegistrarForm): return name def full_clean(self): - """Validation logic to remove the custom suborganization value before clean is triggered. + """Validation logic to temporarily remove the custom suborganization value before clean is triggered. Without this override, the form will throw an 'invalid option' error.""" - # Remove the custom other field before cleaning - data = self.data.copy() if self.data else None + # Ensure self.data is not None before proceeding + if self.data: + # handle case where form has been submitted + # Create a copy of the data for manipulation + data = self.data.copy() - # Remove the 'other' value from suborganization if it exists. - # This is a special value that tracks if the user is requesting a new suborg. - suborganization = self.data.get("portfolio_requesting_entity-sub_organization") - if suborganization and "other" in suborganization: - data["portfolio_requesting_entity-sub_organization"] = "" + # Retrieve sub_organization and store in _original_suborganization + suborganization = data.get("portfolio_requesting_entity-sub_organization") + self._original_suborganization = suborganization + # If the original value was "other", clear it for validation + if self._original_suborganization == "other": + data["portfolio_requesting_entity-sub_organization"] = "" - # Set the modified data back to the form - self.data = data + # Set the modified data back to the form + self.data = data + else: + # handle case of a GET + suborganization = None + if self.initial and "sub_organization" in self.initial: + suborganization = self.initial["sub_organization"] + + # Check if is_requesting_new_suborganization is True + is_requesting_new_suborganization = False + if self.initial and "is_requesting_new_suborganization" in self.initial: + # Call the method if it exists + is_requesting_new_suborganization = self.initial["is_requesting_new_suborganization"]() + + # Determine if "other" should be set + if is_requesting_new_suborganization and suborganization is None: + self._original_suborganization = "other" + else: + self._original_suborganization = suborganization # Call the parent's full_clean method super().full_clean() + # Restore "other" if there are errors + if self.errors: + self.data["portfolio_requesting_entity-sub_organization"] = self._original_suborganization + def clean(self): - """Custom clean implementation to handle our desired logic flow for suborganization. - Given that these fields often rely on eachother, we need to do this in the parent function.""" + """Custom clean implementation to handle our desired logic flow for suborganization.""" cleaned_data = super().clean() - # Do some custom error validation if the requesting entity is a suborg. - # Otherwise, just validate as normal. - suborganization = self.cleaned_data.get("sub_organization") - is_requesting_new_suborganization = self.cleaned_data.get("is_requesting_new_suborganization") - - # Get the value of the yes/no checkbox from RequestingEntityYesNoForm. - # Since self.data stores this as a string, we need to convert "True" => True. + # Get the cleaned data + suborganization = cleaned_data.get("sub_organization") + is_requesting_new_suborganization = cleaned_data.get("is_requesting_new_suborganization") requesting_entity_is_suborganization = self.data.get( "portfolio_requesting_entity-requesting_entity_is_suborganization" ) if requesting_entity_is_suborganization == "True": if is_requesting_new_suborganization: - # Validate custom suborganization fields 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"): @@ -141,6 +183,12 @@ class RequestingEntityForm(RegistrarForm): elif not suborganization: self.add_error("sub_organization", "Suborganization is required.") + # If there are errors, restore the "other" value for rendering + if self.errors and getattr(self, "_original_suborganization", None) == "other": + self.cleaned_data["sub_organization"] = self._original_suborganization + elif not self.data and getattr(self, "_original_suborganization", None) == "other": + self.cleaned_data["sub_organization"] = self._original_suborganization + return cleaned_data @@ -274,7 +322,7 @@ class OrganizationContactForm(RegistrarForm): # uncomment to see if modelChoiceField can be an arg later required=False, queryset=FederalAgency.objects.exclude(agency__in=excluded_agencies), - empty_label="--Select--", + widget=ComboboxWidget, ) organization_name = forms.CharField( label="Organization name", @@ -294,10 +342,11 @@ class OrganizationContactForm(RegistrarForm): ) state_territory = forms.ChoiceField( label="State, territory, or military post", - choices=[("", "--Select--")] + DomainRequest.StateTerritoryChoices.choices, + choices=DomainRequest.StateTerritoryChoices.choices, error_messages={ "required": ("Select the state, territory, or military post where your organization is located.") }, + widget=ComboboxWidget, ) zipcode = forms.CharField( label="Zip code", diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 0a8c4d623..e57b56c4f 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -6,6 +6,7 @@ from django.core.validators import RegexValidator from django.core.validators import MaxLengthValidator from django.utils.safestring import mark_safe +from registrar.forms.utility.combobox import ComboboxWidget from registrar.models import ( PortfolioInvitation, UserPortfolioPermission, @@ -33,6 +34,15 @@ class PortfolioOrgAddressForm(forms.ModelForm): "required": "Enter a 5-digit or 9-digit zip code, like 12345 or 12345-6789.", }, ) + state_territory = forms.ChoiceField( + label="State, territory, or military post", + required=True, + choices=DomainInformation.StateTerritoryChoices.choices, + error_messages={ + "required": ("Select the state, territory, or military post where your organization is located.") + }, + widget=ComboboxWidget(attrs={"required": True}), + ) class Meta: model = Portfolio @@ -47,25 +57,12 @@ class PortfolioOrgAddressForm(forms.ModelForm): error_messages = { "address_line1": {"required": "Enter the street address of your organization."}, "city": {"required": "Enter the city where your organization is located."}, - "state_territory": { - "required": "Select the state, territory, or military post where your organization is located." - }, "zipcode": {"required": "Enter a 5-digit or 9-digit zip code, like 12345 or 12345-6789."}, } widgets = { - # We need to set the required attributed for State/territory - # because for this fields we are creating an individual - # instance of the Select. For the other fields we use the for loop to set - # the class's required attribute to true. "address_line1": forms.TextInput, "address_line2": forms.TextInput, "city": forms.TextInput, - "state_territory": forms.Select( - attrs={ - "required": True, - }, - choices=DomainInformation.StateTerritoryChoices.choices, - ), # "urbanization": forms.TextInput, } diff --git a/src/registrar/forms/utility/combobox.py b/src/registrar/forms/utility/combobox.py new file mode 100644 index 000000000..277aec4f3 --- /dev/null +++ b/src/registrar/forms/utility/combobox.py @@ -0,0 +1,5 @@ +from django.forms import Select + + +class ComboboxWidget(Select): + template_name = "django/forms/widgets/combobox.html" diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 9cf4d36ea..c56b4ff6b 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -5,6 +5,8 @@ import logging from django.core.management import BaseCommand, CommandError from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User +from registrar.models.utility.generic_helper import normalize_string +from django.db.models import F, Q logger = logging.getLogger(__name__) @@ -21,10 +23,21 @@ class Command(BaseCommand): self.failed_portfolios = set() def add_arguments(self, parser): - """Add three arguments: - 1. agency_name => the value of FederalAgency.agency - 2. --parse_requests => if true, adds the given portfolio to each related DomainRequest - 3. --parse_domains => if true, adds the given portfolio to each related DomainInformation + """Add command line arguments to create federal portfolios. + + Required (mutually exclusive) arguments: + --agency_name: Name of a specific FederalAgency to create a portfolio for + --branch: Federal branch to process ("executive", "legislative", or "judicial"). + Creates portfolios for all FederalAgencies in that branch. + + Required (at least one): + --parse_requests: Add the created portfolio(s) to related DomainRequest records + --parse_domains: Add the created portfolio(s) to related DomainInformation records + Note: You can use both --parse_requests and --parse_domains together + + Optional (mutually exclusive with parse options): + --both: Shorthand for using both --parse_requests and --parse_domains + Cannot be used with --parse_requests or --parse_domains """ group = parser.add_mutually_exclusive_group(required=True) group.add_argument( @@ -78,39 +91,101 @@ class Command(BaseCommand): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") + portfolios = [] for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: # C901 'Command.handle' is too complex (12) - self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) + portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) + portfolios.append(portfolio) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) message = f"Failed to create portfolio '{federal_agency.agency}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.FAIL, message) + # POST PROCESS STEP: Add additional suborg info where applicable. + updated_suborg_count = self.post_process_all_suborganization_fields(agencies) + message = f"Added city and state_territory information to {updated_suborg_count} suborgs." + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + TerminalHelper.log_script_run_summary( self.updated_portfolios, self.failed_portfolios, self.skipped_portfolios, debug=False, - skipped_header="----- SOME PORTFOLIOS WERE SKIPPED -----", + skipped_header="----- SOME PORTFOLIOS WERENT CREATED -----", display_as_str=True, ) + # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. + # We only do this for started domain requests. + if parse_requests or both: + TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + prompt_message="This action will update domain requests even if they aren't on a portfolio.", + prompt_title=( + "POST PROCESS STEP: Do you want to clear federal agency on (related) started domain requests?" + ), + verify_message=None, + ) + self.post_process_started_domain_requests(agencies, portfolios) + + def post_process_started_domain_requests(self, agencies, portfolios): + """ + Removes duplicate organization data by clearing federal_agency when it matches the portfolio name. + Only processes domain requests in STARTED status. + """ + message = "Removing duplicate portfolio and federal_agency values from domain requests..." + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + + # For each request, clear the federal agency under these conditions: + # 1. A portfolio *already exists* with the same name as the federal agency. + # 2. Said portfolio (or portfolios) are only the ones specified at the start of the script. + # 3. The domain request is in status "started". + # Note: Both names are normalized so excess spaces are stripped and the string is lowercased. + domain_requests_to_update = DomainRequest.objects.filter( + federal_agency__in=agencies, + federal_agency__agency__isnull=False, + status=DomainRequest.DomainRequestStatus.STARTED, + organization_name__isnull=False, + ) + portfolio_set = {normalize_string(portfolio.organization_name) for portfolio in portfolios if portfolio} + + # Update the request, assuming the given agency name matches the portfolio name + updated_requests = [] + for req in domain_requests_to_update: + agency_name = normalize_string(req.federal_agency.agency) + if agency_name in portfolio_set: + req.federal_agency = None + updated_requests.append(req) + + # Execute the update and Log the results + if TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, + prompt_message=( + f"{len(domain_requests_to_update)} domain requests will be updated. " + f"These records will be changed: {[str(req) for req in updated_requests]}" + ), + prompt_title="Do you wish to commit this update to the database?", + ): + DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") + def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): """Attempts to create a portfolio. If successful, this function will also create new suborganizations""" - portfolio, created = self.create_portfolio(federal_agency) - if created: - self.create_suborganizations(portfolio, federal_agency) - if parse_domains or both: - self.handle_portfolio_domains(portfolio, federal_agency) + portfolio, _ = self.create_portfolio(federal_agency) + self.create_suborganizations(portfolio, federal_agency) + if parse_domains or both: + self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests or both: self.handle_portfolio_requests(portfolio, federal_agency) + return portfolio + def create_portfolio(self, federal_agency): """Creates a portfolio if it doesn't presently exist. Returns portfolio, created.""" @@ -161,7 +236,6 @@ class Command(BaseCommand): federal_agency=federal_agency, organization_name__isnull=False ) org_names = set(valid_agencies.values_list("organization_name", flat=True)) - if not org_names: message = ( "Could not add any suborganizations." @@ -172,7 +246,7 @@ class Command(BaseCommand): return # Check for existing suborgs on the current portfolio - existing_suborgs = Suborganization.objects.filter(name__in=org_names) + existing_suborgs = Suborganization.objects.filter(name__in=org_names, name__isnull=False) if existing_suborgs.exists(): message = f"Some suborganizations already exist for portfolio '{portfolio}'." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) @@ -180,9 +254,7 @@ class Command(BaseCommand): # Create new suborgs, as long as they don't exist in the db already new_suborgs = [] for name in org_names - set(existing_suborgs.values_list("name", flat=True)): - # Stored in variables due to linter wanting type information here. - portfolio_name: str = portfolio.organization_name if portfolio.organization_name is not None else "" - if name is not None and name.lower() == portfolio_name.lower(): + if normalize_string(name) == normalize_string(portfolio.organization_name): # You can use this to populate location information, when this occurs. # However, this isn't needed for now so we can skip it. message = ( @@ -229,12 +301,30 @@ class Command(BaseCommand): # Get all suborg information and store it in a dict to avoid doing a db call suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") for domain_request in domain_requests: + # Set the portfolio domain_request.portfolio = portfolio - if domain_request.organization_name in suborgs: - domain_request.sub_organization = suborgs.get(domain_request.organization_name) + + # Set suborg info + domain_request.sub_organization = suborgs.get(domain_request.organization_name, None) + if domain_request.sub_organization is None: + domain_request.requested_suborganization = normalize_string( + domain_request.organization_name, lowercase=False + ) + domain_request.suborganization_city = normalize_string(domain_request.city, lowercase=False) + domain_request.suborganization_state_territory = domain_request.state_territory + self.updated_portfolios.add(portfolio) - DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) + DomainRequest.objects.bulk_update( + domain_requests, + [ + "portfolio", + "sub_organization", + "requested_suborganization", + "suborganization_city", + "suborganization_state_territory", + ], + ) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) @@ -242,6 +332,8 @@ class Command(BaseCommand): """ Associate portfolio with domains for a federal agency. Updates all relevant domain information records. + + Returns a queryset of DomainInformation objects, or None if nothing changed. """ domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True) if not domain_infos.exists(): @@ -257,9 +349,146 @@ class Command(BaseCommand): suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") for domain_info in domain_infos: domain_info.portfolio = portfolio - if domain_info.organization_name in suborgs: - domain_info.sub_organization = suborgs.get(domain_info.organization_name) + domain_info.sub_organization = suborgs.get(domain_info.organization_name, None) DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + + def post_process_all_suborganization_fields(self, agencies): + """Batch updates suborganization locations from domain and request data. + + Args: + agencies: List of FederalAgency objects to process + + Returns: + int: Number of suborganizations updated + + Priority for location data: + 1. Domain information + 2. Domain request suborganization fields + 3. Domain request standard fields + """ + # Common filter between domaininformation / domain request. + # Filter by only the agencies we've updated thus far. + # Then, only process records without null portfolio, org name, or suborg name. + base_filter = Q( + federal_agency__in=agencies, + portfolio__isnull=False, + organization_name__isnull=False, + sub_organization__isnull=False, + ) & ~Q(organization_name__iexact=F("portfolio__organization_name")) + + # First: Remove null city / state_territory values on domain info / domain requests. + # We want to add city data if there is data to add to begin with! + domains = DomainInformation.objects.filter( + base_filter, + Q(city__isnull=False, state_territory__isnull=False), + ) + requests = DomainRequest.objects.filter( + base_filter, + ( + Q(city__isnull=False, state_territory__isnull=False) + | Q(suborganization_city__isnull=False, suborganization_state_territory__isnull=False) + ), + ) + + # Second: Group domains and requests by normalized organization name. + # This means that later down the line we have to account for "duplicate" org names. + domains_dict = {} + requests_dict = {} + for domain in domains: + normalized_name = normalize_string(domain.organization_name) + domains_dict.setdefault(normalized_name, []).append(domain) + + for request in requests: + normalized_name = normalize_string(request.organization_name) + requests_dict.setdefault(normalized_name, []).append(request) + + # Third: Get suborganizations to update + suborgs_to_edit = Suborganization.objects.filter( + Q(id__in=domains.values_list("sub_organization", flat=True)) + | Q(id__in=requests.values_list("sub_organization", flat=True)) + ) + + # Fourth: Process each suborg to add city / state territory info + for suborg in suborgs_to_edit: + self.post_process_suborganization_fields(suborg, domains_dict, requests_dict) + + # Fifth: Perform a bulk update + return Suborganization.objects.bulk_update(suborgs_to_edit, ["city", "state_territory"]) + + def post_process_suborganization_fields(self, suborg, domains_dict, requests_dict): + """Updates a single suborganization's location data if valid. + + Args: + suborg: Suborganization to update + domains_dict: Dict of domain info records grouped by org name + requests_dict: Dict of domain requests grouped by org name + + Priority matches parent method. Updates are skipped if location data conflicts + between multiple records of the same type. + """ + normalized_suborg_name = normalize_string(suborg.name) + domains = domains_dict.get(normalized_suborg_name, []) + requests = requests_dict.get(normalized_suborg_name, []) + + # Try to get matching domain info + domain = None + if domains: + reference = domains[0] + use_location_for_domain = all( + d.city == reference.city and d.state_territory == reference.state_territory for d in domains + ) + if use_location_for_domain: + domain = reference + + # Try to get matching request info + # Uses consensus: if all city / state_territory info matches, then we can assume the data is "good". + # If not, take the safe route and just skip updating this particular record. + request = None + use_suborg_location_for_request = True + use_location_for_request = True + if requests: + reference = requests[0] + use_suborg_location_for_request = all( + r.suborganization_city + and r.suborganization_state_territory + and r.suborganization_city == reference.suborganization_city + and r.suborganization_state_territory == reference.suborganization_state_territory + for r in requests + ) + use_location_for_request = all( + r.city + and r.state_territory + and r.city == reference.city + and r.state_territory == reference.state_territory + for r in requests + ) + if use_suborg_location_for_request or use_location_for_request: + request = reference + + if not domain and not request: + message = f"Skipping adding city / state_territory information to suborg: {suborg}. Bad data." + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + return + + # PRIORITY: + # 1. Domain info + # 2. Domain request requested suborg fields + # 3. Domain request normal fields + if domain: + suborg.city = normalize_string(domain.city, lowercase=False) + suborg.state_territory = domain.state_territory + elif request and use_suborg_location_for_request: + suborg.city = normalize_string(request.suborganization_city, lowercase=False) + suborg.state_territory = request.suborganization_state_territory + elif request and use_location_for_request: + suborg.city = normalize_string(request.city, lowercase=False) + suborg.state_territory = request.state_territory + + message = ( + f"Added city/state_territory to suborg: {suborg}. " + f"city - {suborg.city}, state - {suborg.state_territory}" + ) + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) diff --git a/src/registrar/management/commands/patch_suborganizations.py b/src/registrar/management/commands/patch_suborganizations.py new file mode 100644 index 000000000..98ff1e36f --- /dev/null +++ b/src/registrar/management/commands/patch_suborganizations.py @@ -0,0 +1,133 @@ +import logging +from django.core.management import BaseCommand +from registrar.models import Suborganization, DomainRequest, DomainInformation +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper +from registrar.models.utility.generic_helper import count_capitals, normalize_string + + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Clean up duplicate suborganizations that differ only by spaces and capitalization" + + def handle(self, **kwargs): + """Process manual deletions and find/remove duplicates. Shows preview + and updates DomainInformation / DomainRequest sub_organization references before deletion.""" + + # First: get a preset list of records we want to delete. + # For extra_records_to_prune: the key gets deleted, the value gets kept. + extra_records_to_prune = { + normalize_string("Assistant Secretary for Preparedness and Response Office of the Secretary"): { + "replace_with": "Assistant Secretary for Preparedness and Response, Office of the Secretary" + }, + normalize_string("US Geological Survey"): {"replace_with": "U.S. Geological Survey"}, + normalize_string("USDA/OC"): {"replace_with": "USDA, Office of Communications"}, + normalize_string("GSA, IC, OGP WebPortfolio"): {"replace_with": "GSA, IC, OGP Web Portfolio"}, + normalize_string("USDA/ARS/NAL"): {"replace_with": "USDA, ARS, NAL"}, + } + + # Second: loop through every Suborganization and return a dict of what to keep, and what to delete + # for each duplicate or "incorrect" record. We do this by pruning records with extra spaces or bad caps + # Note that "extra_records_to_prune" is just a manual mapping. + records_to_prune = self.get_records_to_prune(extra_records_to_prune) + if len(records_to_prune) == 0: + TerminalHelper.colorful_logger(logger.error, TerminalColors.FAIL, "No suborganizations to delete.") + return + + # Third: Build a preview of the changes + total_records_to_remove = 0 + preview_lines = ["The following records will be removed:"] + for data in records_to_prune.values(): + keep = data.get("keep") + delete = data.get("delete") + if keep: + preview_lines.append(f"Keeping: '{keep.name}' (id: {keep.id})") + + for duplicate in delete: + preview_lines.append(f"Removing: '{duplicate.name}' (id: {duplicate.id})") + total_records_to_remove += 1 + preview_lines.append("") + preview = "\n".join(preview_lines) + + # Fourth: Get user confirmation and delete + if TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + prompt_message=preview, + prompt_title=f"Remove {total_records_to_remove} suborganizations?", + verify_message="*** WARNING: This will replace the record on DomainInformation and DomainRequest! ***", + ): + try: + # Update all references to point to the right suborg before deletion + all_suborgs_to_remove = set() + for record in records_to_prune.values(): + best_record = record["keep"] + suborgs_to_remove = {dupe.id for dupe in record["delete"]} + DomainRequest.objects.filter(sub_organization_id__in=suborgs_to_remove).update( + sub_organization=best_record + ) + DomainInformation.objects.filter(sub_organization_id__in=suborgs_to_remove).update( + sub_organization=best_record + ) + all_suborgs_to_remove.update(suborgs_to_remove) + # Delete the suborgs + delete_count, _ = Suborganization.objects.filter(id__in=all_suborgs_to_remove).delete() + TerminalHelper.colorful_logger( + logger.info, TerminalColors.MAGENTA, f"Successfully deleted {delete_count} suborganizations." + ) + except Exception as e: + TerminalHelper.colorful_logger( + logger.error, TerminalColors.FAIL, f"Failed to delete suborganizations: {str(e)}" + ) + + def get_records_to_prune(self, extra_records_to_prune): + """Maps all suborgs into a dictionary with a record to keep, and an array of records to delete.""" + # First: Group all suborganization names by their "normalized" names (finding duplicates). + # Returns a dict that looks like this: + # { + # "amtrak": [, , ], + # "usda/oc": [], + # ...etc + # } + # + name_groups = {} + for suborg in Suborganization.objects.all(): + normalized_name = normalize_string(suborg.name) + name_groups.setdefault(normalized_name, []).append(suborg) + + # Second: find the record we should keep, and the records we should delete + # Returns a dict that looks like this: + # { + # "amtrak": { + # "keep": + # "delete": [, ] + # }, + # "usda/oc": { + # "keep": , + # "delete": [] + # }, + # ...etc + # } + records_to_prune = {} + for normalized_name, duplicate_suborgs in name_groups.items(): + # Delete data from our preset list + if normalized_name in extra_records_to_prune: + # The 'keep' field expects a Suborganization but we just pass in a string, so this is just a workaround. + # This assumes that there is only one item in the name_group array (see usda/oc example). + # But this should be fine, given our data. + hardcoded_record_name = extra_records_to_prune[normalized_name]["replace_with"] + name_group = name_groups.get(normalize_string(hardcoded_record_name)) + keep = name_group[0] if name_group else None + records_to_prune[normalized_name] = {"keep": keep, "delete": duplicate_suborgs} + # Delete duplicates (extra spaces or casing differences) + elif len(duplicate_suborgs) > 1: + # Pick the best record (fewest spaces, most leading capitals) + best_record = max( + duplicate_suborgs, + key=lambda suborg: (-suborg.name.count(" "), count_capitals(suborg.name, leading_only=True)), + ) + records_to_prune[normalized_name] = { + "keep": best_record, + "delete": [s for s in duplicate_suborgs if s != best_record], + } + return records_to_prune diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index eed1027f7..87d9f12e5 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -401,16 +401,15 @@ class TerminalHelper: # Allow the user to inspect the command string # and ask if they wish to proceed proceed_execution = TerminalHelper.query_yes_no_exit( - f"""{TerminalColors.OKCYAN} - ===================================================== - {prompt_title} - ===================================================== - {verify_message} - - {prompt_message} - {TerminalColors.FAIL} - Proceed? (Y = proceed, N = {action_description_for_selecting_no}) - {TerminalColors.ENDC}""" + f"\n{TerminalColors.OKCYAN}" + "=====================================================" + f"\n{prompt_title}\n" + "=====================================================" + f"\n{verify_message}\n" + f"\n{prompt_message}\n" + f"{TerminalColors.FAIL}" + f"Proceed? (Y = proceed, N = {action_description_for_selecting_no})" + f"{TerminalColors.ENDC}" ) # If the user decided to proceed return true. @@ -443,13 +442,14 @@ class TerminalHelper: f.write(file_contents) @staticmethod - def colorful_logger(log_level, color, message): + def colorful_logger(log_level, color, message, exc_info=True): """Adds some color to your log output. Args: log_level: str | Logger.method -> Desired log level. ex: logger.info or "INFO" color: str | TerminalColors -> Output color. ex: TerminalColors.YELLOW or "YELLOW" message: str -> Message to display. + exc_info: bool -> Whether the log should print exc_info or not """ if isinstance(log_level, str) and hasattr(logger, log_level.lower()): @@ -463,4 +463,4 @@ class TerminalHelper: terminal_color = color colored_message = f"{terminal_color}{message}{TerminalColors.ENDC}" - log_method(colored_message) + log_method(colored_message, exc_info=exc_info) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 6eb2fac07..cb481db7a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -4,9 +4,9 @@ import ipaddress import re from datetime import date, timedelta from typing import Optional +from django.db import transaction from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignore - -from django.db import models +from django.db import models, IntegrityError from django.utils import timezone from typing import Any from registrar.models.host import Host @@ -326,9 +326,8 @@ class Domain(TimeStampedModel, DomainHelper): exp_date = self.registry_expiration_date except KeyError: # if no expiration date from registry, set it to today - logger.warning("current expiration date not set; setting to today") + logger.warning("current expiration date not set; setting to today", exc_info=True) exp_date = date.today() - # create RenewDomain request request = commands.RenewDomain(name=self.name, cur_exp_date=exp_date, period=epp.Period(length, unit)) @@ -338,13 +337,14 @@ class Domain(TimeStampedModel, DomainHelper): self._cache["ex_date"] = registry.send(request, cleaned=True).res_data[0].ex_date self.expiration_date = self._cache["ex_date"] self.save() + except RegistryError as err: # if registry error occurs, log the error, and raise it as well - logger.error(f"registry error renewing domain: {err}") + logger.error(f"Registry error renewing domain '{self.name}': {err}") raise (err) except Exception as e: # exception raised during the save to registrar - logger.error(f"error updating expiration date in registrar: {e}") + logger.error(f"Error updating expiration date for domain '{self.name}' in registrar: {e}") raise (e) @Cache @@ -1329,14 +1329,14 @@ class Domain(TimeStampedModel, DomainHelper): def get_default_administrative_contact(self): """Gets the default administrative contact.""" - logger.info("get_default_security_contact() -> Adding administrative security contact") + logger.info("get_default_administrative_contact() -> Adding default administrative contact") contact = PublicContact.get_default_administrative() contact.domain = self return contact def get_default_technical_contact(self): """Gets the default technical contact.""" - logger.info("get_default_security_contact() -> Adding technical security contact") + logger.info("get_default_security_contact() -> Adding default technical contact") contact = PublicContact.get_default_technical() contact.domain = self return contact @@ -1575,7 +1575,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.info("Changing to DNS_NEEDED state") logger.info("able to transition to DNS_NEEDED state") - def get_state_help_text(self) -> str: + def get_state_help_text(self, request=None) -> str: """Returns a str containing additional information about a given state. Returns custom content for when the domain itself is expired.""" @@ -1585,6 +1585,8 @@ class Domain(TimeStampedModel, DomainHelper): help_text = ( "This domain has expired, but it is still online. " "To renew this domain, contact help@get.gov." ) + elif flag_is_active(request, "domain_renewal") and self.is_expiring(): + help_text = "This domain will expire soon. Contact one of the listed domain managers to renew the domain." else: help_text = Domain.State.get_help_text(self.state) @@ -1676,9 +1678,11 @@ class Domain(TimeStampedModel, DomainHelper): for domainContact in contact_data: req = commands.InfoContact(id=domainContact.contact) data = registry.send(req, cleaned=True).res_data[0] + logger.info(f"_fetch_contacts => this is the data: {data}") # Map the object we recieved from EPP to a PublicContact mapped_object = self.map_epp_contact_to_public_contact(data, domainContact.contact, domainContact.type) + logger.info(f"_fetch_contacts => mapped_object: {mapped_object}") # Find/create it in the DB in_db = self._get_or_create_public_contact(mapped_object) @@ -1869,8 +1873,9 @@ class Domain(TimeStampedModel, DomainHelper): missingSecurity = True missingTech = True - if len(cleaned.get("_contacts")) < 3: - for contact in cleaned.get("_contacts"): + contacts = cleaned.get("_contacts", []) + if len(contacts) < 3: + for contact in contacts: if contact.type == PublicContact.ContactTypeChoices.ADMINISTRATIVE: missingAdmin = False if contact.type == PublicContact.ContactTypeChoices.SECURITY: @@ -1889,6 +1894,11 @@ class Domain(TimeStampedModel, DomainHelper): technical_contact = self.get_default_technical_contact() technical_contact.save() + logger.info( + "_add_missing_contacts_if_unknown => Adding contacts. Values are " + f"missingAdmin: {missingAdmin}, missingSecurity: {missingSecurity}, missingTech: {missingTech}" + ) + def _fetch_cache(self, fetch_hosts=False, fetch_contacts=False): """Contact registry for info about a domain.""" try: @@ -2102,8 +2112,21 @@ class Domain(TimeStampedModel, DomainHelper): # Save to DB if it doesn't exist already. if db_contact.count() == 0: # Doesn't run custom save logic, just saves to DB - public_contact.save(skip_epp_save=True) - logger.info(f"Created a new PublicContact: {public_contact}") + try: + with transaction.atomic(): + public_contact.save(skip_epp_save=True) + logger.info(f"Created a new PublicContact: {public_contact}") + except IntegrityError as err: + logger.error( + f"_get_or_create_public_contact() => tried to create a duplicate public contact: {err}", + exc_info=True, + ) + return PublicContact.objects.get( + registry_id=public_contact.registry_id, + contact_type=public_contact.contact_type, + domain=self, + ) + # Append the item we just created return public_contact @@ -2113,7 +2136,7 @@ class Domain(TimeStampedModel, DomainHelper): if existing_contact.email != public_contact.email or existing_contact.registry_id != public_contact.registry_id: existing_contact.delete() public_contact.save() - logger.warning("Requested PublicContact is out of sync " "with DB.") + logger.warning("Requested PublicContact is out of sync with DB.") return public_contact # If it already exists, we can assume that the DB instance was updated during set, so we should just use that. diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 5e425f5a3..c2ba6887f 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,3 +343,27 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value + + +def normalize_string(string_to_normalize, lowercase=True): + """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + if not isinstance(string_to_normalize, str): + logger.error(f"normalize_string => {string_to_normalize} is not type str.") + return string_to_normalize + + new_string = " ".join(string_to_normalize.split()) + return new_string.lower() if lowercase else new_string + + +def count_capitals(text: str, leading_only: bool): + """Counts capital letters in a string. + Args: + text (str): The string to analyze. + leading_only (bool): If False, counts all capital letters. + If True, only counts capitals at the start of words. + Returns: + int: Number of capital letters found. + """ + if leading_only: + return sum(word[0].isupper() for word in text.split() if word) + return sum(c.isupper() for c in text if c) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index cde28e4bd..4ae282f21 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -153,7 +153,9 @@ def validate_user_portfolio_permission(user_portfolio_permission): "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." ) - existing_invitations = PortfolioInvitation.objects.filter(email=user_portfolio_permission.user.email) + existing_invitations = PortfolioInvitation.objects.exclude( + portfolio=user_portfolio_permission.portfolio + ).filter(email=user_portfolio_permission.user.email) if existing_invitations.exists(): raise ValidationError( "This user is already assigned to a portfolio invitation. " diff --git a/src/registrar/templates/django/admin/domain_invitation_change_form.html b/src/registrar/templates/django/admin/domain_invitation_change_form.html new file mode 100644 index 000000000..6ce6ed0d1 --- /dev/null +++ b/src/registrar/templates/django/admin/domain_invitation_change_form.html @@ -0,0 +1,14 @@ +{% extends 'django/admin/email_clipboard_change_form.html' %} +{% load custom_filters %} +{% load i18n static %} + +{% block content_subtitle %} +
+
+

+ If you add someone to a domain here, it will trigger emails to the invitee and all managers of the domain when you click "save." If you don't want to trigger those emails, use the User domain roles permissions table instead. +

+
+
+ {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/django/admin/user_domain_role_change_form.html b/src/registrar/templates/django/admin/user_domain_role_change_form.html new file mode 100644 index 000000000..d8c298bc1 --- /dev/null +++ b/src/registrar/templates/django/admin/user_domain_role_change_form.html @@ -0,0 +1,14 @@ +{% extends 'django/admin/email_clipboard_change_form.html' %} +{% load custom_filters %} +{% load i18n static %} + +{% block content_subtitle %} +
+
+

+ If you add someone to a domain here, it will not trigger any emails. To trigger emails, use the User Domain Role invitations table instead. +

+
+
+ {{ block.super }} +{% endblock %} diff --git a/src/registrar/templates/django/forms/widgets/combobox.html b/src/registrar/templates/django/forms/widgets/combobox.html index 7ff31945b..02cd4e35e 100644 --- a/src/registrar/templates/django/forms/widgets/combobox.html +++ b/src/registrar/templates/django/forms/widgets/combobox.html @@ -11,6 +11,7 @@ for now we just carry the attribute to both the parent element and the select. {{ name }}="{{ value }}" {% endif %} {% endfor %} +data-default-value="{% for group_name, group_choices, group_index in widget.optgroups %}{% for option in group_choices %}{% if option.selected %}{{ option.value }}{% endif %}{% endfor %}{% endfor %}" > - {% include "django/forms/widgets/select.html" %} + {% include "django/forms/widgets/select.html" with is_combobox=True %} diff --git a/src/registrar/templates/django/forms/widgets/select.html b/src/registrar/templates/django/forms/widgets/select.html index cc62eb91d..db6deafe2 100644 --- a/src/registrar/templates/django/forms/widgets/select.html +++ b/src/registrar/templates/django/forms/widgets/select.html @@ -3,6 +3,9 @@ {# hint: spacing in the class string matters #} class="usa-select{% if classes %} {{ classes }}{% endif %}" {% include "django/forms/widgets/attrs.html" %} + {% if is_combobox %} + data-default-value="{% for group_name, group_choices, group_index in widget.optgroups %}{% for option in group_choices %}{% if option.selected %}{{ option.value }}{% endif %}{% endfor %}{% endfor %}" + {% endif %} > {% for group, options, index in widget.optgroups %} {% if group %}{% endif %} diff --git a/src/registrar/templates/domain_base.html b/src/registrar/templates/domain_base.html index b65e9399b..c88492a93 100644 --- a/src/registrar/templates/domain_base.html +++ b/src/registrar/templates/domain_base.html @@ -1,5 +1,7 @@ {% extends "base.html" %} {% load static %} +{% load static url_helpers %} + {% block title %}{{ domain.name }} | {% endblock %} @@ -53,8 +55,11 @@ {% endif %} {% block domain_content %} - + {% if request.path|endswith:"renewal"%} +

Renew {{domain.name}}

+ {%else%}

Domain Overview

+ {% endif%} {% endblock %} {# domain_content #} {% endif %} @@ -62,4 +67,4 @@ -{% endblock %} {# content #} +{% endblock %} {# content #} \ No newline at end of file diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index a5b8e52cb..2cd3e5a5c 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -49,10 +49,18 @@ {% if domain.get_state_help_text %}
- {% if has_domain_renewal_flag and domain.is_expiring and is_domain_manager %} - This domain will expire soon. Renew to maintain access. + {% if has_domain_renewal_flag and domain.is_expired and is_domain_manager %} + This domain has expired, but it is still online. + {% url 'domain-renewal' pk=domain.id as url %} + Renew to maintain access. + {% elif has_domain_renewal_flag and domain.is_expiring and is_domain_manager %} + This domain will expire soon. + {% url 'domain-renewal' pk=domain.id as url %} + Renew to maintain access. {% elif has_domain_renewal_flag and domain.is_expiring and is_portfolio_user %} This domain will expire soon. Contact one of the listed domain managers to renew the domain. + {% elif has_domain_renewal_flag and domain.is_expired and is_portfolio_user %} + This domain has expired, but it is still online. Contact one of the listed domain managers to renew the domain. {% else %} {{ domain.get_state_help_text }} {% endif %} diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html new file mode 100644 index 000000000..32e535ed5 --- /dev/null +++ b/src/registrar/templates/domain_renewal.html @@ -0,0 +1,140 @@ +{% extends "domain_base.html" %} +{% load static url_helpers %} +{% load custom_filters %} + +{% block domain_content %} + {% block breadcrumb %} + + + {% if form.is_policy_acknowledged.errors %} +
+
+ {% for error in form.is_policy_acknowledged.errors %} +

{{ error }}

+ {% endfor %} +
+
+ {% endif %} + + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %} + + {{ block.super }} +
+

Confirm the following information for accuracy

+

Review these details below. We + require that you maintain accurate information for the domain. + The details you provide will only be used to support the administration of .gov and won't be made public. +

+

If you would like to retire your domain instead, please + contact us.

+

Required fields are marked with an asterisk (*). +

+ + + {% url 'user-profile' as url %} + {% include "includes/summary_item.html" with title='Your contact information' value=request.user edit_link=url editable=is_editable contact='true' %} + + {% if analyst_action != 'edit' or analyst_action_location != domain.pk %} + {% if is_portfolio_user and not is_domain_manager %} +
+
+

+ You don't have access to manage {{domain.name}}. If you need to make updates, contact one of the listed domain managers. +

+
+
+ {% endif %} + {% endif %} + + {% url 'domain-security-email' pk=domain.id as url %} + {% if security_email is not None and security_email not in hidden_security_emails%} + {% include "includes/summary_item.html" with title='Security email' value=security_email custom_text_for_value_none='We strongly recommend that you provide a security email. This email will allow the public to report observed or suspected security issues on your domain.' edit_link=url editable=is_editable %} + {% else %} + {% include "includes/summary_item.html" with title='Security email' value='None provided' custom_text_for_value_none='We strongly recommend that you provide a security email. This email will allow the public to report observed or suspected security issues on your domain.' edit_link=url editable=is_editable %} + {% endif %} + + {% url 'domain-users' pk=domain.id as url %} + {% if portfolio %} + {% include "includes/summary_item.html" with title='Domain managers' domain_permissions=True value=domain edit_link=url editable=is_editable %} + {% else %} + {% include "includes/summary_item.html" with title='Domain managers' list=True users=True value=domain.permissions.all edit_link=url editable=is_editable %} + {% endif %} + +
+ +
+ +

+ Acknowledgement of .gov domain requirements

+ + +
+ {% csrf_token %} +
+ + {% if form.is_policy_acknowledged.errors %} + {% for error in form.is_policy_acknowledged.errors %} + + {% endfor %} +
+ {% endif %} + + + + + +
+ + + + +
+
+{% endblock %} {# domain_content #} \ No newline at end of file diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 99ca1bfb7..ca3802720 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -80,7 +80,16 @@ {% include "includes/domain_sidenav_item.html" with item_text="Domain managers" %} {% endwith %} + + {% if has_domain_renewal_flag and is_domain_manager%} + {% if domain.is_expiring or domain.is_expired %} + {% with url_name="domain-renewal" %} + {% include "includes/domain_sidenav_item.html" with item_text="Renewal form" %} + {% endwith %} + {% endif %} + {% endif %} + {% endif %} - + \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_invitation.txt b/src/registrar/templates/emails/domain_invitation.txt index 068040205..a077bff26 100644 --- a/src/registrar/templates/emails/domain_invitation.txt +++ b/src/registrar/templates/emails/domain_invitation.txt @@ -1,36 +1,40 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} -Hi. +Hi,{% if requested_user and requested_user.first_name %} {{ requested_user.first_name }}.{% endif %} -{{ requestor_email }} has added you as a manager on {{ domain.name }}. - -You can manage this domain on the .gov registrar . +{{ requestor_email }} has invited you to manage: +{% for domain in domains %}{{ domain.name }} +{% endfor %} +To manage domain information, visit the .gov registrar . ---------------------------------------------------------------- +{% if not requested_user %} YOU NEED A LOGIN.GOV ACCOUNT -You’ll need a Login.gov account to manage your .gov domain. Login.gov provides -a simple and secure process for signing in to many government services with one -account. +You’ll need a Login.gov account to access the .gov registrar. That account needs to be +associated with the following email address: {{ invitee_email_address }} -If you don’t already have one, follow these steps to create your -Login.gov account . +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 . +{% endif %} DOMAIN MANAGEMENT -As a .gov domain manager, you can add or update information about your domain. -You’ll also serve as a contact for your .gov domain. Please keep your contact -information updated. +As a .gov domain manager, you can add or update information like name servers. You’ll +also serve as a contact for the domains you manage. Please keep your contact +information updated. Learn more about domain management . SOMETHING WRONG? -If you’re not affiliated with {{ domain.name }} or think you received this -message in error, reply to this email. +If you’re not affiliated with the .gov domains mentioned in this invitation 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. +.Gov helps the public identify official, trusted information. Thank you for using a .gov +domain. ---------------------------------------------------------------- @@ -38,5 +42,6 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency +(CISA) {% endautoescape %} diff --git a/src/registrar/templates/emails/domain_invitation_subject.txt b/src/registrar/templates/emails/domain_invitation_subject.txt index 319b80176..9663346d0 100644 --- a/src/registrar/templates/emails/domain_invitation_subject.txt +++ b/src/registrar/templates/emails/domain_invitation_subject.txt @@ -1 +1 @@ -You’ve been added to a .gov domain \ No newline at end of file +You've been invited to manage {% if domains|length > 1 %}.gov domains{% else %}{{ domains.0.name }}{% endif %} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_manager_notification.txt b/src/registrar/templates/emails/domain_manager_notification.txt new file mode 100644 index 000000000..aa8c6bf34 --- /dev/null +++ b/src/registrar/templates/emails/domain_manager_notification.txt @@ -0,0 +1,43 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +Hi,{% if domain_manager and domain_manager.first_name %} {{ domain_manager.first_name }}.{% endif %} + +A domain manager was invited to {{ domain.name }}. +DOMAIN: {{ domain.name }} +INVITED BY: {{ requestor_email }} +INVITED ON: {{date}} +MANAGER INVITED: {{ invited_email_address }} + + +---------------------------------------------------------------- + + +NEXT STEPS + +The person who received the invitation will become a domain manager once they log in to the +.gov registrar. They'll need to access the registrar using a Login.gov account that's +associated with the invited email address. + +If you need to cancel this invitation or remove the domain manager (because they've already +logged in), you can do that by going to this domain in the .gov registrar . + + +WHY DID YOU RECEIVE THIS EMAIL? + +You’re listed as a domain manager for {{ domain.name }}, so you’ll receive a notification whenever +someone is invited to manage that domain. + +If you have questions or concerns, reach out to the person who sent the invitation or 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/domain_manager_notification_subject.txt b/src/registrar/templates/emails/domain_manager_notification_subject.txt new file mode 100644 index 000000000..0e9918de0 --- /dev/null +++ b/src/registrar/templates/emails/domain_manager_notification_subject.txt @@ -0,0 +1 @@ +A domain manager was invited to {{ domain.name }} \ 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 c48e2c9a6..b026a7a6b 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -186,8 +186,7 @@ Created by {% endif %} Status - Action - + Action diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index f9f8b4c9f..f7e36d330 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -172,7 +172,7 @@ >Deleted - {% if has_domain_renewal_flag and num_expiring_domains > 0 %} + {% if has_domain_renewal_flag %}
- Action + Action diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index bbdfc8dee..25387ecbd 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -127,15 +127,15 @@ {% endif %} {% else %} -

+ {% if custom_text_for_value_none %} +

{{ custom_text_for_value_none }}

+ {% endif %} {% if value %} {{ value }} - {% elif custom_text_for_value_none %} - {{ custom_text_for_value_none }} - {% else %} + {% endif %} + {% if not value %} None {% endif %} -

{% endif %}
diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index b750af599..d21678d58 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -200,6 +200,7 @@ def is_domain_subpage(path): "domain-users-add", "domain-request-delete", "domain-user-delete", + "domain-renewal", "invitation-cancel", ] return get_url_name(path) in url_names diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 05b39cf55..bb65ef6b1 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -40,6 +40,7 @@ from epplibwrapper import ( ErrorCode, responses, ) +from registrar.models.suborganization import Suborganization from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.models.user_domain_role import UserDomainRole @@ -577,6 +578,13 @@ class MockDb(TestCase): creator=cls.custom_superuser, federal_agency=cls.federal_agency_3, organization_type="federal" ) + cls.suborganization_1, _ = Suborganization.objects.get_or_create( + name="SubOrg 1", + portfolio=cls.portfolio_1, + city="Nashville", + state_territory="TN", + ) + current_date = get_time_aware_date(datetime(2024, 4, 2)) # Create start and end dates using timedelta @@ -847,6 +855,7 @@ class MockDb(TestCase): status=DomainRequest.DomainRequestStatus.IN_REVIEW, name="city2.gov", portfolio=cls.portfolio_1, + sub_organization=cls.suborganization_1, ) cls.domain_request_3 = completed_domain_request( status=DomainRequest.DomainRequestStatus.STARTED, @@ -862,6 +871,9 @@ class MockDb(TestCase): cls.domain_request_5 = completed_domain_request( status=DomainRequest.DomainRequestStatus.APPROVED, name="city5.gov", + requested_suborganization="requested_suborg", + suborganization_city="SanFran", + suborganization_state_territory="CA", ) cls.domain_request_6 = completed_domain_request( status=DomainRequest.DomainRequestStatus.STARTED, @@ -911,6 +923,7 @@ class MockDb(TestCase): DomainInformation.objects.all().delete() DomainRequest.objects.all().delete() UserDomainRole.objects.all().delete() + Suborganization.objects.all().delete() Portfolio.objects.all().delete() UserPortfolioPermission.objects.all().delete() User.objects.all().delete() @@ -1039,6 +1052,8 @@ def completed_domain_request( # noqa federal_agency=None, federal_type=None, action_needed_reason=None, + city=None, + state_territory=None, portfolio=None, organization_name=None, sub_organization=None, @@ -1081,7 +1096,7 @@ def completed_domain_request( # noqa organization_name=organization_name if organization_name else "Testorg", address_line1="address 1", address_line2="address 2", - state_territory="NY", + state_territory="NY" if not state_territory else state_territory, zipcode="10002", senior_official=so, requested_domain=domain, @@ -1090,6 +1105,10 @@ def completed_domain_request( # noqa investigator=investigator, federal_agency=federal_agency, ) + + if city: + domain_request_kwargs["city"] = city + if has_about_your_organization: domain_request_kwargs["about_your_organization"] = "e-Government" if has_anything_else: diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 3195f8237..036e35a50 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -28,7 +28,6 @@ from registrar.admin import ( TransitionDomainAdmin, UserGroupAdmin, PortfolioAdmin, - UserPortfolioPermissionAdmin, ) from registrar.models import ( Domain, @@ -69,9 +68,7 @@ from django.contrib.sessions.backends.db import SessionStore from django.contrib.auth import get_user_model from django.contrib import messages -from unittest.mock import ANY, patch, Mock -from django.forms import ValidationError - +from unittest.mock import ANY, call, patch, Mock import logging @@ -136,12 +133,18 @@ class TestDomainInvitationAdmin(TestCase): self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite()) self.superuser = create_superuser() self.domain = Domain.objects.create(name="example.com") + self.portfolio = Portfolio.objects.create(organization_name="new portfolio", creator=self.superuser) + DomainInformation.objects.create(domain=self.domain, portfolio=self.portfolio, creator=self.superuser) """Create a client object""" self.client = Client(HTTP_HOST="localhost:8080") def tearDown(self): """Delete all DomainInvitation objects""" + PortfolioInvitation.objects.all().delete() DomainInvitation.objects.all().delete() + DomainInformation.objects.all().delete() + Portfolio.objects.all().delete() + Domain.objects.all().delete() Contact.objects.all().delete() User.objects.all().delete() @@ -163,6 +166,29 @@ class TestDomainInvitationAdmin(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) + + domain, _ = Domain.objects.get_or_create(name="systemofadown.com") + + domain_invitation, _ = DomainInvitation.objects.get_or_create(email="toxicity@systemofadown.com", domain=domain) + + response = self.client.get( + "/admin/registrar/domaininvitation/{}/change/".format(domain_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 domain here, it will trigger emails to the invitee and all managers of the domain", + ) + @less_console_noise_decorator def test_get_filters(self): """Ensures that our filters are displaying correctly""" @@ -189,11 +215,273 @@ class TestDomainInvitationAdmin(TestCase): self.assertContains(response, retrieved_html, count=1) @less_console_noise_decorator - def test_save_model_user_exists(self): - """Test saving a domain invitation when the user exists. + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + def test_add_domain_invitation_success_when_user_not_portfolio_member( + self, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and is not a portfolio member. + + Should send out domain and portfolio invites. + Should trigger success messages for both email sends. + Should attempt to retrieve the domain invitation. + Should attempt to retrieve the portfolio invitation.""" + + user = User.objects.create_user(email="test@example.com", username="username") + + # Create a domain invitation instance + invitation = DomainInvitation(email="test@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_called_once_with( + email="test@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=user, + ) + mock_send_portfolio_email.assert_called_once_with( + email="test@example.com", + requestor=self.superuser, + portfolio=self.portfolio, + ) + + # Assert success message + mock_messages_success.assert_has_calls( + [ + call(request, "test@example.com has been invited to the organization: new portfolio"), + call(request, "test@example.com has been invited to the domain: example.com"), + ] + ) + + # Assert the invitations were saved + self.assertEqual(DomainInvitation.objects.count(), 1) + self.assertEqual(DomainInvitation.objects.first().email, "test@example.com") + self.assertEqual(PortfolioInvitation.objects.count(), 1) + self.assertEqual(PortfolioInvitation.objects.first().email, "test@example.com") + + # Assert invitations were retrieved + domain_invitation = DomainInvitation.objects.get(email=user.email, domain=self.domain) + portfolio_invitation = PortfolioInvitation.objects.get(email=user.email, portfolio=self.portfolio) + + self.assertEqual(domain_invitation.status, DomainInvitation.DomainInvitationStatus.RETRIEVED) + self.assertEqual(portfolio_invitation.status, PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) + self.assertEqual(UserDomainRole.objects.count(), 1) + self.assertEqual(UserDomainRole.objects.first().user, user) + self.assertEqual(UserPortfolioPermission.objects.count(), 1) + self.assertEqual(UserPortfolioPermission.objects.first().user, user) + + @less_console_noise_decorator + @override_flag("organization_feature", active=False) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + def test_add_domain_invitation_success_when_user_not_portfolio_member_and_organization_feature_off( + self, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and organization_feature flag is off. + + Should send out a domain invitation. + Should not send a out portfolio invitation. + Should trigger success message for the domain invitation. + Should retrieve the domain invitation. + Should not create a portfolio invitation.""" + + user = User.objects.create_user(email="test@example.com", username="username") + + # Create a domain invitation instance + invitation = DomainInvitation(email="test@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain but not portfolio + mock_send_domain_email.assert_called_once_with( + email="test@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=user, + ) + mock_send_portfolio_email.assert_not_called() + + # Assert correct invite was created + self.assertEqual(DomainInvitation.objects.count(), 1) + self.assertEqual(PortfolioInvitation.objects.count(), 0) + + # Assert success message + mock_messages_success.assert_called_once_with( + request, "test@example.com has been invited to the domain: example.com" + ) + + # Assert the domain invitation was saved + self.assertEqual(DomainInvitation.objects.count(), 1) + self.assertEqual(DomainInvitation.objects.first().email, "test@example.com") + self.assertEqual(PortfolioInvitation.objects.count(), 0) + + # Assert the domain invitation was retrieved + domain_invitation = DomainInvitation.objects.get(email=user.email, domain=self.domain) + + self.assertEqual(domain_invitation.status, DomainInvitation.DomainInvitationStatus.RETRIEVED) + self.assertEqual(UserDomainRole.objects.count(), 1) + self.assertEqual(UserDomainRole.objects.first().user, user) + self.assertEqual(UserPortfolioPermission.objects.count(), 0) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("multiple_portfolios", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + def test_add_domain_invitation_success_when_user_not_portfolio_member_and_multiple_portfolio_feature_on( + self, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and multiple_portfolio flag is on. + + Should send out a domain invitation. + Should not send a out portfolio invitation. + Should trigger success message for the domain invitation. + Should retrieve the domain invitation. + Should not create a portfolio invitation. + + NOTE: This test may need to be reworked when the multiple_portfolio flag is fully fleshed out. + """ + + user = User.objects.create_user(email="test@example.com", username="username") + + # Create a domain invitation instance + invitation = DomainInvitation(email="test@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain but not portfolio + mock_send_domain_email.assert_called_once_with( + email="test@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=user, + ) + mock_send_portfolio_email.assert_not_called() + + # Assert correct invite was created + self.assertEqual(DomainInvitation.objects.count(), 1) + self.assertEqual(PortfolioInvitation.objects.count(), 0) + + # Assert success message + mock_messages_success.assert_called_once_with( + request, "test@example.com has been invited to the domain: example.com" + ) + + # Assert the domain invitation was saved + self.assertEqual(DomainInvitation.objects.count(), 1) + self.assertEqual(DomainInvitation.objects.first().email, "test@example.com") + self.assertEqual(PortfolioInvitation.objects.count(), 0) + + # Assert the domain invitation was retrieved + domain_invitation = DomainInvitation.objects.get(email=user.email, domain=self.domain) + + self.assertEqual(domain_invitation.status, DomainInvitation.DomainInvitationStatus.RETRIEVED) + self.assertEqual(UserDomainRole.objects.count(), 1) + self.assertEqual(UserDomainRole.objects.first().user, user) + self.assertEqual(UserPortfolioPermission.objects.count(), 0) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + def test_add_domain_invitation_success_when_user_existing_portfolio_member( + self, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and a portfolio invitation exists. + + Should send out domain invitation only. + Should trigger success message for the domain invitation. + Should retrieve the domain invitation.""" + + user = User.objects.create_user(email="test@example.com", username="username") + + # Create a domain invitation instance + invitation = DomainInvitation(email="test@example.com", domain=self.domain) + + UserPortfolioPermission.objects.create( + user=user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + ) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_called_once_with( + email="test@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=user, + ) + mock_send_portfolio_email.assert_not_called + + # Assert retrieve was not called + domain_invitation_mock_retrieve.assert_called_once() + portfolio_invitation_mock_retrieve.assert_not_called() + + # Assert success message + mock_messages_success.assert_called_once_with( + request, "test@example.com has been invited to the domain: example.com" + ) + + # Assert the invitations were saved + self.assertEqual(DomainInvitation.objects.count(), 1) + self.assertEqual(DomainInvitation.objects.first().email, "test@example.com") + self.assertEqual(PortfolioInvitation.objects.count(), 0) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.error") + def test_add_domain_invitation_when_user_not_portfolio_member_raises_exception_sending_portfolio_email( + self, mock_messages_error, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and is not a portfolio member raises + sending portfolio email exception. + + Should only attempt to send the portfolio invitation. + Should trigger error message on portfolio invitation. + Should not attempt to retrieve the domain invitation.""" + + mock_send_portfolio_email.side_effect = MissingEmailError("craving a burger") - Should attempt to retrieve the domain invitation.""" - # Create a user with the same email User.objects.create_user(email="test@example.com", username="username") # Create a domain invitation instance @@ -205,22 +493,180 @@ class TestDomainInvitationAdmin(TestCase): request = self.factory.post("/admin/registrar/DomainInvitation/add/") request.user = self.superuser - # Patch the retrieve method - with patch.object(DomainInvitation, "retrieve") as mock_retrieve: - admin_instance.save_model(request, invitation, form=None, change=False) + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) - # Assert retrieve was called - mock_retrieve.assert_called_once() + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_not_called() + mock_send_portfolio_email.assert_called_once_with( + email="test@example.com", + requestor=self.superuser, + portfolio=self.portfolio, + ) - # Assert the invitation was saved - self.assertEqual(DomainInvitation.objects.count(), 1) - self.assertEqual(DomainInvitation.objects.first().email, "test@example.com") + # Assert retrieve on domain invite only was called + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_not_called() + + # Assert error message + mock_messages_error.assert_called_once_with( + request, "Can't send invitation email. No email is associated with your user account." + ) + + # Assert the invitations were saved + self.assertEqual(DomainInvitation.objects.count(), 0) + self.assertEqual(PortfolioInvitation.objects.count(), 0) @less_console_noise_decorator - def test_save_model_user_does_not_exist(self): + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + @patch("django.contrib.messages.error") + def test_add_domain_invitation_when_user_not_portfolio_member_raises_exception_sending_domain_email( + self, mock_messages_error, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and is not a portfolio member raises + sending domain email exception. + + Should send out the portfolio invitation and attempt to send the domain invitation. + Should trigger portfolio invitation success message. + Should trigger domain invitation error message. + Should not attempt to retrieve the domain invitation. + Should attempt to retrieve the portfolio invitation.""" + + mock_send_domain_email.side_effect = MissingEmailError("craving a burger") + + user = User.objects.create_user(email="test@example.com", username="username") + + # Create a domain invitation instance + invitation = DomainInvitation(email="test@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_called_once_with( + email="test@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=user, + ) + mock_send_portfolio_email.assert_called_once_with( + email="test@example.com", + requestor=self.superuser, + portfolio=self.portfolio, + ) + + # Assert retrieve on domain invite only was called + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_called_once() + + # Assert success message + mock_messages_success.assert_called_once_with( + request, "test@example.com has been invited to the organization: new portfolio" + ) + + # Assert error message + mock_messages_error.assert_called_once_with( + request, "Can't send invitation email. No email is associated with your user account." + ) + + # Assert the invitations were saved + self.assertEqual(DomainInvitation.objects.count(), 0) + self.assertEqual(PortfolioInvitation.objects.count(), 1) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + @patch("django.contrib.messages.error") + def test_add_domain_invitation_when_user_existing_portfolio_member_raises_exception_sending_domain_email( + self, mock_messages_error, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and is not a portfolio member raises + sending domain email exception. + + Should send out the portfolio invitation and attempt to send the domain invitation. + Should trigger portfolio invitation success message. + Should trigger domain invitation error message. + Should not attempt to retrieve the domain invitation. + Should attempt to retrieve the portfolio invitation.""" + + mock_send_domain_email.side_effect = MissingEmailError("craving a burger") + + user = User.objects.create_user(email="test@example.com", username="username") + + UserPortfolioPermission.objects.create( + user=user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + ) + + # Create a domain invitation instance + invitation = DomainInvitation(email="test@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_called_once_with( + email="test@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=user, + ) + mock_send_portfolio_email.assert_not_called() + + # Assert retrieve on domain invite only was called + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_not_called() + + # Assert success message + mock_messages_success.assert_not_called() + + # Assert error message + mock_messages_error.assert_called_once_with( + request, "Can't send invitation email. No email is associated with your user account." + ) + + # Assert the invitations were saved + self.assertEqual(DomainInvitation.objects.count(), 0) + self.assertEqual(PortfolioInvitation.objects.count(), 0) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + def test_add_domain_invitation_success_when_email_not_portfolio_member( + self, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): """Test saving a domain invitation when the user does not exist. - Should not attempt to retrieve the domain invitation.""" + Should send out domain and portfolio invitations. + Should trigger success messages. + Should not attempt to retrieve the domain invitation. + Should not attempt to retrieve the portfolio invitation.""" # Create a domain invitation instance invitation = DomainInvitation(email="nonexistent@example.com", domain=self.domain) @@ -231,15 +677,393 @@ class TestDomainInvitationAdmin(TestCase): request.user = self.superuser # Patch the retrieve method to ensure it is not called - with patch.object(DomainInvitation, "retrieve") as mock_retrieve: - admin_instance.save_model(request, invitation, form=None, change=False) + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_called_once_with( + email="nonexistent@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=None, + ) + mock_send_portfolio_email.assert_called_once_with( + email="nonexistent@example.com", + requestor=self.superuser, + portfolio=self.portfolio, + ) # Assert retrieve was not called - mock_retrieve.assert_not_called() + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_not_called() - # Assert the invitation was saved + # Assert success message + mock_messages_success.assert_has_calls( + [ + call(request, "nonexistent@example.com has been invited to the organization: new portfolio"), + call(request, "nonexistent@example.com has been invited to the domain: example.com"), + ] + ) + + # Assert the invitations were saved self.assertEqual(DomainInvitation.objects.count(), 1) self.assertEqual(DomainInvitation.objects.first().email, "nonexistent@example.com") + self.assertEqual(PortfolioInvitation.objects.count(), 1) + self.assertEqual(PortfolioInvitation.objects.first().email, "nonexistent@example.com") + + @less_console_noise_decorator + @override_flag("organization_feature", active=False) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + def test_add_domain_invitation_success_when_email_not_portfolio_member_and_organization_feature_off( + self, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user does not exist and organization_feature flag is off. + + Should send out a domain invitation. + Should not send a out portfolio invitation. + Should trigger success message for domain invitation. + Should not retrieve the domain invitation. + Should not create a portfolio invitation.""" + # Create a domain invitation instance + invitation = DomainInvitation(email="nonexistent@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain but not portfolio + mock_send_domain_email.assert_called_once_with( + email="nonexistent@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=None, + ) + mock_send_portfolio_email.assert_not_called() + + # Assert retrieve on domain invite only was called + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_not_called() + + # Assert success message + mock_messages_success.assert_called_once_with( + request, "nonexistent@example.com has been invited to the domain: example.com" + ) + + # Assert the domain invitation was saved + self.assertEqual(DomainInvitation.objects.count(), 1) + self.assertEqual(DomainInvitation.objects.first().email, "nonexistent@example.com") + self.assertEqual(PortfolioInvitation.objects.count(), 0) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("multiple_portfolios", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + def test_add_domain_invitation_success_when_email_not_portfolio_member_and_multiple_portfolio_feature_on( + self, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user does not exist and multiple_portfolio flag is on. + + Should send out a domain invitation. + Should not send a out portfolio invitation. + Should trigger success message for domain invitation. + Should not retrieve the domain invitation. + Should not create a portfolio invitation.""" + # Create a domain invitation instance + invitation = DomainInvitation(email="nonexistent@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain but not portfolio + mock_send_domain_email.assert_called_once_with( + email="nonexistent@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=None, + ) + mock_send_portfolio_email.assert_not_called() + + # Assert retrieve on domain invite only was called + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_not_called() + + # Assert success message + mock_messages_success.assert_called_once_with( + request, "nonexistent@example.com has been invited to the domain: example.com" + ) + + # Assert the domain invitation was saved + self.assertEqual(DomainInvitation.objects.count(), 1) + self.assertEqual(DomainInvitation.objects.first().email, "nonexistent@example.com") + self.assertEqual(PortfolioInvitation.objects.count(), 0) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + def test_add_domain_invitation_success_when_email_existing_portfolio_invitation( + self, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user does not exist and a portfolio invitation exists. + + Should send out domain invitation only. + Should trigger success message for the domain invitation. + Should not attempt to retrieve the domain invitation. + Should not attempt to retrieve the portfolio invitation.""" + + PortfolioInvitation.objects.create( + email="nonexistent@example.com", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + + # Create a domain invitation instance + invitation = DomainInvitation(email="nonexistent@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_called_once_with( + email="nonexistent@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=False, + requested_user=None, + ) + mock_send_portfolio_email.assert_not_called + + # Assert retrieve was not called + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_not_called() + + # Assert success message + mock_messages_success.assert_called_once_with( + request, "nonexistent@example.com has been invited to the domain: example.com" + ) + + # Assert the invitations were saved + self.assertEqual(DomainInvitation.objects.count(), 1) + self.assertEqual(DomainInvitation.objects.first().email, "nonexistent@example.com") + self.assertEqual(PortfolioInvitation.objects.count(), 1) + self.assertEqual(PortfolioInvitation.objects.first().email, "nonexistent@example.com") + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.error") + def test_add_domain_invitation_when_user_not_portfolio_email_raises_exception_sending_portfolio_email( + self, mock_messages_error, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and is not a portfolio member raises + sending portfolio email exception. + + Should only attempt to send the portfolio invitation. + Should trigger error message on portfolio invitation. + Should not attempt to retrieve the domain invitation. + Should not attempt to retrieve the portfolio invitation.""" + + mock_send_portfolio_email.side_effect = MissingEmailError("craving a burger") + + # Create a domain invitation instance + invitation = DomainInvitation(email="nonexistent@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_not_called() + mock_send_portfolio_email.assert_called_once_with( + email="nonexistent@example.com", + requestor=self.superuser, + portfolio=self.portfolio, + ) + + # Assert retrieve on domain invite only was called + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_not_called() + + # Assert error message + mock_messages_error.assert_called_once_with( + request, "Can't send invitation email. No email is associated with your user account." + ) + + # Assert the invitations were saved + self.assertEqual(DomainInvitation.objects.count(), 0) + self.assertEqual(PortfolioInvitation.objects.count(), 0) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + @patch("django.contrib.messages.error") + def test_add_domain_invitation_when_user_not_portfolio_email_raises_exception_sending_domain_email( + self, mock_messages_error, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and is not a portfolio member + raises sending domain email exception. + + Should send out the portfolio invitation and attempt to send the domain invitation. + Should trigger portfolio invitation success message. + Should trigger domain invitation error message. + Should not attempt to retrieve the domain invitation. + Should attempt to retrieve the portfolio invitation.""" + + mock_send_domain_email.side_effect = MissingEmailError("craving a burger") + + # Create a domain invitation instance + invitation = DomainInvitation(email="nonexistent@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_called_once_with( + email="nonexistent@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=None, + requested_user=None, + ) + mock_send_portfolio_email.assert_called_once_with( + email="nonexistent@example.com", + requestor=self.superuser, + portfolio=self.portfolio, + ) + + # Assert retrieve on domain invite only was called + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_not_called() + + # Assert success message + mock_messages_success.assert_called_once_with( + request, "nonexistent@example.com has been invited to the organization: new portfolio" + ) + + # Assert error message + mock_messages_error.assert_called_once_with( + request, "Can't send invitation email. No email is associated with your user account." + ) + + # Assert the invitations were saved + self.assertEqual(DomainInvitation.objects.count(), 0) + self.assertEqual(PortfolioInvitation.objects.count(), 1) + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @patch("registrar.admin.send_domain_invitation_email") + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") + @patch("django.contrib.messages.error") + def test_add_domain_invitation_when_user_existing_portfolio_email_raises_exception_sending_domain_email( + self, mock_messages_error, mock_messages_success, mock_send_portfolio_email, mock_send_domain_email + ): + """Test saving a domain invitation when the user exists and is not a portfolio member + raises sending domain email exception. + + Should send out the portfolio invitation and attempt to send the domain invitation. + Should trigger portfolio invitation success message. + Should trigger domain invitation error message. + Should not attempt to retrieve the domain invitation. + Should attempt to retrieve the portfolio invitation.""" + + mock_send_domain_email.side_effect = MissingEmailError("craving a burger") + + PortfolioInvitation.objects.create( + email="nonexistent@example.com", + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + + # Create a domain invitation instance + invitation = DomainInvitation(email="nonexistent@example.com", domain=self.domain) + + admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None) + + # Create a request object + request = self.factory.post("/admin/registrar/DomainInvitation/add/") + request.user = self.superuser + + # Patch the retrieve method to ensure it is not called + with patch.object(DomainInvitation, "retrieve") as domain_invitation_mock_retrieve: + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, invitation, form=None, change=False) + + # Assert sends appropriate emails - domain and portfolio invites + mock_send_domain_email.assert_called_once_with( + email="nonexistent@example.com", + requestor=self.superuser, + domains=self.domain, + is_member_of_different_org=False, + requested_user=None, + ) + mock_send_portfolio_email.assert_not_called() + + # Assert retrieve on domain invite only was called + domain_invitation_mock_retrieve.assert_not_called() + portfolio_invitation_mock_retrieve.assert_not_called() + + # Assert success message + mock_messages_success.assert_not_called() + + # Assert error message + mock_messages_error.assert_called_once_with( + request, "Can't send invitation email. No email is associated with your user account." + ) + + # Assert the invitations were saved + self.assertEqual(DomainInvitation.objects.count(), 0) + self.assertEqual(PortfolioInvitation.objects.count(), 1) class TestUserPortfolioPermissionAdmin(TestCase): @@ -247,8 +1071,6 @@ class TestUserPortfolioPermissionAdmin(TestCase): def setUp(self): """Create a client object""" - self.factory = RequestFactory() - self.admin = ListHeaderAdmin(model=UserPortfolioPermissionAdmin, admin_site=AdminSite()) self.client = Client(HTTP_HOST="localhost:8080") self.superuser = create_superuser() self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser) @@ -256,77 +1078,9 @@ class TestUserPortfolioPermissionAdmin(TestCase): def tearDown(self): """Delete all DomainInvitation objects""" Portfolio.objects.all().delete() - PortfolioInvitation.objects.all().delete() Contact.objects.all().delete() User.objects.all().delete() - - @less_console_noise_decorator - def test_clean_user_portfolio_permission(self): - """Tests validation of user portfolio permission""" - - # Test validation fails when portfolio missing but permissions are present - permission = UserPortfolioPermission(user=self.superuser, roles=["organization_admin"], portfolio=None) - with self.assertRaises(ValidationError) as err: - permission.clean() - self.assertEqual( - str(err.exception), - "When portfolio roles or additional permissions are assigned, portfolio is required.", - ) - - # Test validation fails when portfolio present but no permissions are present - permission = UserPortfolioPermission(user=self.superuser, roles=None, portfolio=self.portfolio) - with self.assertRaises(ValidationError) as err: - permission.clean() - self.assertEqual( - str(err.exception), - "When portfolio is assigned, portfolio roles or additional permissions are required.", - ) - - # Test validation fails with forbidden permissions for single role - forbidden_member_roles = UserPortfolioPermission.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get( - UserPortfolioRoleChoices.ORGANIZATION_MEMBER - ) - permission = UserPortfolioPermission( - user=self.superuser, - roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], - additional_permissions=forbidden_member_roles, - portfolio=self.portfolio, - ) - with self.assertRaises(ValidationError) as err: - permission.clean() - self.assertEqual( - str(err.exception), - "These permissions cannot be assigned to Member: " - "", - ) - - @less_console_noise_decorator - def test_get_forbidden_permissions_with_multiple_roles(self): - """Tests that forbidden permissions are properly handled when a user has multiple roles""" - # Get forbidden permissions for member role - member_forbidden = UserPortfolioPermission.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get( - UserPortfolioRoleChoices.ORGANIZATION_MEMBER - ) - - # Test with both admin and member roles - roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN, UserPortfolioRoleChoices.ORGANIZATION_MEMBER] - - # These permissions would be forbidden for member alone, but should be allowed - # when combined with admin role - permissions = UserPortfolioPermission.get_forbidden_permissions( - roles=roles, additional_permissions=member_forbidden - ) - - # Should return empty set since no permissions are commonly forbidden between admin and member - self.assertEqual(permissions, set()) - - # Verify the same permissions are forbidden when only member role is present - member_only_permissions = UserPortfolioPermission.get_forbidden_permissions( - roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], additional_permissions=member_forbidden - ) - - # Should return the forbidden permissions for member role - self.assertEqual(member_only_permissions, set(member_forbidden)) + UserPortfolioPermission.objects.all().delete() @less_console_noise_decorator def test_has_change_form_description(self): @@ -376,117 +1130,12 @@ class TestPortfolioInvitationAdmin(TestCase): Portfolio.objects.all().delete() PortfolioInvitation.objects.all().delete() Contact.objects.all().delete() + User.objects.all().delete() @classmethod def tearDownClass(self): User.objects.all().delete() - @less_console_noise_decorator - @override_flag("multiple_portfolios", active=False) - def test_clean_multiple_portfolios_inactive(self): - """Tests that users cannot have multiple portfolios or invitations when flag is inactive""" - # Create the first portfolio permission - UserPortfolioPermission.objects.create( - user=self.superuser, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - - # Test a second portfolio permission object (should fail) - second_portfolio = Portfolio.objects.create(organization_name="Second Portfolio", creator=self.superuser) - second_permission = UserPortfolioPermission( - user=self.superuser, portfolio=second_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - - with self.assertRaises(ValidationError) as err: - second_permission.clean() - self.assertIn("users cannot be assigned to multiple portfolios", str(err.exception)) - - # Test that adding a new portfolio invitation also fails - third_portfolio = Portfolio.objects.create(organization_name="Third Portfolio", creator=self.superuser) - invitation = PortfolioInvitation( - email=self.superuser.email, portfolio=third_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - - with self.assertRaises(ValidationError) as err: - invitation.clean() - self.assertIn("users cannot be assigned to multiple portfolios", str(err.exception)) - - @less_console_noise_decorator - @override_flag("multiple_portfolios", active=True) - def test_clean_multiple_portfolios_active(self): - """Tests that users can have multiple portfolios and invitations when flag is active""" - # Create first portfolio permission - UserPortfolioPermission.objects.create( - user=self.superuser, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - - # Second portfolio permission should succeed - second_portfolio = Portfolio.objects.create(organization_name="Second Portfolio", creator=self.superuser) - second_permission = UserPortfolioPermission( - user=self.superuser, portfolio=second_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - second_permission.clean() - second_permission.save() - - # Verify both permissions exist - user_permissions = UserPortfolioPermission.objects.filter(user=self.superuser) - self.assertEqual(user_permissions.count(), 2) - - # Portfolio invitation should also succeed - third_portfolio = Portfolio.objects.create(organization_name="Third Portfolio", creator=self.superuser) - invitation = PortfolioInvitation( - email=self.superuser.email, portfolio=third_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - invitation.clean() - invitation.save() - - # Verify invitation exists - self.assertTrue( - PortfolioInvitation.objects.filter( - email=self.superuser.email, - portfolio=third_portfolio, - ).exists() - ) - - @less_console_noise_decorator - def test_clean_portfolio_invitation(self): - """Tests validation of portfolio invitation permissions""" - - # Test validation fails when portfolio missing but permissions present - invitation = PortfolioInvitation(email="test@example.com", roles=["organization_admin"], portfolio=None) - with self.assertRaises(ValidationError) as err: - invitation.clean() - self.assertEqual( - str(err.exception), - "When portfolio roles or additional permissions are assigned, portfolio is required.", - ) - - # Test validation fails when portfolio present but no permissions - invitation = PortfolioInvitation(email="test@example.com", roles=None, portfolio=self.portfolio) - with self.assertRaises(ValidationError) as err: - invitation.clean() - self.assertEqual( - str(err.exception), - "When portfolio is assigned, portfolio roles or additional permissions are required.", - ) - - # Test validation fails with forbidden permissions - forbidden_member_roles = UserPortfolioPermission.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get( - UserPortfolioRoleChoices.ORGANIZATION_MEMBER - ) - invitation = PortfolioInvitation( - email="test@example.com", - roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], - additional_permissions=forbidden_member_roles, - portfolio=self.portfolio, - ) - with self.assertRaises(ValidationError) as err: - invitation.clean() - self.assertEqual( - str(err.exception), - "These permissions cannot be assigned to Member: " - "", - ) - @less_console_noise_decorator def test_has_model_description(self): """Tests if this model has a model description on the table view""" @@ -532,34 +1181,32 @@ class TestPortfolioInvitationAdmin(TestCase): @less_console_noise_decorator def test_get_filters(self): """Ensures that our filters are displaying correctly""" - with less_console_noise(): - self.client.force_login(self.superuser) + self.client.force_login(self.superuser) - response = self.client.get( - "/admin/registrar/portfolioinvitation/", - {}, - follow=True, - ) + response = self.client.get( + "/admin/registrar/portfolioinvitation/", + {}, + follow=True, + ) - # Assert that the filters are added - self.assertContains(response, "invited", count=4) - self.assertContains(response, "Invited", count=2) - self.assertContains(response, "retrieved", count=2) - self.assertContains(response, "Retrieved", count=2) + # Assert that the filters are added + self.assertContains(response, "invited", count=4) + self.assertContains(response, "Invited", count=2) + self.assertContains(response, "retrieved", count=2) + self.assertContains(response, "Retrieved", count=2) - # Check for the HTML context specificially - invited_html = 'Invited' - retrieved_html = 'Retrieved' + # Check for the HTML context specificially + invited_html = 'Invited' + retrieved_html = 'Retrieved' - self.assertContains(response, invited_html, count=1) - self.assertContains(response, retrieved_html, count=1) + 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) + def test_save_sends_email(self, mock_messages_success, mock_send_email): + """On save_model, an email is sent if an invitation already exists.""" # Create an instance of the admin class admin_instance = PortfolioInvitationAdmin(PortfolioInvitation, admin_site=None) @@ -578,7 +1225,7 @@ class TestPortfolioInvitationAdmin(TestCase): # Call the save_model method admin_instance.save_model(request, portfolio_invitation, None, None) - # Assert that send_portfolio_invitation_email is not called + # Assert that send_portfolio_invitation_email is called mock_send_email.assert_called() # Get the arguments passed to send_portfolio_invitation_email @@ -590,7 +1237,7 @@ class TestPortfolioInvitationAdmin(TestCase): 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.") + mock_messages_success.assert_called_once_with(request, "james.gordon@gotham.gov has been invited.") @less_console_noise_decorator @patch("registrar.admin.send_portfolio_invitation_email") @@ -627,6 +1274,94 @@ class TestPortfolioInvitationAdmin(TestCase): # 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.success") # Mock the `messages.warning` call + def test_add_portfolio_invitation_auto_retrieves_invitation_when_user_exists( + self, mock_messages_success, mock_send_email + ): + """On save_model, we create and retrieve a portfolio invitation if the user exists.""" + + # Create an instance of the admin class + admin_instance = PortfolioInvitationAdmin(PortfolioInvitation, admin_site=None) + + User.objects.create_user(email="james.gordon@gotham.gov", username="username") + + # 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 + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, portfolio_invitation, None, None) + + # Assert that send_portfolio_invitation_email is 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_success.assert_called_once_with(request, "james.gordon@gotham.gov has been invited.") + + # The invitation is not retrieved + portfolio_invitation_mock_retrieve.assert_called_once() + + @less_console_noise_decorator + @patch("registrar.admin.send_portfolio_invitation_email") + @patch("django.contrib.messages.success") # Mock the `messages.warning` call + def test_add_portfolio_invitation_does_not_retrieve_invitation_when_no_user( + self, mock_messages_success, mock_send_email + ): + """On save_model, we create but do not retrieve a portfolio invitation if the user does not exist.""" + + # 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 + with patch.object(PortfolioInvitation, "retrieve") as portfolio_invitation_mock_retrieve: + admin_instance.save_model(request, portfolio_invitation, None, None) + + # Assert that send_portfolio_invitation_email is 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_success.assert_called_once_with(request, "james.gordon@gotham.gov has been invited.") + + # The invitation is not retrieved + portfolio_invitation_mock_retrieve.assert_not_called() + @less_console_noise_decorator @patch("registrar.admin.send_portfolio_invitation_email") @patch("django.contrib.messages.error") # Mock the `messages.error` call @@ -655,9 +1390,7 @@ class TestPortfolioInvitationAdmin(TestCase): 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." - ) + mock_messages_error.assert_called_once_with(request, "Email service unavailable") @less_console_noise_decorator @patch("registrar.admin.send_portfolio_invitation_email") @@ -694,7 +1427,7 @@ class TestPortfolioInvitationAdmin(TestCase): @less_console_noise_decorator @patch("registrar.admin.send_portfolio_invitation_email") - @patch("django.contrib.messages.error") # Mock the `messages.error` call + @patch("django.contrib.messages.warning") # 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) @@ -720,9 +1453,7 @@ class TestPortfolioInvitationAdmin(TestCase): 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." - ) + mock_messages_error.assert_called_once_with(request, "Could not send email invitation.") class TestHostAdmin(TestCase): @@ -1249,6 +1980,31 @@ class TestUserDomainRoleAdmin(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) + + domain, _ = Domain.objects.get_or_create(name="systemofadown.com") + + user_domain_role, _ = UserDomainRole.objects.get_or_create( + user=self.superuser, domain=domain, role=[UserDomainRole.Roles.MANAGER] + ) + + response = self.client.get( + "/admin/registrar/userdomainrole/{}/change/".format(user_domain_role.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 domain here, it will not trigger any emails.", + ) + def test_domain_sortable(self): """Tests if the UserDomainrole sorts by domain correctly""" with less_console_noise(): @@ -2734,7 +3490,7 @@ class TestTransferUser(WebTest): @less_console_noise_decorator def test_transfer_user_transfers_user_portfolio_roles_no_error_when_duplicates(self): - """Assert that duplicate portfolio user roles do not throw errorsd""" + """Assert that duplicate portfolio user roles do not throw errors""" portfolio1 = Portfolio.objects.create(organization_name="Hotel California", creator=self.user2) UserPortfolioPermission.objects.create( user=self.user1, portfolio=portfolio1, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] @@ -2866,7 +3622,7 @@ class TestTransferUser(WebTest): with self.assertRaises(User.DoesNotExist): self.user2.refresh_from_db() - @less_console_noise_decorator + # @less_console_noise_decorator def test_transfer_user_throws_transfer_and_delete_success_messages(self): """Test that success messages for data transfer and user deletion are displayed.""" # Ensure the setup for VerifiedByStaff @@ -2884,11 +3640,13 @@ class TestTransferUser(WebTest): self.assertContains(after_submit, "

Change user

") + print(mock_success_message.call_args_list) + mock_success_message.assert_any_call( ANY, ( "Data transferred successfully for the following objects: ['Changed requestor " - + 'from "Furiosa Jabassa " to "Max Rokatanski " on immortan.joe@citadel.com\']' + + "from Furiosa Jabassa to Max Rokatanski on immortan.joe@citadel.com']" ), ) @@ -2898,7 +3656,7 @@ class TestTransferUser(WebTest): def test_transfer_user_throws_error_message(self): """Test that an error message is thrown if the transfer fails.""" with patch( - "registrar.views.TransferUserView.transfer_user_fields_and_log", side_effect=Exception("Simulated Error") + "registrar.views.TransferUserView.transfer_related_fields_and_log", side_effect=Exception("Simulated Error") ): with patch("django.contrib.messages.error") as mock_error: # Access the transfer user page diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py new file mode 100644 index 000000000..87384d3be --- /dev/null +++ b/src/registrar/tests/test_email_invitations.py @@ -0,0 +1,311 @@ +import unittest +from unittest.mock import patch, MagicMock +from datetime import date +from registrar.utility.email import EmailSendingError +from registrar.utility.email_invitations import send_domain_invitation_email + +from api.tests.common import less_console_noise_decorator + + +class DomainInvitationEmail(unittest.TestCase): + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") + @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations.send_invitation_email") + @patch("registrar.utility.email_invitations.normalize_domains") + def test_send_domain_invitation_email( + self, + mock_normalize_domains, + mock_send_invitation_email, + mock_get_requestor_email, + mock_validate_invitation, + mock_user_domain_role_filter, + mock_send_templated_email, + ): + """Test sending domain invitation email for one domain. + Should also send emails to manager of that domain. + """ + # Setup + mock_domain = MagicMock(name="domain1") + mock_domain.name = "example.com" + mock_normalize_domains.return_value = [mock_domain] + + mock_requestor = MagicMock() + mock_requestor_email = "requestor@example.com" + mock_get_requestor_email.return_value = mock_requestor_email + + mock_user1 = MagicMock() + mock_user1.email = "manager1@example.com" + + mock_user_domain_role_filter.return_value = [MagicMock(user=mock_user1)] + + email = "invitee@example.com" + is_member_of_different_org = False + + # Call the function + send_domain_invitation_email( + email=email, + requestor=mock_requestor, + domains=mock_domain, + is_member_of_different_org=is_member_of_different_org, + ) + + # Assertions + mock_normalize_domains.assert_called_once_with(mock_domain) + mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) + mock_validate_invitation.assert_called_once_with( + email, [mock_domain], mock_requestor, is_member_of_different_org + ) + mock_send_invitation_email.assert_called_once_with(email, mock_requestor_email, [mock_domain], None) + mock_user_domain_role_filter.assert_called_once_with(domain=mock_domain) + mock_send_templated_email.assert_called_once_with( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address=mock_user1.email, + context={ + "domain": mock_domain, + "requestor_email": mock_requestor_email, + "invited_email_address": email, + "domain_manager": mock_user1, + "date": date.today(), + }, + ) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.utility.email_invitations.UserDomainRole.objects.filter") + @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations.send_invitation_email") + @patch("registrar.utility.email_invitations.normalize_domains") + def test_send_domain_invitation_email_multiple_domains( + self, + mock_normalize_domains, + mock_send_invitation_email, + mock_get_requestor_email, + mock_validate_invitation, + mock_user_domain_role_filter, + mock_send_templated_email, + ): + """Test sending domain invitation email for multiple domains. + Should also send emails to managers of each domain. + """ + # Setup + # Create multiple mock domains + mock_domain1 = MagicMock(name="domain1") + mock_domain1.name = "example.com" + mock_domain2 = MagicMock(name="domain2") + mock_domain2.name = "example.org" + + mock_normalize_domains.return_value = [mock_domain1, mock_domain2] + + mock_requestor = MagicMock() + mock_requestor_email = "requestor@example.com" + mock_get_requestor_email.return_value = mock_requestor_email + + mock_user1 = MagicMock() + mock_user1.email = "manager1@example.com" + mock_user2 = MagicMock() + mock_user2.email = "manager2@example.com" + + # Configure domain roles for each domain + def filter_side_effect(domain): + if domain == mock_domain1: + return [MagicMock(user=mock_user1)] + elif domain == mock_domain2: + return [MagicMock(user=mock_user2)] + return [] + + mock_user_domain_role_filter.side_effect = filter_side_effect + + email = "invitee@example.com" + is_member_of_different_org = False + + # Call the function + send_domain_invitation_email( + email=email, + requestor=mock_requestor, + domains=[mock_domain1, mock_domain2], + is_member_of_different_org=is_member_of_different_org, + ) + + # Assertions + mock_normalize_domains.assert_called_once_with([mock_domain1, mock_domain2]) + mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain1, mock_domain2]) + mock_validate_invitation.assert_called_once_with( + email, [mock_domain1, mock_domain2], mock_requestor, is_member_of_different_org + ) + mock_send_invitation_email.assert_called_once_with( + email, mock_requestor_email, [mock_domain1, mock_domain2], None + ) + + # Check that domain manager emails were sent for both domains + mock_user_domain_role_filter.assert_any_call(domain=mock_domain1) + mock_user_domain_role_filter.assert_any_call(domain=mock_domain2) + + mock_send_templated_email.assert_any_call( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address=mock_user1.email, + context={ + "domain": mock_domain1, + "requestor_email": mock_requestor_email, + "invited_email_address": email, + "domain_manager": mock_user1, + "date": date.today(), + }, + ) + mock_send_templated_email.assert_any_call( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address=mock_user2.email, + context={ + "domain": mock_domain2, + "requestor_email": mock_requestor_email, + "invited_email_address": email, + "domain_manager": mock_user2, + "date": date.today(), + }, + ) + + # Verify the total number of calls to send_templated_email + self.assertEqual(mock_send_templated_email.call_count, 2) + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.validate_invitation") + def test_send_domain_invitation_email_raises_invite_validation_exception(self, mock_validate_invitation): + """Test sending domain invitation email for one domain and assert exception + when invite validation fails. + """ + # Setup + mock_validate_invitation.side_effect = ValueError("Validation failed") + email = "invitee@example.com" + requestor = MagicMock() + domain = MagicMock() + + # Call and assert exception + with self.assertRaises(ValueError) as context: + send_domain_invitation_email(email, requestor, domain, is_member_of_different_org=False) + + self.assertEqual(str(context.exception), "Validation failed") + mock_validate_invitation.assert_called_once() + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.get_requestor_email") + def test_send_domain_invitation_email_raises_get_requestor_email_exception(self, mock_get_requestor_email): + """Test sending domain invitation email for one domain and assert exception + when get_requestor_email fails. + """ + # Setup + mock_get_requestor_email.side_effect = ValueError("Validation failed") + email = "invitee@example.com" + requestor = MagicMock() + domain = MagicMock() + + # Call and assert exception + with self.assertRaises(ValueError) as context: + send_domain_invitation_email(email, requestor, domain, is_member_of_different_org=False) + + self.assertEqual(str(context.exception), "Validation failed") + mock_get_requestor_email.assert_called_once() + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations.send_invitation_email") + @patch("registrar.utility.email_invitations.normalize_domains") + def test_send_domain_invitation_email_raises_sending_email_exception( + self, + mock_normalize_domains, + mock_send_invitation_email, + mock_get_requestor_email, + mock_validate_invitation, + ): + """Test sending domain invitation email for one domain and assert exception + when send_invitation_email fails. + """ + # Setup + mock_domain = MagicMock(name="domain1") + mock_domain.name = "example.com" + mock_normalize_domains.return_value = [mock_domain] + + mock_requestor = MagicMock() + mock_requestor_email = "requestor@example.com" + mock_get_requestor_email.return_value = mock_requestor_email + + mock_user1 = MagicMock() + mock_user1.email = "manager1@example.com" + + email = "invitee@example.com" + is_member_of_different_org = False + + mock_send_invitation_email.side_effect = EmailSendingError("Error sending email") + + # Call and assert exception + with self.assertRaises(EmailSendingError) as context: + send_domain_invitation_email( + email=email, + requestor=mock_requestor, + domains=mock_domain, + is_member_of_different_org=is_member_of_different_org, + ) + + # Assertions + mock_normalize_domains.assert_called_once_with(mock_domain) + mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) + mock_validate_invitation.assert_called_once_with( + email, [mock_domain], mock_requestor, is_member_of_different_org + ) + self.assertEqual(str(context.exception), "Error sending email") + + @less_console_noise_decorator + @patch("registrar.utility.email_invitations.send_emails_to_domain_managers") + @patch("registrar.utility.email_invitations.validate_invitation") + @patch("registrar.utility.email_invitations.get_requestor_email") + @patch("registrar.utility.email_invitations.send_invitation_email") + @patch("registrar.utility.email_invitations.normalize_domains") + def test_send_domain_invitation_email_manager_emails_send_mail_exception( + self, + mock_normalize_domains, + mock_send_invitation_email, + mock_get_requestor_email, + mock_validate_invitation, + mock_send_domain_manager_emails, + ): + """Test sending domain invitation email for one domain and assert exception + when send_emails_to_domain_managers fails. + """ + # Setup + mock_domain = MagicMock(name="domain1") + mock_domain.name = "example.com" + mock_normalize_domains.return_value = [mock_domain] + + mock_requestor = MagicMock() + mock_requestor_email = "requestor@example.com" + mock_get_requestor_email.return_value = mock_requestor_email + + email = "invitee@example.com" + is_member_of_different_org = False + + mock_send_domain_manager_emails.side_effect = EmailSendingError("Error sending email") + + # Call and assert exception + with self.assertRaises(EmailSendingError) as context: + send_domain_invitation_email( + email=email, + requestor=mock_requestor, + domains=mock_domain, + is_member_of_different_org=is_member_of_different_org, + ) + + # Assertions + mock_normalize_domains.assert_called_once_with(mock_domain) + mock_get_requestor_email.assert_called_once_with(mock_requestor, [mock_domain]) + mock_validate_invitation.assert_called_once_with( + email, [mock_domain], mock_requestor, is_member_of_different_org + ) + mock_send_invitation_email.assert_called_once_with(email, mock_requestor_email, [mock_domain], None) + self.assertEqual(str(context.exception), "Error sending email") diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 7cce0d2b2..536d1e760 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -32,7 +32,7 @@ import tablib from unittest.mock import patch, call, MagicMock, mock_open from epplibwrapper import commands, common -from .common import MockEppLib, less_console_noise, completed_domain_request, MockSESClient +from .common import MockEppLib, less_console_noise, completed_domain_request, MockSESClient, MockDbForIndividualTests from api.tests.common import less_console_noise_decorator @@ -1516,6 +1516,91 @@ class TestCreateFederalPortfolio(TestCase): ): call_command("create_federal_portfolio", **kwargs) + @less_console_noise_decorator + def test_post_process_started_domain_requests_existing_portfolio(self): + """Ensures that federal agency is cleared when agency name matches portfolio name. + As the name implies, this implicitly tests the "post_process_started_domain_requests" function. + """ + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane", federal_type=BranchChoices.EXECUTIVE) + + # Test records with portfolios and no org names + # Create a portfolio. This script skips over "started" + portfolio = Portfolio.objects.create(organization_name="Sugarcane", creator=self.user) + # Create a domain request with matching org name + matching_request = completed_domain_request( + name="matching.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=federal_agency_2, + user=self.user, + portfolio=portfolio, + ) + + # Create a request not in started (no change should occur) + matching_request_in_wrong_status = completed_domain_request( + name="kinda-matching.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + ) + + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True) + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) + + # Refresh from db + matching_request.refresh_from_db() + matching_request_in_wrong_status.refresh_from_db() + + # Request with matching name should have federal_agency cleared + self.assertIsNone(matching_request.federal_agency) + self.assertIsNotNone(matching_request.portfolio) + self.assertEqual(matching_request.portfolio.organization_name, "Sugarcane") + + # Request with matching name but wrong state should keep its federal agency + self.assertEqual(matching_request_in_wrong_status.federal_agency, self.federal_agency) + self.assertIsNotNone(matching_request_in_wrong_status.portfolio) + self.assertEqual(matching_request_in_wrong_status.portfolio.organization_name, "Test Federal Agency") + + @less_console_noise_decorator + def test_post_process_started_domain_requests(self): + """Tests that federal agency is cleared when agency name + matches an existing portfolio's name, even if the domain request isn't + directly on that portfolio.""" + + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane", federal_type=BranchChoices.EXECUTIVE) + + # Create a request with matching federal_agency name but no direct portfolio association + matching_agency_request = completed_domain_request( + name="agency-match.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=federal_agency_2, + user=self.user, + ) + + # Create a control request that shouldn't match + non_matching_request = completed_domain_request( + name="no-match.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + ) + + # We expect the matching agency to have its fed agency cleared. + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True) + matching_agency_request.refresh_from_db() + non_matching_request.refresh_from_db() + + # Request with matching agency name should have federal_agency cleared + self.assertIsNone(matching_agency_request.federal_agency) + + # Non-matching request should keep its federal_agency + self.assertIsNotNone(non_matching_request.federal_agency) + self.assertEqual(non_matching_request.federal_agency, self.federal_agency) + + @less_console_noise_decorator def test_create_single_portfolio(self): """Test portfolio creation with suborg and senior official.""" self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) @@ -1588,6 +1673,34 @@ class TestCreateFederalPortfolio(TestCase): self.assertTrue(all([creator == User.get_default_user() for creator in creators])) self.assertTrue(all([note == "Auto-generated record" for note in notes])) + def test_script_adds_requested_suborganization_information(self): + """Tests that the script adds the requested suborg fields for domain requests""" + # Create a new domain request with some errant spacing + custom_suborg_request = completed_domain_request( + name="custom_org.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.executive_agency_2, + user=self.user, + organization_name=" requested org name ", + city="Austin ", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS, + ) + + self.assertIsNone(custom_suborg_request.requested_suborganization) + self.assertIsNone(custom_suborg_request.suborganization_city) + self.assertIsNone(custom_suborg_request.suborganization_state_territory) + + # Run the script and test it + self.run_create_federal_portfolio(branch="executive", parse_requests=True) + custom_suborg_request.refresh_from_db() + + self.assertEqual(custom_suborg_request.requested_suborganization, "requested org name") + self.assertEqual(custom_suborg_request.suborganization_city, "Austin") + self.assertEqual( + custom_suborg_request.suborganization_state_territory, DomainRequest.StateTerritoryChoices.TEXAS + ) + def test_create_multiple_portfolios_for_branch_executive(self): """Tests creating all portfolios under a given branch""" federal_choice = DomainRequest.OrganizationChoices.FEDERAL @@ -1731,3 +1844,326 @@ class TestCreateFederalPortfolio(TestCase): self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency) self.assertEqual(existing_portfolio.notes, "Old notes") self.assertEqual(existing_portfolio.creator, self.user) + + @less_console_noise_decorator + def test_post_process_suborganization_fields(self): + """Test suborganization field updates from domain and request data. + Also tests the priority order for updating city and state_territory: + 1. Domain information fields + 2. Domain request suborganization fields + 3. Domain request standard fields + """ + # Create test data with different field combinations + self.domain_info.organization_name = "super" + self.domain_info.city = "Domain City " + self.domain_info.state_territory = "NY" + self.domain_info.save() + + self.domain_request.organization_name = "super" + self.domain_request.suborganization_city = "Request Suborg City" + self.domain_request.suborganization_state_territory = "CA" + self.domain_request.city = "Request City" + self.domain_request.state_territory = "TX" + self.domain_request.save() + + # Create another request/info pair without domain info data + self.domain_info_2.organization_name = "creative" + self.domain_info_2.city = None + self.domain_info_2.state_territory = None + self.domain_info_2.save() + + self.domain_request_2.organization_name = "creative" + self.domain_request_2.suborganization_city = "Second Suborg City" + self.domain_request_2.suborganization_state_territory = "WA" + self.domain_request_2.city = "Second City" + self.domain_request_2.state_territory = "OR" + self.domain_request_2.save() + + # Create a third request/info pair without suborg data + self.domain_info_3.organization_name = "names" + self.domain_info_3.city = None + self.domain_info_3.state_territory = None + self.domain_info_3.save() + + self.domain_request_3.organization_name = "names" + self.domain_request_3.suborganization_city = None + self.domain_request_3.suborganization_state_territory = None + self.domain_request_3.city = "Third City" + self.domain_request_3.state_territory = "FL" + self.domain_request_3.save() + + # Test running the script with both, and just with parse_requests + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True, parse_domains=True) + self.run_create_federal_portfolio( + agency_name="Executive Agency 1", + parse_requests=True, + ) + + self.domain_info.refresh_from_db() + self.domain_request.refresh_from_db() + self.domain_info_2.refresh_from_db() + self.domain_request_2.refresh_from_db() + self.domain_info_3.refresh_from_db() + self.domain_request_3.refresh_from_db() + + # Verify suborganizations were created with correct field values + # Should use domain info values + suborg_1 = Suborganization.objects.get(name=self.domain_info.organization_name) + self.assertEqual(suborg_1.city, "Domain City") + self.assertEqual(suborg_1.state_territory, "NY") + + # Should use domain request suborg values + suborg_2 = Suborganization.objects.get(name=self.domain_info_2.organization_name) + self.assertEqual(suborg_2.city, "Second Suborg City") + self.assertEqual(suborg_2.state_territory, "WA") + + # Should use domain request standard values + suborg_3 = Suborganization.objects.get(name=self.domain_info_3.organization_name) + self.assertEqual(suborg_3.city, "Third City") + self.assertEqual(suborg_3.state_territory, "FL") + + @less_console_noise_decorator + def test_post_process_suborganization_fields_duplicate_records(self): + """Test suborganization field updates when multiple domains/requests exist for the same org. + Tests that: + 1. City / state_territory us updated when all location info matches + 2. Updates are skipped when locations don't match + 3. Priority order is maintained across multiple records: + a. Domain information fields + b. Domain request suborganization fields + c. Domain request standard fields + """ + # Case 1: Multiple records with all fields matching + matching_request_1 = completed_domain_request( + name="matching1.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + organization_name="matching org", + city="Standard City", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS, + suborganization_city="Suborg City", + suborganization_state_territory=DomainRequest.StateTerritoryChoices.CALIFORNIA, + federal_agency=self.federal_agency, + ) + matching_request_1.approve() + domain_info_1 = DomainInformation.objects.get(domain_request=matching_request_1) + domain_info_1.city = "Domain Info City" + domain_info_1.state_territory = DomainRequest.StateTerritoryChoices.NEW_YORK + domain_info_1.save() + + matching_request_2 = completed_domain_request( + name="matching2.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + organization_name="matching org", + city="Standard City", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS, + suborganization_city="Suborg City", + suborganization_state_territory=DomainRequest.StateTerritoryChoices.CALIFORNIA, + federal_agency=self.federal_agency, + ) + matching_request_2.approve() + domain_info_2 = DomainInformation.objects.get(domain_request=matching_request_2) + domain_info_2.city = "Domain Info City" + domain_info_2.state_territory = DomainRequest.StateTerritoryChoices.NEW_YORK + domain_info_2.save() + + # Case 2: Multiple records with only request fields (no domain info) + request_only_1 = completed_domain_request( + name="request1.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + organization_name="request org", + city="Standard City", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS, + suborganization_city="Suborg City", + suborganization_state_territory=DomainRequest.StateTerritoryChoices.CALIFORNIA, + federal_agency=self.federal_agency, + ) + request_only_1.approve() + domain_info_3 = DomainInformation.objects.get(domain_request=request_only_1) + domain_info_3.city = None + domain_info_3.state_territory = None + domain_info_3.save() + + request_only_2 = completed_domain_request( + name="request2.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + organization_name="request org", + city="Standard City", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS, + suborganization_city="Suborg City", + suborganization_state_territory=DomainRequest.StateTerritoryChoices.CALIFORNIA, + federal_agency=self.federal_agency, + ) + request_only_2.approve() + domain_info_4 = DomainInformation.objects.get(domain_request=request_only_2) + domain_info_4.city = None + domain_info_4.state_territory = None + domain_info_4.save() + + # Case 3: Multiple records with only standard fields (no suborg) + standard_only_1 = completed_domain_request( + name="standard1.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + organization_name="standard org", + city="Standard City", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS, + federal_agency=self.federal_agency, + ) + standard_only_1.approve() + domain_info_5 = DomainInformation.objects.get(domain_request=standard_only_1) + domain_info_5.city = None + domain_info_5.state_territory = None + domain_info_5.save() + + standard_only_2 = completed_domain_request( + name="standard2.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + organization_name="standard org", + city="Standard City", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS, + federal_agency=self.federal_agency, + ) + standard_only_2.approve() + domain_info_6 = DomainInformation.objects.get(domain_request=standard_only_2) + domain_info_6.city = None + domain_info_6.state_territory = None + domain_info_6.save() + + # Case 4: Multiple records with mismatched locations + mismatch_request_1 = completed_domain_request( + name="mismatch1.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + organization_name="mismatch org", + city="City One", + state_territory=DomainRequest.StateTerritoryChoices.FLORIDA, + federal_agency=self.federal_agency, + ) + mismatch_request_1.approve() + domain_info_5 = DomainInformation.objects.get(domain_request=mismatch_request_1) + domain_info_5.city = "Different City" + domain_info_5.state_territory = DomainRequest.StateTerritoryChoices.ALASKA + domain_info_5.save() + + mismatch_request_2 = completed_domain_request( + name="mismatch2.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + organization_name="mismatch org", + city="City Two", + state_territory=DomainRequest.StateTerritoryChoices.HAWAII, + federal_agency=self.federal_agency, + ) + mismatch_request_2.approve() + domain_info_6 = DomainInformation.objects.get(domain_request=mismatch_request_2) + domain_info_6.city = "Another City" + domain_info_6.state_territory = DomainRequest.StateTerritoryChoices.CALIFORNIA + domain_info_6.save() + + # Run the portfolio creation script + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True, parse_domains=True) + + # Case 1: Should use domain info values (highest priority) + matching_suborg = Suborganization.objects.get(name="matching org") + self.assertEqual(matching_suborg.city, "Domain Info City") + self.assertEqual(matching_suborg.state_territory, DomainRequest.StateTerritoryChoices.NEW_YORK) + + # Case 2: Should use suborg values (second priority) + request_suborg = Suborganization.objects.get(name="request org") + self.assertEqual(request_suborg.city, "Suborg City") + self.assertEqual(request_suborg.state_territory, DomainRequest.StateTerritoryChoices.CALIFORNIA) + + # Case 3: Should use standard values (lowest priority) + standard_suborg = Suborganization.objects.get(name="standard org") + self.assertEqual(standard_suborg.city, "Standard City") + self.assertEqual(standard_suborg.state_territory, DomainRequest.StateTerritoryChoices.TEXAS) + + # Case 4: Should skip update due to mismatched locations + mismatch_suborg = Suborganization.objects.get(name="mismatch org") + self.assertIsNone(mismatch_suborg.city) + self.assertIsNone(mismatch_suborg.state_territory) + + +class TestPatchSuborganizations(MockDbForIndividualTests): + """Tests for the patch_suborganizations management command.""" + + @less_console_noise_decorator + def run_patch_suborganizations(self): + """Helper method to run the patch_suborganizations command.""" + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.prompt_for_execution", + return_value=True, + ): + call_command("patch_suborganizations") + + @less_console_noise_decorator + def test_space_and_case_duplicates(self): + """Test cleaning up duplicates that differ by spaces and case. + + Should keep the version with: + 1. Fewest spaces + 2. Most leading capitals + """ + # Delete any other suborganizations defined in the initial test dataset + DomainRequest.objects.all().delete() + Suborganization.objects.all().delete() + + Suborganization.objects.create(name="Test Organization ", portfolio=self.portfolio_1) + Suborganization.objects.create(name="test organization", portfolio=self.portfolio_1) + Suborganization.objects.create(name="Test Organization", portfolio=self.portfolio_1) + + # Create an unrelated record to test that it doesn't get deleted, too + Suborganization.objects.create(name="unrelated org", portfolio=self.portfolio_1) + self.run_patch_suborganizations() + self.assertEqual(Suborganization.objects.count(), 2) + self.assertEqual(Suborganization.objects.filter(name__in=["unrelated org", "Test Organization"]).count(), 2) + + @less_console_noise_decorator + def test_hardcoded_record(self): + """Tests that our hardcoded records update as we expect them to""" + # Delete any other suborganizations defined in the initial test dataset + DomainRequest.objects.all().delete() + Suborganization.objects.all().delete() + + # Create orgs with old and new name formats + old_name = "USDA/OC" + new_name = "USDA, Office of Communications" + + Suborganization.objects.create(name=old_name, portfolio=self.portfolio_1) + Suborganization.objects.create(name=new_name, portfolio=self.portfolio_1) + + self.run_patch_suborganizations() + + # Verify only the new one of the two remains + self.assertEqual(Suborganization.objects.count(), 1) + remaining = Suborganization.objects.first() + self.assertEqual(remaining.name, new_name) + + @less_console_noise_decorator + def test_reference_updates(self): + """Test that references are updated on domain info and domain request before deletion.""" + # Create suborganizations + keep_org = Suborganization.objects.create(name="Test Organization", portfolio=self.portfolio_1) + delete_org = Suborganization.objects.create(name="test organization ", portfolio=self.portfolio_1) + unrelated_org = Suborganization.objects.create(name="awesome", portfolio=self.portfolio_1) + + # We expect these references to update + self.domain_request_1.sub_organization = delete_org + self.domain_information_1.sub_organization = delete_org + self.domain_request_1.save() + self.domain_information_1.save() + + # But not these ones + self.domain_request_2.sub_organization = unrelated_org + self.domain_information_2.sub_organization = unrelated_org + self.domain_request_2.save() + self.domain_information_2.save() + + self.run_patch_suborganizations() + + self.domain_request_1.refresh_from_db() + self.domain_information_1.refresh_from_db() + self.domain_request_2.refresh_from_db() + self.domain_information_2.refresh_from_db() + + self.assertEqual(self.domain_request_1.sub_organization, keep_org) + self.assertEqual(self.domain_information_1.sub_organization, keep_org) + self.assertEqual(self.domain_request_2.sub_organization, unrelated_org) + self.assertEqual(self.domain_information_2.sub_organization, unrelated_org) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 46604a44a..d8db0f043 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -28,6 +28,7 @@ from registrar.models.verified_by_staff import VerifiedByStaff # type: ignore from .common import ( MockSESClient, completed_domain_request, + create_superuser, create_test_user, ) from waffle.testutils import override_flag @@ -155,6 +156,7 @@ class TestPortfolioInvitations(TestCase): roles=[self.portfolio_role_base, self.portfolio_role_admin], additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], ) + self.superuser = create_superuser() def tearDown(self): super().tearDown() @@ -294,10 +296,158 @@ class TestPortfolioInvitations(TestCase): # Verify self.assertEquals(self.invitation.get_portfolio_permissions(), perm_list) + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=False) + def test_clean_multiple_portfolios_inactive(self): + """Tests that users cannot have multiple portfolios or invitations when flag is inactive""" + # Create the first portfolio permission + UserPortfolioPermission.objects.create( + user=self.superuser, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + # Test a second portfolio permission object (should fail) + second_portfolio = Portfolio.objects.create(organization_name="Second Portfolio", creator=self.superuser) + second_permission = UserPortfolioPermission( + user=self.superuser, portfolio=second_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + with self.assertRaises(ValidationError) as err: + second_permission.clean() + self.assertIn("users cannot be assigned to multiple portfolios", str(err.exception)) + + # Test that adding a new portfolio invitation also fails + third_portfolio = Portfolio.objects.create(organization_name="Third Portfolio", creator=self.superuser) + invitation = PortfolioInvitation( + email=self.superuser.email, portfolio=third_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + with self.assertRaises(ValidationError) as err: + invitation.clean() + self.assertIn("users cannot be assigned to multiple portfolios", str(err.exception)) + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=True) + def test_clean_multiple_portfolios_active(self): + """Tests that users can have multiple portfolios and invitations when flag is active""" + # Create first portfolio permission + UserPortfolioPermission.objects.create( + user=self.superuser, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + # Second portfolio permission should succeed + second_portfolio = Portfolio.objects.create(organization_name="Second Portfolio", creator=self.superuser) + second_permission = UserPortfolioPermission( + user=self.superuser, portfolio=second_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + second_permission.clean() + second_permission.save() + + # Verify both permissions exist + user_permissions = UserPortfolioPermission.objects.filter(user=self.superuser) + self.assertEqual(user_permissions.count(), 2) + + # Portfolio invitation should also succeed + third_portfolio = Portfolio.objects.create(organization_name="Third Portfolio", creator=self.superuser) + invitation = PortfolioInvitation( + email=self.superuser.email, portfolio=third_portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + invitation.clean() + invitation.save() + + # Verify invitation exists + self.assertTrue( + PortfolioInvitation.objects.filter( + email=self.superuser.email, + portfolio=third_portfolio, + ).exists() + ) + + @less_console_noise_decorator + def test_clean_portfolio_invitation(self): + """Tests validation of portfolio invitation permissions""" + + # Test validation fails when portfolio missing but permissions present + invitation = PortfolioInvitation(email="test@example.com", roles=["organization_admin"], portfolio=None) + with self.assertRaises(ValidationError) as err: + invitation.clean() + self.assertEqual( + str(err.exception), + "When portfolio roles or additional permissions are assigned, portfolio is required.", + ) + + # Test validation fails when portfolio present but no permissions + invitation = PortfolioInvitation(email="test@example.com", roles=None, portfolio=self.portfolio) + with self.assertRaises(ValidationError) as err: + invitation.clean() + self.assertEqual( + str(err.exception), + "When portfolio is assigned, portfolio roles or additional permissions are required.", + ) + + # Test validation fails with forbidden permissions + forbidden_member_roles = UserPortfolioPermission.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get( + UserPortfolioRoleChoices.ORGANIZATION_MEMBER + ) + invitation = PortfolioInvitation( + email="test@example.com", + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=forbidden_member_roles, + portfolio=self.portfolio, + ) + with self.assertRaises(ValidationError) as err: + invitation.clean() + self.assertEqual( + str(err.exception), + "These permissions cannot be assigned to Member: " + "", + ) + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=False) + def test_clean_user_portfolio_permission_multiple_portfolios_flag_off_and_duplicate_permission(self): + """MISSING TEST: Test validation of multiple_portfolios flag. + Scenario 1: Flag is inactive, and the user has existing portfolio permissions + + NOTE: Refer to the same test under TestUserPortfolioPermission""" + + pass + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=False) + def test_clean_user_portfolio_permission_multiple_portfolios_flag_off_and_existing_invitation(self): + """MISSING TEST: Test validation of multiple_portfolios flag. + Scenario 2: Flag is inactive, and the user has existing portfolio invitation to another portfolio + + NOTE: Refer to the same test under TestUserPortfolioPermission""" + + pass + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=True) + def test_clean_user_portfolio_permission_multiple_portfolios_flag_on_and_duplicate_permission(self): + """MISSING TEST: Test validation of multiple_portfolios flag. + Scenario 3: Flag is active, and the user has existing portfolio invitation + + NOTE: Refer to the same test under TestUserPortfolioPermission""" + + pass + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=True) + def test_clean_user_portfolio_permission_multiple_portfolios_flag_on_and_existing_invitation(self): + """MISSING TEST: Test validation of multiple_portfolios flag. + Scenario 4: Flag is active, and the user has existing portfolio invitation to another portfolio + + NOTE: Refer to the same test under TestUserPortfolioPermission""" + + pass + class TestUserPortfolioPermission(TestCase): @less_console_noise_decorator def setUp(self): + self.superuser = create_superuser() + self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser) self.user, _ = User.objects.get_or_create(email="mayor@igorville.gov") self.user2, _ = User.objects.get_or_create(email="user2@igorville.gov", username="user2") super().setUp() @@ -311,6 +461,7 @@ class TestUserPortfolioPermission(TestCase): Portfolio.objects.all().delete() User.objects.all().delete() UserDomainRole.objects.all().delete() + PortfolioInvitation.objects.all().delete() @less_console_noise_decorator @override_flag("multiple_portfolios", active=True) @@ -427,6 +578,178 @@ class TestUserPortfolioPermission(TestCase): # Assert self.assertEqual(portfolio_permission.get_managed_domains_count(), 1) + @less_console_noise_decorator + def test_clean_user_portfolio_permission(self): + """Tests validation of user portfolio permission""" + + # Test validation fails when portfolio missing but permissions are present + permission = UserPortfolioPermission(user=self.superuser, roles=["organization_admin"], portfolio=None) + with self.assertRaises(ValidationError) as err: + permission.clean() + self.assertEqual( + str(err.exception), + "When portfolio roles or additional permissions are assigned, portfolio is required.", + ) + + # Test validation fails when portfolio present but no permissions are present + permission = UserPortfolioPermission(user=self.superuser, roles=None, portfolio=self.portfolio) + with self.assertRaises(ValidationError) as err: + permission.clean() + self.assertEqual( + str(err.exception), + "When portfolio is assigned, portfolio roles or additional permissions are required.", + ) + + # Test validation fails with forbidden permissions for single role + forbidden_member_roles = UserPortfolioPermission.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get( + UserPortfolioRoleChoices.ORGANIZATION_MEMBER + ) + permission = UserPortfolioPermission( + user=self.superuser, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=forbidden_member_roles, + portfolio=self.portfolio, + ) + with self.assertRaises(ValidationError) as err: + permission.clean() + self.assertEqual( + str(err.exception), + "These permissions cannot be assigned to Member: " + "", + ) + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=False) + def test_clean_user_portfolio_permission_multiple_portfolios_flag_off_and_duplicate_permission(self): + """Test validation of multiple_portfolios flag. + Scenario 1: Flag is inactive, and the user has existing portfolio permissions""" + + # existing permission + UserPortfolioPermission.objects.create( + user=self.superuser, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + portfolio=self.portfolio, + ) + + permission = UserPortfolioPermission( + user=self.superuser, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + portfolio=self.portfolio, + ) + + with self.assertRaises(ValidationError) as err: + permission.clean() + + self.assertEqual( + str(err.exception.messages[0]), + "This user is already assigned to a portfolio. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios.", + ) + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=False) + def test_clean_user_portfolio_permission_multiple_portfolios_flag_off_and_existing_invitation(self): + """Test validation of multiple_portfolios flag. + Scenario 2: Flag is inactive, and the user has existing portfolio invitation to another portfolio""" + + portfolio2 = Portfolio.objects.create(creator=self.superuser, organization_name="Joey go away") + + PortfolioInvitation.objects.create( + email=self.superuser.email, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], portfolio=portfolio2 + ) + + permission = UserPortfolioPermission( + user=self.superuser, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + portfolio=self.portfolio, + ) + + with self.assertRaises(ValidationError) as err: + permission.clean() + + self.assertEqual( + str(err.exception.messages[0]), + "This user is already assigned to a portfolio invitation. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios.", + ) + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=True) + def test_clean_user_portfolio_permission_multiple_portfolios_flag_on_and_duplicate_permission(self): + """Test validation of multiple_portfolios flag. + Scenario 3: Flag is active, and the user has existing portfolio invitation""" + + # existing permission + UserPortfolioPermission.objects.create( + user=self.superuser, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + portfolio=self.portfolio, + ) + + permission = UserPortfolioPermission( + user=self.superuser, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + portfolio=self.portfolio, + ) + + # Should not raise any exceptions + try: + permission.clean() + except ValidationError: + self.fail("ValidationError was raised unexpectedly when flag is active.") + + @less_console_noise_decorator + @override_flag("multiple_portfolios", active=True) + def test_clean_user_portfolio_permission_multiple_portfolios_flag_on_and_existing_invitation(self): + """Test validation of multiple_portfolios flag. + Scenario 4: Flag is active, and the user has existing portfolio invitation to another portfolio""" + + portfolio2 = Portfolio.objects.create(creator=self.superuser, organization_name="Joey go away") + + PortfolioInvitation.objects.create( + email=self.superuser.email, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], portfolio=portfolio2 + ) + + permission = UserPortfolioPermission( + user=self.superuser, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + portfolio=self.portfolio, + ) + + # Should not raise any exceptions + try: + permission.clean() + except ValidationError: + self.fail("ValidationError was raised unexpectedly when flag is active.") + + @less_console_noise_decorator + def test_get_forbidden_permissions_with_multiple_roles(self): + """Tests that forbidden permissions are properly handled when a user has multiple roles""" + # Get forbidden permissions for member role + member_forbidden = UserPortfolioPermission.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get( + UserPortfolioRoleChoices.ORGANIZATION_MEMBER + ) + + # Test with both admin and member roles + roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN, UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + + # These permissions would be forbidden for member alone, but should be allowed + # when combined with admin role + permissions = UserPortfolioPermission.get_forbidden_permissions( + roles=roles, additional_permissions=member_forbidden + ) + + # Should return empty set since no permissions are commonly forbidden between admin and member + self.assertEqual(permissions, set()) + + # Verify the same permissions are forbidden when only member role is present + member_only_permissions = UserPortfolioPermission.get_forbidden_permissions( + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], additional_permissions=member_forbidden + ) + + # Should return the forbidden permissions for member role + self.assertEqual(member_only_permissions, set(member_forbidden)) + class TestUser(TestCase): """Test actions that occur on user login, diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 15a88a608..083725a55 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -349,6 +349,70 @@ class TestDomainCache(MockEppLib): class TestDomainCreation(MockEppLib): """Rule: An approved domain request must result in a domain""" + @less_console_noise_decorator + def test_get_or_create_public_contact_race_condition(self): + """ + Scenario: Two processes try to create the same security contact simultaneously + Given a domain in UNKNOWN state + When a race condition occurs during contact creation + Then no IntegrityError is raised + And only one security contact exists in database + And the correct public contact is returned + + CONTEXT: We ran into an intermittent but somewhat rare issue where IntegrityError + was raised when creating PublicContact. + Per our logs, this seemed to appear during periods of high app activity. + """ + domain, _ = Domain.objects.get_or_create(name="defaultsecurity.gov") + + self.first_call = True + + def mock_filter(*args, **kwargs): + """Simulates a race condition by creating a + duplicate contact between the first filter and save. + """ + # Return an empty queryset for the first call. Otherwise just proceed as normal. + if self.first_call: + self.first_call = False + duplicate = PublicContact( + domain=domain, + contact_type=PublicContact.ContactTypeChoices.SECURITY, + registry_id="defaultSec", + email="dotgov@cisa.dhs.gov", + name="Registry Customer Service", + ) + duplicate.save(skip_epp_save=True) + return PublicContact.objects.none() + + return PublicContact.objects.filter(*args, **kwargs) + + with patch.object(PublicContact.objects, "filter", side_effect=mock_filter): + try: + public_contact = PublicContact( + domain=domain, + contact_type=PublicContact.ContactTypeChoices.SECURITY, + registry_id="defaultSec", + email="dotgov@cisa.dhs.gov", + name="Registry Customer Service", + ) + returned_public_contact = domain._get_or_create_public_contact(public_contact) + except IntegrityError: + self.fail( + "IntegrityError was raised during contact creation due to a race condition. " + "This indicates that concurrent contact creation is not working in some cases. " + "The error occurs when two processes try to create the same contact simultaneously. " + "Expected behavior: gracefully handle duplicate creation and return existing contact." + ) + + # Verify that only one contact exists and its correctness + security_contacts = PublicContact.objects.filter( + domain=domain, contact_type=PublicContact.ContactTypeChoices.SECURITY + ) + self.assertEqual(security_contacts.count(), 1) + self.assertEqual(returned_public_contact, security_contacts.get()) + self.assertEqual(returned_public_contact.registry_id, "defaultSec") + self.assertEqual(returned_public_contact.email, "dotgov@cisa.dhs.gov") + @boto3_mocking.patching def test_approved_domain_request_creates_domain_locally(self): """ diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 4a41238c7..9d410e430 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -729,6 +729,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): # "Submitted at", "Status", "Domain type", + "Portfolio", "Federal type", "Federal agency", "Organization name", @@ -736,6 +737,10 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): "City", "State/territory", "Region", + "Suborganization", + "Requested suborg", + "Suborg city", + "Suborg state/territory", "Creator first name", "Creator last name", "Creator email", @@ -765,28 +770,30 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): expected_content = ( # Header - "Domain request,Status,Domain type,Federal type,Federal agency,Organization name,Election office," - "City,State/territory,Region,Creator first name,Creator last name,Creator email," + "Domain request,Status,Domain type,Portfolio,Federal type,Federal agency,Organization name," + "Election office,City,State/territory,Region,Suborganization,Requested suborg,Suborg city," + "Suborg state/territory,Creator first name,Creator last name,Creator email," "Creator approved domains count,Creator active requests count,Alternative domains,SO first name," "SO last name,SO email,SO title/role,Request purpose,Request additional details,Other contacts," "CISA regional representative,Current websites,Investigator\n" # Content - "city5.gov,Approved,Federal,Executive,,Testorg,N/A,,NY,2,,,,1,0,city1.gov,Testy,Tester,testy@town.com," - "Chief Tester,Purpose of the site,There is more,Testy Tester testy2@town.com,,city.com,\n" - "city2.gov,In review,Federal,Executive,Portfolio 1 Federal Agency,,N/A,,NY,2,,,,0,1,city1.gov,,,,," - "Purpose of the site,There is more,Testy Tester testy2@town.com,,city.com,\n" - "city3.gov,Submitted,Federal,Executive,Portfolio 1 Federal Agency,,N/A,,NY,2,,,,0,1," + "city5.gov,Approved,Federal,No,Executive,,Testorg,N/A,,NY,2,requested_suborg,SanFran,CA,,,,,1,0," + "city1.gov,Testy,Tester,testy@town.com,Chief Tester,Purpose of the site,There is more," + "Testy Tester testy2@town.com,,city.com,\n" + "city2.gov,In review,Federal,Yes,Executive,Portfolio 1 Federal Agency,,N/A,,,2,SubOrg 1,,,,,,,0," + "1,city1.gov,,,,,Purpose of the site,There is more,Testy Tester testy2@town.com,,city.com,\n" + "city3.gov,Submitted,Federal,Yes,Executive,Portfolio 1 Federal Agency,,N/A,,,2,,,,,,,,0,1," '"cheeseville.gov, city1.gov, igorville.gov",,,,,Purpose of the site,CISA-first-name CISA-last-name | ' 'There is more,"Meow Tester24 te2@town.com, Testy1232 Tester24 te2@town.com, ' 'Testy Tester testy2@town.com",' 'test@igorville.com,"city.com, https://www.example2.com, https://www.example.com",\n' - "city4.gov,Submitted,City,Executive,,Testorg,Yes,,NY,2,,,,0,1,city1.gov,Testy," + "city4.gov,Submitted,City,No,Executive,,Testorg,Yes,,NY,2,,,,,,,,0,1,city1.gov,Testy," "Tester,testy@town.com," "Chief Tester,Purpose of the site,CISA-first-name CISA-last-name | There is more," "Testy Tester testy2@town.com," "cisaRep@igorville.gov,city.com,\n" - "city6.gov,Submitted,Federal,Executive,Portfolio 1 Federal Agency,,N/A,,NY,2,,,,0,1,city1.gov,,,,," - "Purpose of the site,CISA-first-name CISA-last-name | There is more,Testy Tester testy2@town.com," + "city6.gov,Submitted,Federal,Yes,Executive,Portfolio 1 Federal Agency,,N/A,,,2,,,,,,,,0,1,city1.gov," + ",,,,Purpose of the site,CISA-first-name CISA-last-name | There is more,Testy Tester testy2@town.com," "cisaRep@igorville.gov,city.com,\n" ) @@ -900,6 +907,7 @@ class MemberExportTest(MockDbForIndividualTests, MockEppLib): # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() + self.maxDiff = None self.assertEqual(csv_content, expected_content) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index aedfc41c2..45758e502 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -9,7 +9,7 @@ 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 -from .common import MockEppLib, MockSESClient, create_user # type: ignore +from .common import MockEppLib, create_user # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore @@ -439,35 +439,47 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): username="usertest", ) - self.expiringdomain, _ = Domain.objects.get_or_create( - name="expiringdomain.gov", + self.domaintorenew, _ = Domain.objects.get_or_create( + name="domainrenewal.gov", ) UserDomainRole.objects.get_or_create( - user=self.user, domain=self.expiringdomain, role=UserDomainRole.Roles.MANAGER + user=self.user, domain=self.domaintorenew, role=UserDomainRole.Roles.MANAGER ) - DomainInformation.objects.get_or_create(creator=self.user, domain=self.expiringdomain) + DomainInformation.objects.get_or_create(creator=self.user, domain=self.domaintorenew) self.portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org", creator=self.user) self.user.save() - def custom_is_expired(self): + def expiration_date_one_year_out(self): + todays_date = datetime.today() + new_expiration_date = todays_date.replace(year=todays_date.year + 1) + return new_expiration_date + + def custom_is_expired_false(self): return False + def custom_is_expired_true(self): + return True + def custom_is_expiring(self): return True + def custom_renew_domain(self): + self.domain_with_ip.expiration_date = self.expiration_date_one_year_out() + self.domain_with_ip.save() + @override_flag("domain_renewal", active=True) def test_expiring_domain_on_detail_page_as_domain_manager(self): self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( - Domain, "is_expired", self.custom_is_expired + Domain, "is_expired", self.custom_is_expired_false ): - self.assertEquals(self.expiringdomain.state, Domain.State.UNKNOWN) + self.assertEquals(self.domaintorenew.state, Domain.State.UNKNOWN) detail_page = self.client.get( - reverse("domain", kwargs={"pk": self.expiringdomain.id}), + reverse("domain", kwargs={"pk": self.domaintorenew.id}), ) self.assertContains(detail_page, "Expiring soon") @@ -498,17 +510,17 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, ], ) - expiringdomain2, _ = Domain.objects.get_or_create(name="bogusdomain2.gov") + domaintorenew2, _ = Domain.objects.get_or_create(name="bogusdomain2.gov") DomainInformation.objects.get_or_create( - creator=non_dom_manage_user, domain=expiringdomain2, portfolio=self.portfolio + creator=non_dom_manage_user, domain=domaintorenew2, portfolio=self.portfolio ) non_dom_manage_user.refresh_from_db() self.client.force_login(non_dom_manage_user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( - Domain, "is_expired", self.custom_is_expired + Domain, "is_expired", self.custom_is_expired_false ): detail_page = self.client.get( - reverse("domain", kwargs={"pk": expiringdomain2.id}), + reverse("domain", kwargs={"pk": domaintorenew2.id}), ) self.assertContains(detail_page, "Contact one of the listed domain managers to renew the domain.") @@ -517,20 +529,164 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): def test_expiring_domain_on_detail_page_in_org_model_as_a_domain_manager(self): portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org2", creator=self.user) - expiringdomain3, _ = Domain.objects.get_or_create(name="bogusdomain3.gov") + domaintorenew3, _ = Domain.objects.get_or_create(name="bogusdomain3.gov") - UserDomainRole.objects.get_or_create(user=self.user, domain=expiringdomain3, role=UserDomainRole.Roles.MANAGER) - DomainInformation.objects.get_or_create(creator=self.user, domain=expiringdomain3, portfolio=portfolio) + UserDomainRole.objects.get_or_create(user=self.user, domain=domaintorenew3, role=UserDomainRole.Roles.MANAGER) + DomainInformation.objects.get_or_create(creator=self.user, domain=domaintorenew3, portfolio=portfolio) self.user.refresh_from_db() self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( - Domain, "is_expired", self.custom_is_expired + Domain, "is_expired", self.custom_is_expired_false ): detail_page = self.client.get( - reverse("domain", kwargs={"pk": expiringdomain3.id}), + reverse("domain", kwargs={"pk": domaintorenew3.id}), ) self.assertContains(detail_page, "Renew to maintain access") + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_and_sidebar_expiring(self): + self.client.force_login(self.user) + with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( + Domain, "is_expiring", self.custom_is_expiring + ): + # Grab the detail page + detail_page = self.client.get( + reverse("domain", kwargs={"pk": self.domaintorenew.id}), + ) + + # Make sure we see the link as a domain manager + self.assertContains(detail_page, "Renew to maintain access") + + # Make sure we can see Renewal form on the sidebar since it's expiring + self.assertContains(detail_page, "Renewal form") + + # Grab link to the renewal page + renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domaintorenew.id}) + self.assertContains(detail_page, f'href="{renewal_form_url}"') + + # Simulate clicking the link + response = self.client.get(renewal_form_url) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, f"Renew {self.domaintorenew.name}") + + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_and_sidebar_expired(self): + + self.client.force_login(self.user) + + with patch.object(Domain, "is_expired", self.custom_is_expired_true), patch.object( + Domain, "is_expired", self.custom_is_expired_true + ): + # Grab the detail page + detail_page = self.client.get( + reverse("domain", kwargs={"pk": self.domaintorenew.id}), + ) + + print("puglesss", self.domaintorenew.is_expired) + # Make sure we see the link as a domain manager + self.assertContains(detail_page, "Renew to maintain access") + + # Make sure we can see Renewal form on the sidebar since it's expired + self.assertContains(detail_page, "Renewal form") + + # Grab link to the renewal page + renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domaintorenew.id}) + self.assertContains(detail_page, f'href="{renewal_form_url}"') + + # Simulate clicking the link + response = self.client.get(renewal_form_url) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, f"Renew {self.domaintorenew.name}") + + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_your_contact_info_edit(self): + with less_console_noise(): + # Start on the Renewal page for the domain + renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) + + # Verify we see "Your contact information" on the renewal form + self.assertContains(renewal_page, "Your contact information") + + # Verify that the "Edit" button for Your contact is there and links to correct URL + edit_button_url = reverse("user-profile") + self.assertContains(renewal_page, f'href="{edit_button_url}"') + + # Simulate clicking on edit button + edit_page = renewal_page.click(href=edit_button_url, index=1) + self.assertEqual(edit_page.status_code, 200) + self.assertContains(edit_page, "Review the details below and update any required information") + + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_security_email_edit(self): + with less_console_noise(): + # Start on the Renewal page for the domain + renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) + + # Verify we see "Security email" on the renewal form + self.assertContains(renewal_page, "Security email") + + # Verify we see "strong recommend" blurb + self.assertContains(renewal_page, "We strongly recommend that you provide a security email.") + + # Verify that the "Edit" button for Security email is there and links to correct URL + edit_button_url = reverse("domain-security-email", kwargs={"pk": self.domain_with_ip.id}) + self.assertContains(renewal_page, f'href="{edit_button_url}"') + + # Simulate clicking on edit button + edit_page = renewal_page.click(href=edit_button_url, index=1) + self.assertEqual(edit_page.status_code, 200) + self.assertContains(edit_page, "A security contact should be capable of evaluating") + + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_domain_manager_edit(self): + with less_console_noise(): + # Start on the Renewal page for the domain + renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) + + # Verify we see "Domain managers" on the renewal form + self.assertContains(renewal_page, "Domain managers") + + # Verify that the "Edit" button for Domain managers is there and links to correct URL + edit_button_url = reverse("domain-users", kwargs={"pk": self.domain_with_ip.id}) + self.assertContains(renewal_page, f'href="{edit_button_url}"') + + # Simulate clicking on edit button + edit_page = renewal_page.click(href=edit_button_url, index=1) + self.assertEqual(edit_page.status_code, 200) + self.assertContains(edit_page, "Domain managers can update all information related to a domain") + + @override_flag("domain_renewal", active=True) + def test_ack_checkbox_not_checked(self): + + # Grab the renewal URL + renewal_url = reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id}) + + # Test that the checkbox is not checked + response = self.client.post(renewal_url, data={"submit_button": "next"}) + + error_message = "Check the box if you read and agree to the requirements for operating a .gov domain." + self.assertContains(response, error_message) + + @override_flag("domain_renewal", active=True) + def test_ack_checkbox_checked(self): + + # Grab the renewal URL + with patch.object(Domain, "renew_domain", self.custom_renew_domain): + renewal_url = reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id}) + + # Click the check, and submit + response = self.client.post(renewal_url, data={"is_policy_acknowledged": "on", "submit_button": "next"}) + + # Check that it redirects after a successfully submits + self.assertRedirects(response, reverse("domain", kwargs={"pk": self.domain_with_ip.id})) + + # Check for the updated expiration + formatted_new_expiration_date = self.expiration_date_one_year_out().strftime("%b. %-d, %Y") + redirect_response = self.client.get(reverse("domain", kwargs={"pk": self.domain_with_ip.id}), follow=True) + self.assertContains(redirect_response, formatted_new_expiration_date) + class TestDomainManagers(TestDomainOverview): @classmethod @@ -564,6 +720,8 @@ class TestDomainManagers(TestDomainOverview): def tearDown(self): """Ensure that the user has its original permissions""" PortfolioInvitation.objects.all().delete() + UserPortfolioPermission.objects.all().delete() + User.objects.exclude(id=self.user.id).delete() super().tearDown() @less_console_noise_decorator @@ -592,11 +750,12 @@ class TestDomainManagers(TestDomainOverview): response = self.client.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) self.assertContains(response, "Add a domain manager") - @boto3_mocking.patching @less_console_noise_decorator - def test_domain_user_add_form(self): + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_user_add_form(self, mock_send_domain_email): """Adding an existing user works.""" get_user_model().objects.get_or_create(email="mayor@igorville.gov") + user = User.objects.filter(email="mayor@igorville.gov").first() add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -604,10 +763,15 @@ class TestDomainManagers(TestDomainOverview): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - mock_client = MockSESClient() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with less_console_noise(): - success_result = add_page.form.submit() + success_result = add_page.form.submit() + + mock_send_domain_email.assert_called_once_with( + email="mayor@igorville.gov", + requestor=self.user, + domains=self.domain, + is_member_of_different_org=None, + requested_user=user, + ) self.assertEqual(success_result.status_code, 302) self.assertEqual( @@ -651,21 +815,76 @@ class TestDomainManagers(TestDomainOverview): 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.assertEqual(call_args["domains"], self.domain) self.assertIsNone(call_args.get("is_member_of_different_org")) - # Assert that the PortfolioInvitation is created + # Assert that the PortfolioInvitation is created and retrieved 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.assertEqual(portfolio_invitation.status, PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) + + # Assert that the UserPortfolioPermission is created + user_portfolio_permission = UserPortfolioPermission.objects.filter( + user=self.user, portfolio=self.portfolio + ).first() + self.assertIsNotNone(user_portfolio_permission, "User portfolio permission should be created") 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_to_new_email( + self, mock_send_domain_email, mock_send_portfolio_email + ): + """Adding an email not associated with a user works and sends portfolio invitation.""" + 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"] = "notauser@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="notauser@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"], "notauser@igorville.gov") + self.assertEqual(call_args["requestor"], self.user) + self.assertEqual(call_args["domains"], self.domain) + self.assertIsNone(call_args.get("is_member_of_different_org")) + + # Assert that the PortfolioInvitation is created + portfolio_invitation = PortfolioInvitation.objects.filter( + email="notauser@igorville.gov", portfolio=self.portfolio + ).first() + self.assertIsNotNone(portfolio_invitation, "Portfolio invitation should be created.") + self.assertEqual(portfolio_invitation.email, "notauser@igorville.gov") + self.assertEqual(portfolio_invitation.portfolio, self.portfolio) + self.assertEqual(portfolio_invitation.status, PortfolioInvitation.PortfolioInvitationStatus.INVITED) + + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + success_page = success_result.follow() + self.assertContains(success_page, "notauser@igorville.gov") + @boto3_mocking.patching @override_flag("organization_feature", active=True) @less_console_noise_decorator @@ -701,7 +920,7 @@ class TestDomainManagers(TestDomainOverview): 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.assertEqual(call_args["domains"], self.domain) self.assertIsNone(call_args.get("is_member_of_different_org")) # Assert that no PortfolioInvitation is created @@ -759,15 +978,15 @@ class TestDomainManagers(TestDomainOverview): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) success_page = success_result.follow() - self.assertContains(success_page, "Could not send email invitation.") + self.assertContains(success_page, "Failed to send email.") - @boto3_mocking.patching @less_console_noise_decorator - def test_domain_invitation_created(self): + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_invitation_created(self, mock_send_domain_email): """Add user on a nonexistent email creates an invitation. Adding a non-existent user sends an email as a side-effect, so mock - out the boto3 SES email sending here. + out send_domain_invitation_email here. """ # make sure there is no user with this email email_address = "mayor@igorville.gov" @@ -780,10 +999,11 @@ class TestDomainManagers(TestDomainOverview): add_page.form["email"] = email_address self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - mock_client = MockSESClient() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with less_console_noise(): - success_result = add_page.form.submit() + success_result = add_page.form.submit() + + mock_send_domain_email.assert_called_once_with( + email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None + ) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) success_page = success_result.follow() @@ -792,13 +1012,13 @@ class TestDomainManagers(TestDomainOverview): self.assertContains(success_page, "Cancel") # link to cancel invitation self.assertTrue(DomainInvitation.objects.filter(email=email_address).exists()) - @boto3_mocking.patching @less_console_noise_decorator - def test_domain_invitation_created_for_caps_email(self): + @patch("registrar.views.domain.send_domain_invitation_email") + def test_domain_invitation_created_for_caps_email(self, mock_send_domain_email): """Add user on a nonexistent email with CAPS creates an invitation to lowercase email. Adding a non-existent user sends an email as a side-effect, so mock - out the boto3 SES email sending here. + out send_domain_invitation_email here. """ # make sure there is no user with this email email_address = "mayor@igorville.gov" @@ -812,9 +1032,11 @@ class TestDomainManagers(TestDomainOverview): add_page.form["email"] = caps_email_address self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - mock_client = MockSESClient() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - success_result = add_page.form.submit() + success_result = add_page.form.submit() + + mock_send_domain_email.assert_called_once_with( + email="mayor@igorville.gov", requestor=self.user, domains=self.domain, is_member_of_different_org=None + ) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) success_page = success_result.follow() @@ -2660,7 +2882,6 @@ class TestDomainRenewal(TestWithUser): UserDomainRole.objects.filter(user=self.user, domain=self.domain_with_expiring_soon_date).delete() self.client.force_login(self.user) domains_page = self.client.get("/") - self.assertNotContains(domains_page, "Expiring soon") self.assertNotContains(domains_page, "will expire soon") @less_console_noise_decorator @@ -2698,5 +2919,4 @@ class TestDomainRenewal(TestWithUser): UserDomainRole.objects.filter(user=self.user, domain=self.domain_with_expiring_soon_date).delete() self.client.force_login(self.user) domains_page = self.client.get("/") - self.assertNotContains(domains_page, "Expiring soon") self.assertNotContains(domains_page, "will expire soon") diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 9bc97874d..69502d683 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -2106,25 +2106,75 @@ class TestPortfolioInvitedMemberDomainsView(TestWithUser, WebTest): self.assertEqual(response.status_code, 404) -class TestPortfolioMemberDomainsEditView(TestPortfolioMemberDomainsView): +class TestPortfolioMemberDomainsEditView(TestWithUser, WebTest): @classmethod def setUpClass(cls): super().setUpClass() - cls.url = reverse("member-domains-edit", kwargs={"pk": cls.portfolio_permission.pk}) + # Create Portfolio + cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Test Portfolio") + # Create domains for testing + cls.domain1 = Domain.objects.create(name="1.gov") + cls.domain2 = Domain.objects.create(name="2.gov") + cls.domain3 = Domain.objects.create(name="3.gov") @classmethod def tearDownClass(cls): super().tearDownClass() + Portfolio.objects.all().delete() + User.objects.all().delete() + Domain.objects.all().delete() def setUp(self): super().setUp() - names = ["1.gov", "2.gov", "3.gov"] - Domain.objects.bulk_create([Domain(name=name) for name in names]) + # Create test member + self.user_member = User.objects.create( + username="test_member", + first_name="Second", + last_name="User", + email="second@example.com", + phone="8003112345", + title="Member", + ) + # Create test user with no perms + self.user_no_perms = User.objects.create( + username="test_user_no_perms", + first_name="No", + last_name="Permissions", + email="user_no_perms@example.com", + phone="8003112345", + title="No Permissions", + ) + # Assign permissions to the user making requests + self.portfolio_permission = UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + # Assign permissions to test member + self.permission = UserPortfolioPermission.objects.create( + user=self.user_member, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + # Create url to be used in all tests + self.url = reverse("member-domains-edit", kwargs={"pk": self.portfolio_permission.pk}) def tearDown(self): super().tearDown() UserDomainRole.objects.all().delete() - Domain.objects.all().delete() + DomainInvitation.objects.all().delete() + UserPortfolioPermission.objects.all().delete() + PortfolioInvitation.objects.all().delete() + Portfolio.objects.exclude(id=self.portfolio.id).delete() + User.objects.exclude(id=self.user.id).delete() @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -2180,12 +2230,13 @@ class TestPortfolioMemberDomainsEditView(TestPortfolioMemberDomainsView): @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) - def test_post_with_valid_added_domains(self): + @patch("registrar.views.portfolios.send_domain_invitation_email") + def test_post_with_valid_added_domains(self, mock_send_domain_email): """Test that domains can be successfully added.""" self.client.force_login(self.user) data = { - "added_domains": json.dumps([1, 2, 3]), # Mock domain IDs + "added_domains": json.dumps([self.domain1.id, self.domain2.id, self.domain3.id]), # Mock domain IDs } response = self.client.post(self.url, data) @@ -2198,31 +2249,43 @@ class TestPortfolioMemberDomainsEditView(TestPortfolioMemberDomainsView): self.assertEqual(len(messages), 1) self.assertEqual(str(messages[0]), "The domain assignment changes have been saved.") + expected_domains = [self.domain1, self.domain2, self.domain3] + # Verify that the invitation email was sent + mock_send_domain_email.assert_called_once() + call_args = mock_send_domain_email.call_args.kwargs + self.assertEqual(call_args["email"], "info@example.com") + self.assertEqual(call_args["requestor"], self.user) + self.assertEqual(list(call_args["domains"]), list(expected_domains)) + self.assertIsNone(call_args.get("is_member_of_different_org")) + @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) - def test_post_with_valid_removed_domains(self): + @patch("registrar.views.portfolios.send_domain_invitation_email") + def test_post_with_valid_removed_domains(self, mock_send_domain_email): """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]) + domains = [self.domain1, self.domain2, self.domain3] + UserDomainRole.objects.bulk_create([UserDomainRole(domain=domain, user=self.user) for domain in domains]) data = { - "removed_domains": json.dumps([1, 2]), + "removed_domains": json.dumps([self.domain1.id, self.domain2.id]), } 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) + self.assertEqual(UserDomainRole.objects.filter(domain=self.domain3, 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.") + # assert that send_domain_invitation_email is not called + mock_send_domain_email.assert_not_called() UserDomainRole.objects.all().delete() @@ -2290,26 +2353,93 @@ class TestPortfolioMemberDomainsEditView(TestPortfolioMemberDomainsView): self.assertEqual(len(messages), 1) self.assertEqual(str(messages[0]), "No changes detected.") + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + @patch("registrar.views.portfolios.send_domain_invitation_email") + def test_post_when_send_domain_email_raises_exception(self, mock_send_domain_email): + """Test attempt to add new domains when an EmailSendingError raised.""" + self.client.force_login(self.user) -class TestPortfolioInvitedMemberEditDomainsView(TestPortfolioInvitedMemberDomainsView): + data = { + "added_domains": json.dumps([self.domain1.id, self.domain2.id, self.domain3.id]), # Mock domain IDs + } + mock_send_domain_email.side_effect = EmailSendingError("Failed to send email") + response = self.client.post(self.url, data) + + # Check that the UserDomainRole objects were not created + self.assertEqual(UserDomainRole.objects.filter(user=self.user, role=UserDomainRole.Roles.MANAGER).count(), 0) + + # Check for an error message and a redirect to edit form + self.assertRedirects(response, reverse("member-domains-edit", kwargs={"pk": self.portfolio_permission.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual( + str(messages[0]), + "An unexpected error occurred: Failed to send email. If the issue persists, please contact help@get.gov.", + ) + + +class TestPortfolioInvitedMemberEditDomainsView(TestWithUser, WebTest): @classmethod def setUpClass(cls): super().setUpClass() - cls.url = reverse("invitedmember-domains-edit", kwargs={"pk": cls.invitation.pk}) + # Create Portfolio + cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Test Portfolio") + # Create domains for testing + cls.domain1 = Domain.objects.create(name="1.gov") + cls.domain2 = Domain.objects.create(name="2.gov") + cls.domain3 = Domain.objects.create(name="3.gov") @classmethod def tearDownClass(cls): super().tearDownClass() + Portfolio.objects.all().delete() + User.objects.all().delete() + Domain.objects.all().delete() def setUp(self): super().setUp() - names = ["1.gov", "2.gov", "3.gov"] - Domain.objects.bulk_create([Domain(name=name) for name in names]) + # Add a user with no permissions + self.user_no_perms = User.objects.create( + username="test_user_no_perms", + first_name="No", + last_name="Permissions", + email="user_no_perms@example.com", + phone="8003112345", + title="No Permissions", + ) + # Add an invited member who has been invited to manage domains + self.invited_member_email = "invited@example.com" + self.invitation = PortfolioInvitation.objects.create( + email=self.invited_member_email, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + ], + ) + + # Assign permissions to the user making requests + UserPortfolioPermission.objects.create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + self.url = reverse("invitedmember-domains-edit", kwargs={"pk": self.invitation.pk}) def tearDown(self): super().tearDown() Domain.objects.all().delete() DomainInvitation.objects.all().delete() + UserPortfolioPermission.objects.all().delete() + PortfolioInvitation.objects.all().delete() + Portfolio.objects.exclude(id=self.portfolio.id).delete() + User.objects.exclude(id=self.user.id).delete() @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -2364,12 +2494,13 @@ class TestPortfolioInvitedMemberEditDomainsView(TestPortfolioInvitedMemberDomain @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) - def test_post_with_valid_added_domains(self): + @patch("registrar.views.portfolios.send_domain_invitation_email") + def test_post_with_valid_added_domains(self, mock_send_domain_email): """Test adding new domains successfully.""" self.client.force_login(self.user) data = { - "added_domains": json.dumps([1, 2, 3]), # Mock domain IDs + "added_domains": json.dumps([self.domain1.id, self.domain2.id, self.domain3.id]), } response = self.client.post(self.url, data) @@ -2387,10 +2518,20 @@ class TestPortfolioInvitedMemberEditDomainsView(TestPortfolioInvitedMemberDomain self.assertEqual(len(messages), 1) self.assertEqual(str(messages[0]), "The domain assignment changes have been saved.") + expected_domains = [self.domain1, self.domain2, self.domain3] + # Verify that the invitation email was sent + mock_send_domain_email.assert_called_once() + call_args = mock_send_domain_email.call_args.kwargs + self.assertEqual(call_args["email"], "invited@example.com") + self.assertEqual(call_args["requestor"], self.user) + self.assertEqual(list(call_args["domains"]), list(expected_domains)) + self.assertFalse(call_args.get("is_member_of_different_org")) + @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): + @patch("registrar.views.portfolios.send_domain_invitation_email") + def test_post_with_existing_and_new_added_domains(self, _): """Test updating existing and adding new invitations.""" self.client.force_login(self.user) @@ -2398,29 +2539,33 @@ class TestPortfolioInvitedMemberEditDomainsView(TestPortfolioInvitedMemberDomain DomainInvitation.objects.bulk_create( [ DomainInvitation( - domain_id=1, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.CANCELED + domain=self.domain1, + email="invited@example.com", + status=DomainInvitation.DomainInvitationStatus.CANCELED, ), DomainInvitation( - domain_id=2, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + domain=self.domain2, + email="invited@example.com", + status=DomainInvitation.DomainInvitationStatus.INVITED, ), ] ) data = { - "added_domains": json.dumps([1, 2, 3]), + "added_domains": json.dumps([self.domain1.id, self.domain2.id, self.domain3.id]), } 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.objects.get(domain=self.domain1, 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 + domain=self.domain3, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED ).exists() ) @@ -2430,7 +2575,8 @@ class TestPortfolioInvitedMemberEditDomainsView(TestPortfolioInvitedMemberDomain @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) - def test_post_with_valid_removed_domains(self): + @patch("registrar.views.portfolios.send_domain_invitation_email") + def test_post_with_valid_removed_domains(self, mock_send_domain_email): """Test removing domains successfully.""" self.client.force_login(self.user) @@ -2438,33 +2584,39 @@ class TestPortfolioInvitedMemberEditDomainsView(TestPortfolioInvitedMemberDomain DomainInvitation.objects.bulk_create( [ DomainInvitation( - domain_id=1, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + domain=self.domain1, + email="invited@example.com", + status=DomainInvitation.DomainInvitationStatus.INVITED, ), DomainInvitation( - domain_id=2, email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + domain=self.domain2, + email="invited@example.com", + status=DomainInvitation.DomainInvitationStatus.INVITED, ), ] ) data = { - "removed_domains": json.dumps([1]), + "removed_domains": json.dumps([self.domain1.id]), } 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.objects.get(domain=self.domain1, 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.objects.get(domain=self.domain2, 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})) + # assert that send_domain_invitation_email is not called + mock_send_domain_email.assert_not_called() @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -2530,6 +2682,37 @@ class TestPortfolioInvitedMemberEditDomainsView(TestPortfolioInvitedMemberDomain self.assertEqual(len(messages), 1) self.assertEqual(str(messages[0]), "No changes detected.") + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + @patch("registrar.views.portfolios.send_domain_invitation_email") + def test_post_when_send_domain_email_raises_exception(self, mock_send_domain_email): + """Test attempt to add new domains when an EmailSendingError raised.""" + self.client.force_login(self.user) + + data = { + "added_domains": json.dumps([self.domain1.id, self.domain2.id, self.domain3.id]), + } + mock_send_domain_email.side_effect = EmailSendingError("Failed to send email") + response = self.client.post(self.url, data) + + # Check that the DomainInvitation objects were not created + self.assertEqual( + DomainInvitation.objects.filter( + email="invited@example.com", status=DomainInvitation.DomainInvitationStatus.INVITED + ).count(), + 0, + ) + + # Check for an error message and a redirect to edit form + self.assertRedirects(response, reverse("invitedmember-domains-edit", kwargs={"pk": self.invitation.pk})) + messages = list(response.wsgi_request._messages) + self.assertEqual(len(messages), 1) + self.assertEqual( + str(messages[0]), + "An unexpected error occurred: Failed to send email. If the issue persists, please contact help@get.gov.", + ) + class TestRequestingEntity(WebTest): """The requesting entity page is a domain request form that only exists @@ -2696,7 +2879,7 @@ class TestRequestingEntity(WebTest): form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = True form["portfolio_requesting_entity-is_requesting_new_suborganization"] = True - form["portfolio_requesting_entity-sub_organization"] = "" + form["portfolio_requesting_entity-sub_organization"] = "other" form["portfolio_requesting_entity-requested_suborganization"] = "moon" form["portfolio_requesting_entity-suborganization_city"] = "kepler" @@ -2759,18 +2942,34 @@ class TestRequestingEntity(WebTest): session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + # For 2 the tests below, it is required to submit a form without submitting a value + # for the select/combobox. WebTest will not do this; by default, WebTest will submit + # the first choice in a select. So, need to manipulate the form to remove the + # particular select/combobox that will not be submitted, and then post the form. + form_action = f"/request/{domain_request.pk}/portfolio_requesting_entity/" + # Test missing suborganization selection form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = True - form["portfolio_requesting_entity-sub_organization"] = "" - - response = form.submit() + form["portfolio_requesting_entity-is_requesting_new_suborganization"] = False + # remove sub_organization from the form submission + form_data = form.submit_fields() + form_data = [(key, value) for key, value in form_data if key != "portfolio_requesting_entity-sub_organization"] + response = self.app.post(form_action, dict(form_data)) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) self.assertContains(response, "Suborganization is required.", status_code=200) # Test missing custom suborganization details + form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = True form["portfolio_requesting_entity-is_requesting_new_suborganization"] = True - response = form.submit() - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + form["portfolio_requesting_entity-sub_organization"] = "other" + # remove suborganization_state_territory from the form submission + form_data = form.submit_fields() + form_data = [ + (key, value) + for key, value in form_data + if key != "portfolio_requesting_entity-suborganization_state_territory" + ] + response = self.app.post(form_action, dict(form_data)) self.assertContains(response, "Enter the name of your suborganization.", status_code=200) self.assertContains(response, "Enter the city where your suborganization is located.", status_code=200) self.assertContains( @@ -2879,7 +3078,7 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): ], ) - cls.new_member_email = "davekenn4242@gmail.com" + cls.new_member_email = "newmember@example.com" AllowedEmail.objects.get_or_create(email=cls.new_member_email) @@ -2933,11 +3132,13 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): self.assertEqual(final_response.status_code, 302) # Redirects # Validate Database Changes + # Validate that portfolio invitation was created but not retrieved 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) + self.assertEqual(portfolio_invite.status, PortfolioInvitation.PortfolioInvitationStatus.INVITED) # Check that an email was sent self.assertTrue(mock_client.send_email.called) @@ -3228,6 +3429,52 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): # 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_member_invite_for_existing_user_who_is_not_a_member(self, mock_send_email): + """Tests the member invitation flow for existing user who is not a 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) + + new_user = User.objects.create(email="newuser@example.com") + + # Simulate submission of member invite for the newly created user + response = self.client.post( + reverse("new-member"), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "email": "newuser@example.com", + }, + ) + self.assertEqual(response.status_code, 302) + + # Validate Database Changes + # Validate that portfolio invitation was created and retrieved + portfolio_invite = PortfolioInvitation.objects.filter( + email="newuser@example.com", portfolio=self.portfolio + ).first() + self.assertIsNotNone(portfolio_invite) + self.assertEqual(portfolio_invite.email, "newuser@example.com") + self.assertEqual(portfolio_invite.status, PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) + # Validate UserPortfolioPermission + user_portfolio_permission = UserPortfolioPermission.objects.filter( + user=new_user, portfolio=self.portfolio + ).first() + self.assertIsNotNone(user_portfolio_permission) + + # assert that send_portfolio_invitation_email is called + mock_send_email.assert_called_once() + call_args = mock_send_email.call_args.kwargs + self.assertEqual(call_args["email"], "newuser@example.com") + self.assertEqual(call_args["requestor"], self.user) + self.assertIsNone(call_args.get("is_member_of_different_org")) + class TestEditPortfolioMemberView(WebTest): """Tests for the edit member page on portfolios""" diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 6e5773ebb..1bb53a7a3 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1660,6 +1660,27 @@ class DomainRequestExport(BaseExport): default=F("organization_name"), output_field=CharField(), ), + "converted_city": Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__city")), + # Otherwise, return the natively assigned value + default=F("city"), + output_field=CharField(), + ), + "converted_state_territory": Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__state_territory")), + # Otherwise, return the natively assigned value + default=F("state_territory"), + output_field=CharField(), + ), + "converted_suborganization_name": Case( + # When sub_organization is present, use its name + When(sub_organization__isnull=False, then=F("sub_organization__name")), + # Otherwise, return empty string + default=Value(""), + output_field=CharField(), + ), "converted_so_email": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__senior_official__email")), @@ -1786,6 +1807,10 @@ class DomainRequestExport(BaseExport): status = model.get("status") status_display = DomainRequest.DomainRequestStatus.get_status_label(status) if status else None + # Handle the portfolio field. Display as a Yes/No + portfolio = model.get("portfolio") + portfolio_display = "Yes" if portfolio is not None else "No" + # Handle the region field. state_territory = model.get("state_territory") region = get_region(state_territory) if state_territory else None @@ -1819,6 +1844,7 @@ class DomainRequestExport(BaseExport): "Election office": human_readable_election_board, "Federal type": human_readable_federal_type, "Domain type": human_readable_org_type, + "Portfolio": portfolio_display, "Request additional details": additional_details, # Annotated fields - passed into the request dict. "Creator approved domains count": model.get("creator_approved_domains_count", 0), @@ -1827,6 +1853,10 @@ class DomainRequestExport(BaseExport): "Other contacts": model.get("all_other_contacts"), "Current websites": model.get("all_current_websites"), # Untouched FK fields - passed into the request dict. + "Suborganization": model.get("converted_suborganization_name"), + "Requested suborg": model.get("requested_suborganization"), + "Suborg city": model.get("suborganization_city"), + "Suborg state/territory": model.get("suborganization_state_territory"), "Federal agency": model.get("converted_federal_agency"), "SO first name": model.get("converted_senior_official_first_name"), "SO last name": model.get("converted_senior_official_last_name"), @@ -1838,8 +1868,8 @@ class DomainRequestExport(BaseExport): "Investigator": model.get("investigator__email"), # Untouched fields "Organization name": model.get("converted_organization_name"), - "City": model.get("city"), - "State/territory": model.get("state_territory"), + "City": model.get("converted_city"), + "State/territory": model.get("converted_state_territory"), "Request purpose": model.get("purpose"), "CISA regional representative": model.get("cisa_representative_email"), "Last submitted date": model.get("last_submitted_date"), @@ -2006,6 +2036,7 @@ class DomainRequestDataFull(DomainRequestExport): "Last status update", "Status", "Domain type", + "Portfolio", "Federal type", "Federal agency", "Organization name", @@ -2013,6 +2044,10 @@ class DomainRequestDataFull(DomainRequestExport): "City", "State/territory", "Region", + "Suborganization", + "Requested suborg", + "Suborg city", + "Suborg state/territory", "Creator first name", "Creator last name", "Creator email", diff --git a/src/registrar/utility/db_helpers.py b/src/registrar/utility/db_helpers.py new file mode 100644 index 000000000..5b7e0392c --- /dev/null +++ b/src/registrar/utility/db_helpers.py @@ -0,0 +1,20 @@ +from contextlib import contextmanager +from django.db import transaction, IntegrityError +from psycopg2 import errorcodes + + +@contextmanager +def ignore_unique_violation(): + """ + Execute within an atomic transaction so that if a unique constraint violation occurs, + the individual transaction is rolled back without invalidating any larger transaction. + """ + with transaction.atomic(): + try: + yield + except IntegrityError as e: + if e.__cause__.pgcode == errorcodes.UNIQUE_VIOLATION: + # roll back to the savepoint, effectively ignoring this transaction + pass + else: + raise e diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index 7171b8902..25e9db0f3 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -1,5 +1,6 @@ +from datetime import date from django.conf import settings -from registrar.models import DomainInvitation +from registrar.models import Domain, DomainInvitation, UserDomainRole from registrar.utility.errors import ( AlreadyDomainInvitedError, AlreadyDomainManagerError, @@ -7,23 +8,24 @@ from registrar.utility.errors import ( OutsideOrgMemberError, ) from registrar.utility.waffle import flag_is_active_for_user -from registrar.utility.email import send_templated_email +from registrar.utility.email import EmailSendingError, send_templated_email import logging logger = logging.getLogger(__name__) -def send_domain_invitation_email(email: str, requestor, domain, is_member_of_different_org): +def send_domain_invitation_email( + email: str, requestor, domains: Domain | list[Domain], is_member_of_different_org, requested_user=None +): """ 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. + domains (Domain or list of Domain): The domain objects for which the invitation is being sent. is_member_of_different_org (bool): if an email belongs to a different org + requested_user (User | None): The recipient if the email belongs to a user in the registrar Raises: MissingEmailError: If the requestor has no email associated with their account. @@ -32,26 +34,95 @@ def send_domain_invitation_email(email: str, requestor, domain, is_member_of_dif 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 + domains = normalize_domains(domains) + requestor_email = get_requestor_email(requestor, domains) - # 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 + validate_invitation(email, domains, requestor, is_member_of_different_org) - # Check if the recipient is part of a different organization - # COMMENT: this does not account for multiple_portfolios flag being active + send_invitation_email(email, requestor_email, domains, requested_user) + + # send emails to domain managers + for domain in domains: + send_emails_to_domain_managers( + email=email, + requestor_email=requestor_email, + domain=domain, + requested_user=requested_user, + ) + + +def send_emails_to_domain_managers(email: str, requestor_email, domain: Domain, requested_user=None): + """ + Notifies all domain managers of the provided domain of a change + Raises: + EmailSendingError + """ + # Get each domain manager from list + user_domain_roles = UserDomainRole.objects.filter(domain=domain) + for user_domain_role in user_domain_roles: + # Send email to each domain manager + user = user_domain_role.user + try: + send_templated_email( + "emails/domain_manager_notification.txt", + "emails/domain_manager_notification_subject.txt", + to_address=user.email, + context={ + "domain": domain, + "requestor_email": requestor_email, + "invited_email_address": email, + "domain_manager": user, + "date": date.today(), + }, + ) + except EmailSendingError as err: + raise EmailSendingError( + f"Could not send email manager notification to {user.email} for domain: {domain.name}" + ) from err + + +def normalize_domains(domains: Domain | list[Domain]) -> list[Domain]: + """Ensures domains is always a list.""" + return [domains] if isinstance(domains, Domain) else domains + + +def get_requestor_email(requestor, domains): + """Get the requestor's email or raise an error if it's missing. + + If the requestor is staff, default email is returned. + """ + if requestor.is_staff: + return settings.DEFAULT_FROM_EMAIL + + if not requestor.email or requestor.email.strip() == "": + domain_names = ", ".join([domain.name for domain in domains]) + raise MissingEmailError(email=requestor.email, domain=domain_names) + + return requestor.email + + +def validate_invitation(email, domains, requestor, is_member_of_different_org): + """Validate the invitation conditions.""" + check_outside_org_membership(email, requestor, is_member_of_different_org) + + for domain in domains: + validate_existing_invitation(email, domain) + + # NOTE: should we also be validating against existing user_domain_roles + + +def check_outside_org_membership(email, requestor, is_member_of_different_org): + """Raise an error if the email belongs to a different organization.""" 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 + raise OutsideOrgMemberError(email=email) - # Check for an existing invitation + +def validate_existing_invitation(email, domain): + """Check for existing invitations and handle their status.""" try: invite = DomainInvitation.objects.get(email=email, domain=domain) if invite.status == DomainInvitation.DomainInvitationStatus.RETRIEVED: @@ -64,16 +135,24 @@ def send_domain_invitation_email(email: str, requestor, domain, is_member_of_dif 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_invitation_email(email, requestor_email, domains, requested_user): + """Send the invitation email.""" + try: + send_templated_email( + "emails/domain_invitation.txt", + "emails/domain_invitation_subject.txt", + to_address=email, + context={ + "domains": domains, + "requestor_email": requestor_email, + "invitee_email_address": email, + "requested_user": requested_user, + }, + ) + except EmailSendingError as err: + domain_names = ", ".join([domain.name for domain in domains]) + raise EmailSendingError(f"Could not send email invitation to {email} for domains: {domain_names}") from err def send_portfolio_invitation_email(email: str, requestor, portfolio): @@ -98,17 +177,22 @@ def send_portfolio_invitation_email(email: str, requestor, portfolio): # 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 + raise MissingEmailError(email=email, portfolio=portfolio) 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, - }, - ) + try: + send_templated_email( + "emails/portfolio_invitation.txt", + "emails/portfolio_invitation_subject.txt", + to_address=email, + context={ + "portfolio": portfolio, + "requestor_email": requestor_email, + "email": email, + }, + ) + except EmailSendingError as err: + raise EmailSendingError( + f"Could not sent email invitation to {email} for portfolio {portfolio}. Portfolio invitation not saved." + ) from err diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 039fb3696..cc18d7269 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -46,8 +46,17 @@ class AlreadyDomainInvitedError(InvitationError): 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.") + def __init__(self, email=None, domain=None, portfolio=None): + # Default message if no additional info is provided + message = "Can't send invitation email. No email is associated with your user account." + + # Customize message based on provided arguments + if email and domain: + message = f"Can't send email to '{email}' on domain '{domain}'. No email exists for the requestor." + elif email and portfolio: + message = f"Can't send email to '{email}' for portfolio '{portfolio}'. No email exists for the requestor." + + super().__init__(message) class OutsideOrgMemberError(ValueError): diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index a80b16b1a..4e3faced1 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -14,6 +14,7 @@ from .domain import ( DomainInvitationCancelView, DomainDeleteUserView, PrototypeDomainDNSRecordView, + DomainRenewalView, ) from .user_profile import UserProfileView, FinishProfileSetupView from .health import * diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index f544a20f7..f82d7005d 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -10,13 +10,12 @@ import logging import requests from django.contrib import messages from django.contrib.messages.views import SuccessMessageMixin -from django.db import IntegrityError from django.http import HttpResponseRedirect -from django.shortcuts import redirect +from django.shortcuts import redirect, render, get_object_or_404 from django.urls import reverse from django.views.generic.edit import FormMixin from django.conf import settings -from registrar.forms.domain import DomainSuborganizationForm +from registrar.forms.domain import DomainSuborganizationForm, DomainRenewalForm from registrar.models import ( Domain, DomainRequest, @@ -31,22 +30,23 @@ 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, DsDataErrorCodes, SecurityEmailError, SecurityEmailErrorCodes, - OutsideOrgMemberError, ) from registrar.models.utility.contact_error import ContactError from registrar.views.utility.permission_views import UserDomainRolePermissionDeleteView from registrar.utility.waffle import flag_is_active_for_user +from registrar.views.utility.invitation_helper import ( + get_org_membership, + get_requested_user, + handle_invitation_exceptions, +) from ..forms import ( SeniorOfficialContactForm, @@ -311,6 +311,47 @@ class DomainView(DomainBaseView): self._update_session_with_domain() +class DomainRenewalView(DomainView): + """Domain detail overview page.""" + + template_name = "domain_renewal.html" + + def post(self, request, pk): + + domain = get_object_or_404(Domain, id=pk) + + form = DomainRenewalForm(request.POST) + + if form.is_valid(): + + # check for key in the post request data + if "submit_button" in request.POST: + try: + domain.renew_domain() + messages.success(request, "This domain has been renewed for one year.") + except Exception: + messages.error( + request, + "This domain has not been renewed for one year, " + "please email help@get.gov if this problem persists.", + ) + return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) + + # if not valid, render the template with error messages + # passing editable, has_domain_renewal_flag, and is_editable for re-render + return render( + request, + "domain_renewal.html", + { + "domain": domain, + "form": form, + "is_editable": True, + "has_domain_renewal_flag": True, + "is_domain_manager": True, + }, + ) + + class DomainOrgNameAddressView(DomainFormBaseView): """Organization view""" @@ -1149,43 +1190,13 @@ class DomainAddUserView(DomainFormBaseView): def get_success_url(self): return reverse("domain-users", kwargs={"pk": self.object.pk}) - 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. - - Returns a tuple (member_of_a_different_org, member_of_this_org). - """ - - # COMMENT: this code does not take into account multiple portfolios flag - - # 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) + requested_user = 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 @@ -1196,55 +1207,47 @@ class DomainAddUserView(DomainFormBaseView): 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()) - + member_of_a_different_org, member_of_this_org = get_org_membership(domain_org, requested_email, requested_user) try: + # COMMENT: this code does not take into account multiple portfolios flag being set to TRUE + + # 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 + # 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 + ): + send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=domain_org) + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create( + email=requested_email, portfolio=domain_org, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + ) + # if user exists for email, immediately retrieve portfolio invitation upon creation + if requested_user is not None: + portfolio_invitation.retrieve() + portfolio_invitation.save() + messages.success(self.request, f"{requested_email} has been invited to the organization: {domain_org}") + 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) + handle_invitation_exceptions(self.request, 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 - 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, + domains=self.object, is_member_of_different_org=member_of_different_org, ) DomainInvitation.objects.get_or_create(email=email, domain=self.object) @@ -1255,8 +1258,9 @@ class DomainAddUserView(DomainFormBaseView): send_domain_invitation_email( email=email, requestor=requestor, - domain=self.object, + domains=self.object, is_member_of_different_org=member_of_different_org, + requested_user=requested_user, ) UserDomainRole.objects.create( user=requested_user, @@ -1265,57 +1269,6 @@ class DomainAddUserView(DomainFormBaseView): ) messages.success(self.request, f"Added user {email}.") - 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, - ) - 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: - logger.warning("Could not send email invitation (Other Exception)", exc_info=True) - messages.warning(self.request, "Could not send email invitation.") - - 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: - 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): object: DomainInvitation diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 751e28d85..c4f60ca35 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -8,13 +8,14 @@ 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 import Domain 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.email_invitations import send_domain_invitation_email, send_portfolio_invitation_email from registrar.utility.errors import MissingEmailError from registrar.utility.enums import DefaultUserValues from registrar.views.utility.mixins import PortfolioMemberPermission @@ -33,6 +34,8 @@ from django.views.generic import View from django.views.generic.edit import FormMixin from django.db import IntegrityError +from registrar.views.utility.invitation_helper import get_org_membership + logger = logging.getLogger(__name__) @@ -237,6 +240,7 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V removed_domains = request.POST.get("removed_domains") portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) member = portfolio_permission.user + portfolio = portfolio_permission.portfolio added_domain_ids = self._parse_domain_ids(added_domains, "added domains") if added_domain_ids is None: @@ -248,7 +252,7 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V if added_domain_ids or removed_domain_ids: try: - self._process_added_domains(added_domain_ids, member) + self._process_added_domains(added_domain_ids, member, request.user, portfolio) 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})) @@ -258,15 +262,15 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V "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.") + logger.error("A database error occurred while saving changes.", exc_info=True) 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"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)}") + logger.error(f"An unexpected error occurred: {str(e)}", exc_info=True) return redirect(reverse("member-domains-edit", kwargs={"pk": pk})) else: messages.info(request, "No changes detected.") @@ -287,16 +291,26 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V logger.error(f"Invalid data for {domain_type}") return None - def _process_added_domains(self, added_domain_ids, member): + def _process_added_domains(self, added_domain_ids, member, requestor, portfolio): """ Processes added domains by bulk creating UserDomainRole instances. """ if added_domain_ids: + # get added_domains from ids to pass to send email method and bulk create + added_domains = Domain.objects.filter(id__in=added_domain_ids) + member_of_a_different_org, _ = get_org_membership(portfolio, member.email, member) + send_domain_invitation_email( + email=member.email, + requestor=requestor, + domains=added_domains, + is_member_of_different_org=member_of_a_different_org, + requested_user=member, + ) # 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 + UserDomainRole(domain=domain, user=member, role=UserDomainRole.Roles.MANAGER) + for domain in added_domains ], ignore_conflicts=True, # Avoid duplicate entries ) @@ -443,6 +457,7 @@ class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermission removed_domains = request.POST.get("removed_domains") portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) email = portfolio_invitation.email + portfolio = portfolio_invitation.portfolio added_domain_ids = self._parse_domain_ids(added_domains, "added domains") if added_domain_ids is None: @@ -454,7 +469,7 @@ class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermission if added_domain_ids or removed_domain_ids: try: - self._process_added_domains(added_domain_ids, email) + self._process_added_domains(added_domain_ids, email, request.user, portfolio) 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})) @@ -464,15 +479,15 @@ class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermission "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.") + logger.error("A database error occurred while saving changes.", exc_info=True) 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"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)}.") + logger.error(f"An unexpected error occurred: {str(e)}.", exc_info=True) return redirect(reverse("invitedmember-domains-edit", kwargs={"pk": pk})) else: messages.info(request, "No changes detected.") @@ -493,33 +508,41 @@ class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermission logger.error(f"Invalid data for {domain_type}.") return None - def _process_added_domains(self, added_domain_ids, email): + def _process_added_domains(self, added_domain_ids, email, requestor, portfolio): """ Processes added domain invitations by updating existing invitations or creating new ones. """ - if not added_domain_ids: - return + if added_domain_ids: + # get added_domains from ids to pass to send email method and bulk create + added_domains = Domain.objects.filter(id__in=added_domain_ids) + member_of_a_different_org, _ = get_org_membership(portfolio, email, None) + send_domain_invitation_email( + email=email, + requestor=requestor, + domains=added_domains, + is_member_of_different_org=member_of_a_different_org, + ) - # 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) + # Update existing invitations from CANCELED to INVITED + existing_invitations = DomainInvitation.objects.filter(domain__in=added_domains, 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) + # 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 - ] - ) + # 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): """ @@ -754,7 +777,11 @@ class PortfolioAddMemberView(PortfolioMembersPermissionView, FormMixin): try: if not requested_user or not permission_exists: send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=portfolio) - form.save() + portfolio_invitation = form.save() + # if user exists for email, immediately retrieve portfolio invitation upon creation + if requested_user is not None: + portfolio_invitation.retrieve() + portfolio_invitation.save() messages.success(self.request, f"{requested_email} has been invited.") else: if permission_exists: diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py index 69a0f4997..f574b76d9 100644 --- a/src/registrar/views/transfer_user.py +++ b/src/registrar/views/transfer_user.py @@ -1,19 +1,19 @@ import logging +from django.db import transaction +from django.db.models import ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel, ManyToManyRel, OneToOneRel from django.shortcuts import render, get_object_or_404, redirect from django.views import View from registrar.models.domain import Domain -from registrar.models.domain_information import DomainInformation from registrar.models.domain_request import DomainRequest -from registrar.models.portfolio import Portfolio from registrar.models.user import User from django.contrib.admin import site from django.contrib import messages from registrar.models.user_domain_role import UserDomainRole from registrar.models.user_portfolio_permission import UserPortfolioPermission -from registrar.models.verified_by_staff import VerifiedByStaff -from typing import Any, List + +from registrar.utility.db_helpers import ignore_unique_violation logger = logging.getLogger(__name__) @@ -21,22 +21,8 @@ logger = logging.getLogger(__name__) class TransferUserView(View): """Transfer user methods that set up the transfer_user template and handle the forms on it.""" - JOINS = [ - (DomainRequest, "creator"), - (DomainInformation, "creator"), - (Portfolio, "creator"), - (DomainRequest, "investigator"), - (UserDomainRole, "user"), - (VerifiedByStaff, "requestor"), - (UserPortfolioPermission, "user"), - ] - - # Future-proofing in case joined fields get added on the user model side - # This was tested in the first portfolio model iteration and works - USER_FIELDS: List[Any] = [] - def get(self, request, user_id): - """current_user referes to the 'source' user where the button that redirects to this view was clicked. + """current_user refers to the 'source' user where the button that redirects to this view was clicked. other_users exclude current_user and populate a dropdown, selected_user is the selection in the dropdown. This also querries the relevant domains and domain requests, and the admin context needed for the sidenav.""" @@ -70,86 +56,122 @@ class TransferUserView(View): return render(request, "admin/transfer_user.html", context) def post(self, request, user_id): - """This handles the transfer from selected_user to current_user then deletes selected_user. - - NOTE: We have a ticket to refactor this into a more solid lookup for related fields in #2645""" - + """This handles the transfer from selected_user to current_user then deletes selected_user.""" current_user = get_object_or_404(User, pk=user_id) selected_user_id = request.POST.get("selected_user") selected_user = get_object_or_404(User, pk=selected_user_id) try: - change_logs = [] + # Make this atomic so that we don't get any partial transfers + with transaction.atomic(): + change_logs = [] - # Transfer specific fields - self.transfer_user_fields_and_log(selected_user, current_user, change_logs) + # Dynamically handle related fields + self.transfer_related_fields_and_log(selected_user, current_user, change_logs) - # Perform the updates and log the changes - for model_class, field_name in self.JOINS: - self.update_joins_and_log(model_class, field_name, selected_user, current_user, change_logs) - - # Success message if any related objects were updated - if change_logs: - success_message = f"Data transferred successfully for the following objects: {change_logs}" - messages.success(request, success_message) - - selected_user.delete() - messages.success(request, f"Deleted {selected_user} {selected_user.username}") + # Success message if any related objects were updated + if change_logs: + success_message = f"Data transferred successfully for the following objects: {change_logs}" + messages.success(request, success_message) + selected_user.delete() + messages.success(request, f"Deleted {selected_user} {selected_user.username}") except Exception as e: messages.error(request, f"An error occurred during the transfer: {e}") + logger.error(f"An error occurred during the transfer: {e}", exc_info=True) return redirect("admin:registrar_user_change", object_id=user_id) - @classmethod - def update_joins_and_log(cls, model_class, field_name, selected_user, current_user, change_logs): + def transfer_related_fields_and_log(self, selected_user, current_user, change_logs): """ - Helper function to update the user join fields for a given model and log the changes. + Dynamically find all related fields to the User model and transfer them from selected_user to current_user. + Handles ForeignKey, OneToOneField, ManyToManyField, and ManyToOneRel relationships. """ + user_model = User - filter_kwargs = {field_name: selected_user} - updated_objects = model_class.objects.filter(**filter_kwargs) + for related_field in user_model._meta.get_fields(): + if related_field.is_relation: + # Field objects represent forward relationships + if isinstance(related_field, OneToOneField): + self._handle_one_to_one(related_field, selected_user, current_user, change_logs) + elif isinstance(related_field, ManyToManyField): + self._handle_many_to_many(related_field, selected_user, current_user, change_logs) + elif isinstance(related_field, ForeignKey): + self._handle_foreign_key(related_field, selected_user, current_user, change_logs) + # Relationship objects represent reverse relationships + elif isinstance(related_field, ManyToOneRel): + # ManyToOneRel is a reverse ForeignKey + self._handle_foreign_key_reverse(related_field, selected_user, current_user, change_logs) + elif isinstance(related_field, OneToOneRel): + self._handle_one_to_one_reverse(related_field, selected_user, current_user, change_logs) + elif isinstance(related_field, ManyToManyRel): + self._handle_many_to_many_reverse(related_field, selected_user, current_user, change_logs) + else: + logger.error(f"Unknown relationship type for field {related_field}") + raise ValueError(f"Unknown relationship type for field {related_field}") - for obj in updated_objects: - # Check for duplicate UserDomainRole before updating - if model_class == UserDomainRole: - if model_class.objects.filter(user=current_user, domain=obj.domain).exists(): - continue # Skip the update to avoid a duplicate + def _handle_foreign_key_reverse(self, related_field: ManyToOneRel, selected_user, current_user, change_logs): + # Handle reverse ForeignKey relationships + related_manager = getattr(selected_user, related_field.get_accessor_name(), None) + if related_manager and related_manager.exists(): + for related_object in related_manager.all(): + with ignore_unique_violation(): + setattr(related_object, related_field.field.name, current_user) + related_object.save() + self.log_change(related_object, selected_user, current_user, related_field.field.name, change_logs) - if model_class == UserPortfolioPermission: - if model_class.objects.filter(user=current_user, portfolio=obj.portfolio).exists(): - continue # Skip the update to avoid a duplicate + def _handle_foreign_key(self, related_field: ForeignKey, selected_user, current_user, change_logs): + # Handle ForeignKey relationships + related_object = getattr(selected_user, related_field.name, None) + if related_object: + setattr(current_user, related_field.name, related_object) + current_user.save() + self.log_change(related_object, selected_user, current_user, related_field.name, change_logs) - # Update the field on the object and save it - setattr(obj, field_name, current_user) - obj.save() + def _handle_one_to_one(self, related_field: OneToOneField, selected_user, current_user, change_logs): + # Handle OneToOne relationship + related_object = getattr(selected_user, related_field.name, None) + if related_object: + with ignore_unique_violation(): + setattr(current_user, related_field.name, related_object) + current_user.save() + self.log_change(related_object, selected_user, current_user, related_field.name, change_logs) - # Log the change - cls.log_change(obj, field_name, selected_user, current_user, change_logs) + def _handle_many_to_many(self, related_field: ManyToManyField, selected_user, current_user, change_logs): + # Handle ManyToMany relationship + related_name = related_field.remote_field.name + related_manager = getattr(selected_user, related_name, None) + if related_manager and related_manager.exists(): + for instance in related_manager.all(): + with ignore_unique_violation(): + getattr(instance, related_name).remove(selected_user) + getattr(instance, related_name).add(current_user) + self.log_change(instance, selected_user, current_user, related_name, change_logs) + + def _handle_many_to_many_reverse(self, related_field: ManyToManyRel, selected_user, current_user, change_logs): + # Handle reverse relationship + related_name = related_field.field.name + related_manager = getattr(selected_user, related_name, None) + if related_manager and related_manager.exists(): + for instance in related_manager.all(): + with ignore_unique_violation(): + getattr(instance, related_name).remove(selected_user) + getattr(instance, related_name).add(current_user) + self.log_change(instance, selected_user, current_user, related_name, change_logs) + + def _handle_one_to_one_reverse(self, related_field: OneToOneRel, selected_user, current_user, change_logs): + # Handle reverse relationship + field_name = related_field.get_accessor_name() + related_instance = getattr(selected_user, field_name, None) + if related_instance: + setattr(related_instance, field_name, current_user) + related_instance.save() + self.log_change(related_instance, selected_user, current_user, field_name, change_logs) @classmethod - def transfer_user_fields_and_log(cls, selected_user, current_user, change_logs): - """ - Transfers portfolio fields from the selected_user to the current_user. - Logs the changes for each transferred field. - """ - for field in cls.USER_FIELDS: - field_value = getattr(selected_user, field, None) - - if field_value: - setattr(current_user, field, field_value) - cls.log_change(current_user, field, field_value, field_value, change_logs) - - current_user.save() - - @classmethod - def log_change(cls, obj, field_name, field_value, new_value, change_logs): - """Logs the change for a specific field on an object""" - log_entry = f'Changed {field_name} from "{field_value}" to "{new_value}" on {obj}' - + def log_change(cls, obj, selected_user, current_user, field_name, change_logs): + log_entry = f"Changed {field_name} from {selected_user} to {current_user} on {obj}" logger.info(log_entry) - - # Collect the related object for the success message change_logs.append(log_entry) @classmethod diff --git a/src/registrar/views/utility/invitation_helper.py b/src/registrar/views/utility/invitation_helper.py new file mode 100644 index 000000000..5c730d0c3 --- /dev/null +++ b/src/registrar/views/utility/invitation_helper.py @@ -0,0 +1,86 @@ +from django.contrib import messages +from django.db import IntegrityError +from registrar.models import PortfolioInvitation, User, UserPortfolioPermission +from registrar.utility.email import EmailSendingError +import logging + +from registrar.utility.errors import ( + AlreadyDomainInvitedError, + AlreadyDomainManagerError, + MissingEmailError, + OutsideOrgMemberError, +) + +logger = logging.getLogger(__name__) + +# These methods are used by multiple views which share similar logic and function +# when creating invitations and sending associated emails. These can be reused in +# any view, and were initially developed for domain.py, portfolios.py and admin.py + + +def get_org_membership(org, email, user): + """ + Determines if an email/user belongs to a different organization or this organization + as either a member or an invited member. + + This function returns a tuple (member_of_a_different_org, member_of_this_org), + which provides: + - member_of_a_different_org: True if the user/email is associated with an organization other than the given org. + - member_of_this_org: True if the user/email is associated with the given org. + + Note: This implementation assumes single portfolio ownership for a user. + If the "multiple portfolios" feature is enabled, this logic may not account for + situations where a user or email belongs to multiple organizations. + """ + + # Check for existing permissions or invitations for the user + existing_org_permission = UserPortfolioPermission.objects.filter(user=user).first() + existing_org_invitation = PortfolioInvitation.objects.filter(email=email).first() + + # Determine membership in a different organization + member_of_a_different_org = (existing_org_permission and existing_org_permission.portfolio != org) or ( + existing_org_invitation and existing_org_invitation.portfolio != org + ) + + # Determine membership in the same organization + member_of_this_org = (existing_org_permission and existing_org_permission.portfolio == org) or ( + existing_org_invitation and existing_org_invitation.portfolio == org + ) + + return member_of_a_different_org, member_of_this_org + + +def get_requested_user(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 + + +def handle_invitation_exceptions(request, exception, email): + """Handle exceptions raised during the process.""" + if isinstance(exception, EmailSendingError): + logger.warning(str(exception), exc_info=True) + messages.error(request, str(exception)) + elif isinstance(exception, MissingEmailError): + messages.error(request, str(exception)) + logger.error(str(exception), exc_info=True) + elif isinstance(exception, OutsideOrgMemberError): + logger.warning( + "Could not send email. Can not invite member of a .gov organization to a different organization.", + exc_info=True, + ) + messages.error( + request, + f"{email} is already a member of another .gov organization.", + ) + elif isinstance(exception, AlreadyDomainManagerError): + messages.warning(request, str(exception)) + elif isinstance(exception, AlreadyDomainInvitedError): + messages.warning(request, str(exception)) + elif isinstance(exception, IntegrityError): + messages.warning(request, f"{email} is already a manager for this domain") + else: + logger.warning("Could not send email invitation (Other Exception)", exc_info=True) + messages.warning(request, "Could not send email invitation.")