diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8ecf36f52..31c75e05e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1329,6 +1329,14 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): get_roles.short_description = "Roles" # type: ignore + def delete_queryset(self, request, queryset): + """We override the delete method in the model. + When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action + the model delete does not get called. This method gets called instead. + This override makes sure our code in the model gets executed in these situations.""" + for obj in queryset: + obj.delete() # Calls the overridden delete method on each instance + class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom user domain role admin class.""" @@ -1407,10 +1415,13 @@ class BaseInvitationAdmin(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. """ - # store current messages from request so that they are preserved throughout the method + # store current messages from request in storage so that they are preserved throughout the + # method, as some flows remove and replace all messages, and so we store here to retrieve + # later 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) + # Check if there are any error messages in the `messages` framework + # error messages stop the workflow; other message levels allow flow to continue as normal + has_errors = any(message.level_tag in ["error"] for message in storage) if has_errors: # Re-render the change form if there are errors or warnings @@ -1552,13 +1563,14 @@ class DomainInvitationAdmin(BaseInvitationAdmin): portfolio_invitation.save() messages.success(request, f"{requested_email} has been invited to the organization: {domain_org}") - send_domain_invitation_email( + if not 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, - ) + ): + messages.warning(request, "Could not send email confirmation to existing domain managers.") if requested_user is not None: # Domain Invitation creation for an existing User obj.retrieve() @@ -1657,6 +1669,14 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin): # Call the parent save method to save the object super().save_model(request, obj, form, change) + def delete_queryset(self, request, queryset): + """We override the delete method in the model. + When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action, + the model delete does not get called. This method gets called instead. + This override makes sure our code in the model gets executed in these situations.""" + for obj in queryset: + obj.delete() # Calls the overridden delete method on each instance + class DomainInformationResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the diff --git a/src/registrar/assets/src/js/getgov/table-base.js b/src/registrar/assets/src/js/getgov/table-base.js index 0abfee9b6..ce4397887 100644 --- a/src/registrar/assets/src/js/getgov/table-base.js +++ b/src/registrar/assets/src/js/getgov/table-base.js @@ -129,7 +129,7 @@ export class BaseTable { this.displayName = itemName; this.sectionSelector = itemName + 's'; this.tableWrapper = document.getElementById(`${this.sectionSelector}__table-wrapper`); - this.tableHeaders = document.querySelectorAll(`#${this.sectionSelector} th[data-sortable]`); + this.tableHeaderSortButtons = document.querySelectorAll(`#${this.sectionSelector} th[data-sortable] button`); this.currentSortBy = 'id'; this.currentOrder = 'asc'; this.currentStatus = []; @@ -303,13 +303,18 @@ export class BaseTable { * A helper that resets sortable table headers * */ - unsetHeader = (header) => { - header.removeAttribute('aria-sort'); - let headerName = header.innerText; - const headerLabel = `${headerName}, sortable column, currently unsorted"`; - const headerButtonLabel = `Click to sort by ascending order.`; - header.setAttribute("aria-label", headerLabel); - header.querySelector('.usa-table__header__button').setAttribute("title", headerButtonLabel); + unsetHeader = (headerSortButton) => { + let header = headerSortButton.closest('th'); + if (header) { + header.removeAttribute('aria-sort'); + let headerName = header.innerText; + const headerLabel = `${headerName}, sortable column, currently unsorted"`; + const headerButtonLabel = `Click to sort by ascending order.`; + header.setAttribute("aria-label", headerLabel); + header.querySelector('.usa-table__header__button').setAttribute("title", headerButtonLabel); + } else { + console.warn('Issue with DOM'); + } }; /** @@ -505,24 +510,21 @@ export class BaseTable { // Add event listeners to table headers for sorting initializeTableHeaders() { - this.tableHeaders.forEach(header => { - header.addEventListener('click', event => { - let button = header.querySelector('.usa-table__header__button') - const sortBy = header.getAttribute('data-sortable'); - let order = 'asc'; - // sort order will be ascending, unless the currently sorted column is ascending, and the user - // is selecting the same column to sort in descending order - if (sortBy === this.currentSortBy) { - order = this.currentOrder === 'asc' ? 'desc' : 'asc'; - } - // load the results with the updated sort - this.loadTable(1, sortBy, order); - // If the click occurs outside of the button, need to simulate a button click in order - // for USWDS listener on the button to execute. - // Check first to see if click occurs outside of the button - if (!button.contains(event.target)) { - // Simulate a button click - button.click(); + this.tableHeaderSortButtons.forEach(tableHeader => { + tableHeader.addEventListener('click', event => { + let header = tableHeader.closest('th'); + if (header) { + const sortBy = header.getAttribute('data-sortable'); + let order = 'asc'; + // sort order will be ascending, unless the currently sorted column is ascending, and the user + // is selecting the same column to sort in descending order + if (sortBy === this.currentSortBy) { + order = this.currentOrder === 'asc' ? 'desc' : 'asc'; + } + // load the results with the updated sort + this.loadTable(1, sortBy, order); + } else { + console.warn('Issue with DOM'); } }); }); @@ -587,9 +589,9 @@ export class BaseTable { // Reset UI and accessibility resetHeaders() { - this.tableHeaders.forEach(header => { + this.tableHeaderSortButtons.forEach(headerSortButton => { // Unset sort UI in headers - this.unsetHeader(header); + this.unsetHeader(headerSortButton); }); // Reset the announcement region this.tableAnnouncementRegion.innerHTML = ''; diff --git a/src/registrar/assets/src/js/getgov/table-member-domains.js b/src/registrar/assets/src/js/getgov/table-member-domains.js index f9b789e1f..d1455c4dc 100644 --- a/src/registrar/assets/src/js/getgov/table-member-domains.js +++ b/src/registrar/assets/src/js/getgov/table-member-domains.js @@ -35,16 +35,19 @@ export class MemberDomainsTable extends BaseTable { showElement(dataWrapper); hideElement(noSearchResultsWrapper); hideElement(noDataWrapper); + this.tableAnnouncementRegion.innerHTML = ''; } else { hideElement(dataWrapper); showElement(noSearchResultsWrapper); hideElement(noDataWrapper); + this.tableAnnouncementRegion.innerHTML = this.noSearchResultsWrapper.innerHTML; } } else { hideElement(searchSection); hideElement(dataWrapper); hideElement(noSearchResultsWrapper); showElement(noDataWrapper); + this.tableAnnouncementRegion.innerHTML = this.noDataWrapper.innerHTML; } }; } diff --git a/src/registrar/assets/src/sass/_theme/_tooltips.scss b/src/registrar/assets/src/sass/_theme/_tooltips.scss index 22b5cf534..e1e31cbec 100644 --- a/src/registrar/assets/src/sass/_theme/_tooltips.scss +++ b/src/registrar/assets/src/sass/_theme/_tooltips.scss @@ -29,7 +29,7 @@ font-weight: 400 !important; } -.domains__table { +.domains__table, .usa-table { /* Trick tooltips in the domains table to do 2 things... 1 - Shrink itself to a padded viewport window diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index a58e3e2f9..58250e85c 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -25,6 +25,7 @@ from typing import Final from botocore.config import Config import json import logging +import traceback from django.utils.log import ServerFormatter # # # ### @@ -471,7 +472,11 @@ class JsonFormatter(logging.Formatter): "lineno": record.lineno, "message": record.getMessage(), } - return json.dumps(log_record) + # Capture exception info if it exists + if record.exc_info: + log_record["exception"] = "".join(traceback.format_exception(*record.exc_info)) + + return json.dumps(log_record, ensure_ascii=False) class JsonServerFormatter(ServerFormatter): diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 11c564c36..8feeb0794 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -8,6 +8,7 @@ from registrar.models import DomainInvitation, UserPortfolioPermission from .utility.portfolio_helper import ( UserPortfolioPermissionChoices, UserPortfolioRoleChoices, + cleanup_after_portfolio_member_deletion, validate_portfolio_invitation, ) # type: ignore from .utility.time_stamped_model import TimeStampedModel @@ -115,3 +116,27 @@ class PortfolioInvitation(TimeStampedModel): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() validate_portfolio_invitation(self) + + def delete(self, *args, **kwargs): + + User = get_user_model() + + email = self.email # Capture the email before the instance is deleted + portfolio = self.portfolio # Capture the portfolio before the instance is deleted + + # Call the superclass delete method to actually delete the instance + super().delete(*args, **kwargs) + + if self.status == self.PortfolioInvitationStatus.INVITED: + + # Query the user by email + users = User.objects.filter(email=email) + + if users.count() > 1: + # This should never happen, log an error if more than one object is returned + logger.error(f"Multiple users found with the same email: {email}") + + # Retrieve the first user, or None if no users are found + user = users.first() + + cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=email, user=user) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index c4be90a9b..11d9c56e3 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -5,6 +5,7 @@ from registrar.models.utility.portfolio_helper import ( UserPortfolioRoleChoices, DomainRequestPermissionDisplay, MemberPermissionDisplay, + cleanup_after_portfolio_member_deletion, validate_user_portfolio_permission, ) from .utility.time_stamped_model import TimeStampedModel @@ -188,3 +189,13 @@ class UserPortfolioPermission(TimeStampedModel): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() validate_user_portfolio_permission(self) + + def delete(self, *args, **kwargs): + + user = self.user # Capture the user before the instance is deleted + portfolio = self.portfolio # Capture the portfolio before the instance is deleted + + # Call the superclass delete method to actually delete the instance + super().delete(*args, **kwargs) + + cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=user.email, user=user) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index b3bb07c3d..8c42b80c7 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -210,3 +210,32 @@ def validate_portfolio_invitation(portfolio_invitation): "This user is already assigned to a portfolio invitation. " "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." ) + + +def cleanup_after_portfolio_member_deletion(portfolio, email, user=None): + """ + Cleans up after removing a portfolio member or a portfolio invitation. + + Args: + portfolio: portfolio + user: passed when removing a portfolio member. + email: passed when removing a portfolio invitation, or passed as user.email + when removing a portfolio member. + """ + + DomainInvitation = apps.get_model("registrar.DomainInvitation") + UserDomainRole = apps.get_model("registrar.UserDomainRole") + + # Fetch domain invitations matching the criteria + invitations = DomainInvitation.objects.filter( + email=email, domain__domain_info__portfolio=portfolio, status=DomainInvitation.DomainInvitationStatus.INVITED + ) + + # Call `cancel_invitation` on each invitation + for invitation in invitations: + invitation.cancel_invitation() + invitation.save() + + if user: + # Remove user's domain roles for the current portfolio + UserDomainRole.objects.filter(user=user, domain__domain_info__portfolio=portfolio).delete() diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index b026a7a6b..e411f1494 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -57,7 +57,7 @@ -