From 7ddf1d87bc7f2c4da8033e767fc79865ec8bf191 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 28 Jun 2024 14:55:40 -0600 Subject: [PATCH 01/71] changes --- src/docker-compose.yml | 2 +- src/registrar/admin.py | 10 +++--- src/registrar/models/domain_request.py | 31 ++++++++++++++----- .../action_needed_reasons/custom_email.txt | 2 ++ 4 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 src/registrar/templates/emails/action_needed_reasons/custom_email.txt diff --git a/src/docker-compose.yml b/src/docker-compose.yml index 1a9064ac8..39282ff96 100644 --- a/src/docker-compose.yml +++ b/src/docker-compose.yml @@ -67,8 +67,8 @@ services: # command: "python" command: > bash -c " python manage.py migrate && - python manage.py load && python manage.py createcachetable && + python manage.py load && python manage.py runserver 0.0.0.0:8080" db: diff --git a/src/registrar/admin.py b/src/registrar/admin.py index eee2eda2f..44dedd25d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1690,8 +1690,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().save_model(request, obj, form, change) # == Handle non-status changes == # - # Change this in #1901. Add a check on "not self.action_needed_reason_email" - if obj.action_needed_reason: + if obj.action_needed_reason and not self.action_needed_reason_email: self._handle_action_needed_reason_email(obj) should_save = True @@ -1911,14 +1910,17 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Call the superclass method with updated extra_context return super().change_view(request, object_id, form_url, extra_context) + # TODO - scrap this approach and just centralize everything def get_all_action_needed_reason_emails_as_json(self, domain_request): """Returns a json dictionary of every action needed reason and its associated email for this particular domain request.""" emails = {} for action_needed_reason in domain_request.ActionNeededReasons: enum_value = action_needed_reason.value - # Change this in #1901. Just add a check for the current value. - emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value) + if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: + emails[enum_value] = domain_request.action_needed_reason_email + else: + emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value) return json.dumps(emails) def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index d26bb3284..febf0a5e6 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -672,7 +672,7 @@ class DomainRequest(TimeStampedModel): logger.error(f"Can't query an approved domain while attempting {called_from}") def _send_status_update_email( - self, new_status, email_template, email_template_subject, send_email=True, bcc_address="", wrap_email=False + self, new_status, email_template, email_template_subject, bcc_address="", context=None, **kwargs ): """Send a status update email to the creator. @@ -683,13 +683,21 @@ class DomainRequest(TimeStampedModel): If the waffle flag "profile_feature" is active, then this email will be sent to the domain request creator rather than the submitter + kwargs: send_email: bool -> Used to bypass the send_templated_email function, in the event we just want to log that an email would have been sent, rather than actually sending one. wrap_email: bool -> Wraps emails using `wrap_text_and_preserve_paragraphs` if any given paragraph exceeds our desired max length (for prettier display). + + custom_email_content: str -> Renders an email with the content of this string as its body text. """ + # Email config options + wrap_email = kwargs.get("wrap_email", False) + send_email = kwargs.get("send_email", True) + custom_email_content = kwargs.get("custom_email_content", None) + recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter if recipient is None or recipient.email is None: logger.warning( @@ -705,15 +713,21 @@ class DomainRequest(TimeStampedModel): return None try: + if not context: + context = { + "domain_request": self, + # This is the user that we refer to in the email + "recipient": recipient, + } + + if custom_email_content: + context["custom_email_content"] = custom_email_content + send_templated_email( email_template, email_template_subject, recipient.email, - context={ - "domain_request": self, - # This is the user that we refer to in the email - "recipient": recipient, - }, + context=context, bcc_address=bcc_address, wrap_email=wrap_email, ) @@ -844,12 +858,12 @@ class DomainRequest(TimeStampedModel): if self.action_needed_reason and self.action_needed_reason != self.ActionNeededReasons.OTHER: self._send_action_needed_reason_email(send_email) - def _send_action_needed_reason_email(self, send_email=True): + def _send_action_needed_reason_email(self, send_email=True, custom_email_content=None): """Sends out an automatic email for each valid action needed reason provided""" # Store the filenames of the template and template subject email_template_name: str = "" - email_template_subject_name: str = "" + email_template_subject_name: str = "" if not custom_email_content else "custom_email" # Check for the "type" of action needed reason. can_send_email = True @@ -877,6 +891,7 @@ class DomainRequest(TimeStampedModel): email_template_subject=f"emails/action_needed_reasons/{email_template_subject_name}", send_email=send_email, bcc_address=bcc_address, + custom_email_content=custom_email_content, wrap_email=True, ) diff --git a/src/registrar/templates/emails/action_needed_reasons/custom_email.txt b/src/registrar/templates/emails/action_needed_reasons/custom_email.txt new file mode 100644 index 000000000..8e58c8f2e --- /dev/null +++ b/src/registrar/templates/emails/action_needed_reasons/custom_email.txt @@ -0,0 +1,2 @@ +{% comment %} {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} {% endcomment %} +{{ custom_email_content }} \ No newline at end of file From 1cbd9234bee5a49655330fec6ee0316c744f8b10 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 28 Jun 2024 15:24:22 -0600 Subject: [PATCH 02/71] basic logic --- src/registrar/admin.py | 14 +++++++------- src/registrar/assets/js/get-gov-admin.js | 3 ++- src/registrar/models/domain_request.py | 9 +++++---- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b2db130d9..1fed780e1 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1606,7 +1606,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "is_election_board", "federal_agency", "status_history", - "action_needed_reason_email", ) # Read only that we'll leverage for CISA Analysts @@ -1705,7 +1704,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().save_model(request, obj, form, change) # == Handle non-status changes == # - if obj.action_needed_reason and not self.action_needed_reason_email: + if obj.action_needed_reason and not obj.action_needed_reason_email: self._handle_action_needed_reason_email(obj) should_save = True @@ -1933,13 +1932,14 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): emails = {} for action_needed_reason in domain_request.ActionNeededReasons: enum_value = action_needed_reason.value + custom_text = None if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: - emails[enum_value] = domain_request.action_needed_reason_email - else: - emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value) + custom_text = domain_request.action_needed_reason_email + + emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value, custom_text) return json.dumps(emails) - def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str): + def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str, custom_text=None): """Returns the default email associated with the given action needed reason""" if action_needed_reason is None or action_needed_reason == domain_request.ActionNeededReasons.OTHER: return { @@ -1961,7 +1961,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return { "subject_text": subject_template.render(context=context), - "email_body_text": template.render(context=context), + "email_body_text": template.render(context=context) if not custom_text else custom_text, } def process_log_entry(self, log_entry): diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 898c41c4b..6d753744f 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -525,7 +525,7 @@ function initializeWidgetOnList(list, parentId) { */ (function () { let actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); - let actionNeededEmail = document.querySelector("#action_needed_reason_email_view_more"); + let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); if(actionNeededReasonDropdown && actionNeededEmail) { // Add a change listener to the action needed reason dropdown handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail); @@ -546,6 +546,7 @@ function initializeWidgetOnList(list, parentId) { let actionNeededEmails = JSON.parse(document.getElementById('action-needed-emails-data').textContent) let emailData = actionNeededEmails[reason]; if (emailData) { + // TODO: do we need a revert to default button? let emailBody = emailData.email_body_text if (emailBody) { actionNeededEmail.value = emailBody diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index febf0a5e6..830ad9480 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -785,8 +785,8 @@ class DomainRequest(TimeStampedModel): "submission confirmation", "emails/submission_confirmation.txt", "emails/submission_confirmation_subject.txt", - True, - bcc_address, + send_email=True, + bcc_address=bcc_address, ) @transition( @@ -856,7 +856,8 @@ class DomainRequest(TimeStampedModel): # Send out an email if an action needed reason exists if self.action_needed_reason and self.action_needed_reason != self.ActionNeededReasons.OTHER: - self._send_action_needed_reason_email(send_email) + custom_email_content = self.action_needed_reason_email + self._send_action_needed_reason_email(send_email, custom_email_content) def _send_action_needed_reason_email(self, send_email=True, custom_email_content=None): """Sends out an automatic email for each valid action needed reason provided""" @@ -951,7 +952,7 @@ class DomainRequest(TimeStampedModel): "domain request approved", "emails/status_change_approved.txt", "emails/status_change_approved_subject.txt", - send_email, + send_email=send_email, ) @transition( From b6a70e6d3c585968d3b3c4291fcfbc75ac03e1c9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 1 Jul 2024 09:56:09 -0600 Subject: [PATCH 03/71] custom content logic --- src/registrar/admin.py | 6 +++++- src/registrar/models/domain_request.py | 11 +++++++---- .../emails/action_needed_reasons/custom_email.txt | 5 +++-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1fed780e1..e2cd6e17d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1948,7 +1948,11 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): } # Get the email body - template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" + if not custom_text: + template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" + else: + template_path = f"emails/action_needed_reasons/custom_email.txt" + template = get_template(template_path) # Get the email subject diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 830ad9480..62eb3f4f8 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -618,7 +618,8 @@ class DomainRequest(TimeStampedModel): if was_already_action_needed and (reason_exists and reason_changed): # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: - self._send_action_needed_reason_email() + _email_content = self.action_needed_reason_email + self._send_action_needed_reason_email(custom_email_content=_email_content) def sync_yes_no_form_fields(self): """Some yes/no forms use a db field to track whether it was checked or not. @@ -863,8 +864,8 @@ class DomainRequest(TimeStampedModel): """Sends out an automatic email for each valid action needed reason provided""" # Store the filenames of the template and template subject - email_template_name: str = "" - email_template_subject_name: str = "" if not custom_email_content else "custom_email" + email_template_name: str = "" if not custom_email_content else "custom_email.txt" + email_template_subject_name: str = "" # Check for the "type" of action needed reason. can_send_email = True @@ -876,8 +877,10 @@ class DomainRequest(TimeStampedModel): # Assumes that the template name matches the action needed reason if nothing is specified. # This is so you can override if you need, or have this taken care of for you. - if not email_template_name and not email_template_subject_name: + if not email_template_name: email_template_name = f"{self.action_needed_reason}.txt" + + if not email_template_subject_name: email_template_subject_name = f"{self.action_needed_reason}_subject.txt" bcc_address = "" diff --git a/src/registrar/templates/emails/action_needed_reasons/custom_email.txt b/src/registrar/templates/emails/action_needed_reasons/custom_email.txt index 8e58c8f2e..58e539816 100644 --- a/src/registrar/templates/emails/action_needed_reasons/custom_email.txt +++ b/src/registrar/templates/emails/action_needed_reasons/custom_email.txt @@ -1,2 +1,3 @@ -{% comment %} {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} {% endcomment %} -{{ custom_email_content }} \ No newline at end of file +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{{ custom_email_content }} +{% endautoescape %} From 5815d6733c9db3f258879178ace83cb069e3860b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jul 2024 11:28:58 -0600 Subject: [PATCH 04/71] hash --- src/registrar/models/domain_request.py | 29 +++++++++++++++++-- .../models/utility/generic_helper.py | 5 ++++ src/registrar/tests/test_admin.py | 11 +++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 62eb3f4f8..66c70f9a7 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1,7 +1,7 @@ from __future__ import annotations from typing import Union import logging - +from django.template.loader import get_template from django.apps import apps from django.conf import settings from django.db import models @@ -10,7 +10,7 @@ from django.utils import timezone from waffle import flag_is_active from registrar.models.domain import Domain from registrar.models.federal_agency import FederalAgency -from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, convert_string_to_sha256_hash from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices @@ -864,9 +864,16 @@ class DomainRequest(TimeStampedModel): """Sends out an automatic email for each valid action needed reason provided""" # Store the filenames of the template and template subject - email_template_name: str = "" if not custom_email_content else "custom_email.txt" + email_template_name: str = "" email_template_subject_name: str = "" + # Check if the current email that we sent out is the same as our defaults. + # If these hashes differ, then that means that we're sending custom content. + default_email_hash = self._get_action_needed_reason_email_hash() + current_email_hash = convert_string_to_sha256_hash(self.action_needed_reason_email) + if default_email_hash != current_email_hash: + email_template_name = "custom_email.txt" + # Check for the "type" of action needed reason. can_send_email = True match self.action_needed_reason: @@ -899,6 +906,22 @@ class DomainRequest(TimeStampedModel): wrap_email=True, ) + # TODO - rework this + def _get_action_needed_reason_email_hash(self): + """Returns the default email associated with the given action needed reason""" + if self.action_needed_reason is None or self.action_needed_reason == self.ActionNeededReasons.OTHER: + return None + + # Get the email body + template_path = f"emails/action_needed_reasons/{self.action_needed_reason}.txt" + template = get_template(template_path) + + recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter + # Return the content of the rendered views + context = {"domain_request": self, "recipient": recipient} + body_text = template.render(context=context) + return convert_string_to_sha256_hash(body_text) + @transition( field="status", source=[ diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index f9d4303c4..f35e2f619 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -2,6 +2,7 @@ import time import logging +import hashlib from urllib.parse import urlparse, urlunparse, urlencode @@ -321,3 +322,7 @@ def convert_queryset_to_dict(queryset, is_model=True, key="id"): request_dict = {value[key]: value for value in queryset} return request_dict + + +def convert_string_to_sha256_hash(string_to_convert): + return hashlib.sha256(string_to_convert.encode('utf-8')).hexdigest() \ No newline at end of file diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 44a131d55..3dd0cdb6c 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1540,6 +1540,17 @@ class TestDomainRequestAdmin(MockEppLib): # Should be unchanged from before self.assertEqual(len(self.mock_client.EMAILS_SENT), 4) + # Tests if an analyst can override existing email content + questionable_ao = DomainRequest.ActionNeededReasons.QUESTIONABLE_AUTHORIZING_OFFICIAL + domain_request.action_needed_reason_email = "custom email content" + domain_request.save() + self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_ao) + + self.assert_email_is_accurate( + "custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL + ) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) + def test_save_model_sends_submitted_email(self): """When transitioning to submitted from started or withdrawn on a domain request, an email is sent out. From 34f6b03078a09499da281fbcce7b1d13414ae60a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jul 2024 11:29:57 -0600 Subject: [PATCH 05/71] Update test_admin.py --- src/registrar/tests/test_admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 11f9bf5fe..ca5e4a93c 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1480,10 +1480,10 @@ class TestDomainRequestAdmin(MockEppLib): self.assertEqual(len(self.mock_client.EMAILS_SENT), 4) # Tests if an analyst can override existing email content - questionable_ao = DomainRequest.ActionNeededReasons.QUESTIONABLE_AUTHORIZING_OFFICIAL + questionable_so = DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL domain_request.action_needed_reason_email = "custom email content" domain_request.save() - self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_ao) + self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_so) self.assert_email_is_accurate( "custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL From a8fa24411e9238f43626b32a1d644e8bdea1f2c8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jul 2024 12:56:26 -0600 Subject: [PATCH 06/71] Email logic (needs cleanup) --- src/registrar/admin.py | 21 ++++++++++++------- src/registrar/models/domain_request.py | 16 +++++++------- .../models/utility/generic_helper.py | 4 +++- src/registrar/utility/email.py | 2 ++ 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a7e078f46..9efbf1dae 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1703,12 +1703,21 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().save_model(request, obj, form, change) # == Handle non-status changes == # - if obj.action_needed_reason and not obj.action_needed_reason_email: - self._handle_action_needed_reason_email(obj) - should_save = True - # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) + + if obj.action_needed_reason: + text = self._get_action_needed_reason_default_email_text(obj, obj.action_needed_reason) + body_text = text.get("email_body_text") + if body_text: + body_text.strip().lstrip("\n") + is_default_email = body_text == obj.action_needed_reason_email + reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + if is_default_email and reason_changed: + obj.action_needed_reason_email = body_text + should_save = True + + if obj.status == original_obj.status: # If the status hasn't changed, let the base function take care of it return super().save_model(request, obj, form, change) @@ -1721,10 +1730,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if should_save: return super().save_model(request, obj, form, change) - def _handle_action_needed_reason_email(self, obj): - text = self._get_action_needed_reason_default_email_text(obj, obj.action_needed_reason) - obj.action_needed_reason_email = text.get("email_body_text") - def _handle_status_change(self, request, obj, original_obj): """ Checks for various conditions when a status change is triggered. diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 2c155a6d9..53c5765e9 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -619,7 +619,7 @@ class DomainRequest(TimeStampedModel): # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: _email_content = self.action_needed_reason_email - self._send_action_needed_reason_email(custom_email_content=_email_content) + self._send_action_needed_reason_email(email_content=_email_content) def sync_yes_no_form_fields(self): """Some yes/no forms use a db field to track whether it was checked or not. @@ -857,10 +857,10 @@ class DomainRequest(TimeStampedModel): # Send out an email if an action needed reason exists if self.action_needed_reason and self.action_needed_reason != self.ActionNeededReasons.OTHER: - custom_email_content = self.action_needed_reason_email - self._send_action_needed_reason_email(send_email, custom_email_content) + email_content = self.action_needed_reason_email + self._send_action_needed_reason_email(send_email, email_content) - def _send_action_needed_reason_email(self, send_email=True, custom_email_content=None): + def _send_action_needed_reason_email(self, send_email=True, email_content=None): """Sends out an automatic email for each valid action needed reason provided""" # Store the filenames of the template and template subject @@ -871,7 +871,8 @@ class DomainRequest(TimeStampedModel): # If these hashes differ, then that means that we're sending custom content. default_email_hash = self._get_action_needed_reason_email_hash() current_email_hash = convert_string_to_sha256_hash(self.action_needed_reason_email) - if default_email_hash != current_email_hash: + if self.action_needed_reason_email and default_email_hash != current_email_hash: + print(f"sending custom email for: {current_email_hash}") email_template_name = "custom_email.txt" # Check for the "type" of action needed reason. @@ -902,7 +903,7 @@ class DomainRequest(TimeStampedModel): email_template_subject=f"emails/action_needed_reasons/{email_template_subject_name}", send_email=send_email, bcc_address=bcc_address, - custom_email_content=custom_email_content, + custom_email_content=email_content, wrap_email=True, ) @@ -919,7 +920,8 @@ class DomainRequest(TimeStampedModel): recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter # Return the content of the rendered views context = {"domain_request": self, "recipient": recipient} - body_text = template.render(context=context) + body_text = template.render(context=context).strip().lstrip("\n") + print(f"body: {body_text}") return convert_string_to_sha256_hash(body_text) @transition( diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index f35e2f619..75c2492c4 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -324,5 +324,7 @@ def convert_queryset_to_dict(queryset, is_model=True, key="id"): return request_dict -def convert_string_to_sha256_hash(string_to_convert): +def convert_string_to_sha256_hash(string_to_convert: str): + if not string_to_convert: + return None return hashlib.sha256(string_to_convert.encode('utf-8')).hexdigest() \ No newline at end of file diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 6fdaa3e3d..3d49dab02 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -45,6 +45,8 @@ def send_templated_email( template = get_template(template_name) email_body = template.render(context=context) + if email_body: + email_body.strip().lstrip("\n") subject_template = get_template(subject_template_name) subject = subject_template.render(context=context) From 55a74f63dd09478eb027c525669ef61c150cf0ef Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 09:11:09 -0600 Subject: [PATCH 07/71] Cleanup --- src/registrar/models/domain_request.py | 18 +++++++----------- src/registrar/models/utility/generic_helper.py | 6 ------ 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 53c5765e9..5ec72aad5 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -868,11 +868,9 @@ class DomainRequest(TimeStampedModel): email_template_subject_name: str = "" # Check if the current email that we sent out is the same as our defaults. - # If these hashes differ, then that means that we're sending custom content. - default_email_hash = self._get_action_needed_reason_email_hash() - current_email_hash = convert_string_to_sha256_hash(self.action_needed_reason_email) - if self.action_needed_reason_email and default_email_hash != current_email_hash: - print(f"sending custom email for: {current_email_hash}") + # If these differ, then that means that we're sending custom content. + default_email = self.get_default_action_needed_reason_email(self.action_needed_reason) + if self.action_needed_reason_email and self.action_needed_reason_email != default_email: email_template_name = "custom_email.txt" # Check for the "type" of action needed reason. @@ -907,22 +905,20 @@ class DomainRequest(TimeStampedModel): wrap_email=True, ) - # TODO - rework this - def _get_action_needed_reason_email_hash(self): + def get_default_action_needed_reason_email(self, action_needed_reason): """Returns the default email associated with the given action needed reason""" - if self.action_needed_reason is None or self.action_needed_reason == self.ActionNeededReasons.OTHER: + if action_needed_reason is None or action_needed_reason == self.ActionNeededReasons.OTHER: return None # Get the email body - template_path = f"emails/action_needed_reasons/{self.action_needed_reason}.txt" + template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" template = get_template(template_path) recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter # Return the content of the rendered views context = {"domain_request": self, "recipient": recipient} body_text = template.render(context=context).strip().lstrip("\n") - print(f"body: {body_text}") - return convert_string_to_sha256_hash(body_text) + return body_text @transition( field="status", diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 75c2492c4..72dee8204 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -322,9 +322,3 @@ def convert_queryset_to_dict(queryset, is_model=True, key="id"): request_dict = {value[key]: value for value in queryset} return request_dict - - -def convert_string_to_sha256_hash(string_to_convert: str): - if not string_to_convert: - return None - return hashlib.sha256(string_to_convert.encode('utf-8')).hexdigest() \ No newline at end of file From 87c7a4dba282d46ea7d3ae7241f06a53d53be6b2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 09:19:33 -0600 Subject: [PATCH 08/71] Update admin.py --- src/registrar/admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ae3a95740..a1ba6559d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1958,7 +1958,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Call the superclass method with updated extra_context return super().change_view(request, object_id, form_url, extra_context) - # TODO - scrap this approach and just centralize everything def get_all_action_needed_reason_emails_as_json(self, domain_request): """Returns a json dictionary of every action needed reason and its associated email for this particular domain request.""" From 4bdf2cbe8f27c0e3712b44e9577b937b5c58607c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 09:27:55 -0600 Subject: [PATCH 09/71] Fix imports --- src/registrar/assets/js/get-gov-admin.js | 1 - src/registrar/models/domain_request.py | 5 ++--- src/registrar/utility/email.py | 2 ++ 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 83c958566..2a01eb304 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -548,7 +548,6 @@ function initializeWidgetOnList(list, parentId) { let actionNeededEmails = JSON.parse(document.getElementById('action-needed-emails-data').textContent) let emailData = actionNeededEmails[reason]; if (emailData) { - // TODO: do we need a revert to default button? let emailBody = emailData.email_body_text if (emailBody) { actionNeededEmail.value = emailBody diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 7bb3a5a96..387840161 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -10,7 +10,7 @@ from django.utils import timezone from waffle import flag_is_active from registrar.models.domain import Domain from registrar.models.federal_agency import FederalAgency -from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, convert_string_to_sha256_hash +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices @@ -627,8 +627,7 @@ class DomainRequest(TimeStampedModel): if was_already_action_needed and (reason_exists and reason_changed): # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: - _email_content = self.action_needed_reason_email - self._send_action_needed_reason_email(email_content=_email_content) + self._send_action_needed_reason_email(email_content=self.action_needed_reason_email) def sync_yes_no_form_fields(self): """Some yes/no forms use a db field to track whether it was checked or not. diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 3d49dab02..7f0b997fa 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -45,6 +45,8 @@ def send_templated_email( template = get_template(template_name) email_body = template.render(context=context) + + # Do cleanup on the email body. Mostly for emails with custom content. if email_body: email_body.strip().lstrip("\n") From f84989589762c4ff268742f78065b28406c64b8c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 12:09:45 -0600 Subject: [PATCH 10/71] Logic to update label --- src/registrar/assets/js/get-gov.js | 26 +++++++++++++++++++++ src/registrar/assets/sass/_theme/_base.scss | 2 +- src/registrar/forms/user_profile.py | 5 +++- src/registrar/models/contact.py | 7 ++++-- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 7052d786f..6373f176f 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1834,6 +1834,32 @@ document.addEventListener('DOMContentLoaded', function() { // When the edit button is clicked, show the input field under it handleEditButtonClick(fieldName, button); + + let editableFormGroup = button.parentElement.parentElement.parentElement; + if (editableFormGroup){ + let readonlyField = editableFormGroup.querySelector(".input-with-edit-button__readonly-field") + let inputField = document.getElementById(`id_${fieldName}`).value; + if (!inputField) { + return; + } + + let inputFieldValue = inputField.value + if (readonlyField && inputFieldValue){ + if (fieldName == "full_name"){ + let firstName = document.querySelector(`#id_first_name`).value; + let middleName = document.querySelector(`#id_middle_name`).value; + let lastName = document.querySelector(`#id_last_name`).value; + if (firstName && middleName && lastName) { + let values = [firstName.value, middleName.value, lastName.value] + readonlyField.innerHTML = values.join(" "); + }else { + readonlyField.innerHTML = "Unknown"; + } + }else { + readonlyField.innerHTML = inputValue; + } + } + } } }); } diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index b2bad1edb..b7cedfe85 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -190,7 +190,7 @@ abbr[title] { svg.usa-icon { color: #{$dhs-red}; } - div.readonly-field { + div.input-with-edit-button__readonly-field-field { color: #{$dhs-red}; } } diff --git a/src/registrar/forms/user_profile.py b/src/registrar/forms/user_profile.py index 682e1a5df..60e5032c8 100644 --- a/src/registrar/forms/user_profile.py +++ b/src/registrar/forms/user_profile.py @@ -93,4 +93,7 @@ class FinishSetupProfileForm(UserProfileForm): self.fields["title"].label = "Title or role in your organization" # Define the "full_name" value - self.fields["full_name"].initial = self.instance.get_formatted_name() + full_name = None + if self.instance.first_name and self.instance.last_name: + full_name = self.instance.get_formatted_name(return_unknown_when_none=False) + self.fields["full_name"].initial = full_name diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index f94938dd1..b0d6f3ac3 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -102,10 +102,13 @@ class Contact(TimeStampedModel): return getattr(self, relation).count() > threshold return False - def get_formatted_name(self): + def get_formatted_name(self, return_unknown_when_none=True): """Returns the contact's name in Western order.""" names = [n for n in [self.first_name, self.middle_name, self.last_name] if n] - return " ".join(names) if names else "Unknown" + if names: + return " ".join(names) + else: + return "Unknown" if return_unknown_when_none else None def has_contact_info(self): return bool(self.title or self.email or self.phone) From 08a3ba53780f2f5dde7f62d050bfc242b3ff8262 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 12:11:48 -0600 Subject: [PATCH 11/71] css changes --- src/registrar/assets/sass/_theme/_base.scss | 2 +- src/registrar/templates/includes/readonly_input.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index b7cedfe85..5fb8ce86b 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -190,7 +190,7 @@ abbr[title] { svg.usa-icon { color: #{$dhs-red}; } - div.input-with-edit-button__readonly-field-field { + div.input-with-edit-button__readonly-field { color: #{$dhs-red}; } } diff --git a/src/registrar/templates/includes/readonly_input.html b/src/registrar/templates/includes/readonly_input.html index ebd5d788e..47db97f00 100644 --- a/src/registrar/templates/includes/readonly_input.html +++ b/src/registrar/templates/includes/readonly_input.html @@ -8,7 +8,7 @@ {%endif %} -
+
{% if field.name != "phone" %} {{ field.value }} {% else %} From a9c91bc94a79703b8eff9e4a976f027311278106 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 12:13:38 -0600 Subject: [PATCH 12/71] js bug --- src/registrar/assets/js/get-gov.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 6373f176f..73e5d1ee7 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1838,7 +1838,7 @@ document.addEventListener('DOMContentLoaded', function() { let editableFormGroup = button.parentElement.parentElement.parentElement; if (editableFormGroup){ let readonlyField = editableFormGroup.querySelector(".input-with-edit-button__readonly-field") - let inputField = document.getElementById(`id_${fieldName}`).value; + let inputField = document.getElementById(`id_${fieldName}`); if (!inputField) { return; } From 7a3f00971378824b140e32b610e26dbff2e6ae2a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 12:54:10 -0600 Subject: [PATCH 13/71] Edge cases --- src/registrar/assets/js/get-gov.js | 4 +--- src/registrar/forms/user_profile.py | 7 +++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 73e5d1ee7..34e22e49d 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1844,7 +1844,7 @@ document.addEventListener('DOMContentLoaded', function() { } let inputFieldValue = inputField.value - if (readonlyField && inputFieldValue){ + if (readonlyField && inputFieldValue || fieldName == "full_name"){ if (fieldName == "full_name"){ let firstName = document.querySelector(`#id_first_name`).value; let middleName = document.querySelector(`#id_middle_name`).value; @@ -1855,8 +1855,6 @@ document.addEventListener('DOMContentLoaded', function() { }else { readonlyField.innerHTML = "Unknown"; } - }else { - readonlyField.innerHTML = inputValue; } } } diff --git a/src/registrar/forms/user_profile.py b/src/registrar/forms/user_profile.py index 60e5032c8..02bc4e58f 100644 --- a/src/registrar/forms/user_profile.py +++ b/src/registrar/forms/user_profile.py @@ -71,7 +71,7 @@ class UserProfileForm(forms.ModelForm): class FinishSetupProfileForm(UserProfileForm): """Form for updating user profile.""" - full_name = forms.CharField(required=True, label="Full name") + full_name = forms.CharField(required=False, label="Full name") def clean(self): cleaned_data = super().clean() @@ -93,7 +93,10 @@ class FinishSetupProfileForm(UserProfileForm): self.fields["title"].label = "Title or role in your organization" # Define the "full_name" value - full_name = None + full_name = "" if self.instance.first_name and self.instance.last_name: full_name = self.instance.get_formatted_name(return_unknown_when_none=False) self.fields["full_name"].initial = full_name + + # Set full_name as required for styling purposes + self.fields["full_name"].widget.attrs['required'] = 'required' From 2c6aa1bab1143c3965edd0aafd3040778334ae65 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 12:56:15 -0600 Subject: [PATCH 14/71] Update get-gov.js --- src/registrar/assets/js/get-gov.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 34e22e49d..30b80d356 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1844,7 +1844,7 @@ document.addEventListener('DOMContentLoaded', function() { } let inputFieldValue = inputField.value - if (readonlyField && inputFieldValue || fieldName == "full_name"){ + if (readonlyField && (inputFieldValue || fieldName == "full_name")){ if (fieldName == "full_name"){ let firstName = document.querySelector(`#id_first_name`).value; let middleName = document.querySelector(`#id_middle_name`).value; @@ -1855,6 +1855,8 @@ document.addEventListener('DOMContentLoaded', function() { }else { readonlyField.innerHTML = "Unknown"; } + + inputField.classList.add("text-base") } } } From 876041fe40a5bce462e287ce0af1f75d3c90436b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 14:18:20 -0600 Subject: [PATCH 15/71] Update get-gov.js --- src/registrar/assets/js/get-gov.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 30b80d356..90fb5711e 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1839,12 +1839,12 @@ document.addEventListener('DOMContentLoaded', function() { if (editableFormGroup){ let readonlyField = editableFormGroup.querySelector(".input-with-edit-button__readonly-field") let inputField = document.getElementById(`id_${fieldName}`); - if (!inputField) { + if (!inputField || !readonlyField) { return; } let inputFieldValue = inputField.value - if (readonlyField && (inputFieldValue || fieldName == "full_name")){ + if (inputFieldValue || fieldName == "full_name"){ if (fieldName == "full_name"){ let firstName = document.querySelector(`#id_first_name`).value; let middleName = document.querySelector(`#id_middle_name`).value; @@ -1857,6 +1857,8 @@ document.addEventListener('DOMContentLoaded', function() { } inputField.classList.add("text-base") + }else { + readonlyField.innerHTML = inputFieldValue } } } From 0edc8484735564e456f3008fb1e171e3ef36712e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 14:25:05 -0600 Subject: [PATCH 16/71] Cleanup --- src/registrar/assets/js/get-gov.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 90fb5711e..74f2715b2 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1846,17 +1846,23 @@ document.addEventListener('DOMContentLoaded', function() { let inputFieldValue = inputField.value if (inputFieldValue || fieldName == "full_name"){ if (fieldName == "full_name"){ - let firstName = document.querySelector(`#id_first_name`).value; - let middleName = document.querySelector(`#id_middle_name`).value; - let lastName = document.querySelector(`#id_last_name`).value; - if (firstName && middleName && lastName) { + let firstName = document.querySelector(`#id_first_name`); + let middleName = document.querySelector(`#id_middle_name`); + let lastName = document.querySelector(`#id_last_name`); + if (firstName && lastName) { let values = [firstName.value, middleName.value, lastName.value] + console.log(values) readonlyField.innerHTML = values.join(" "); }else { readonlyField.innerHTML = "Unknown"; } - - inputField.classList.add("text-base") + + // Technically, the full_name field is optional, but we want to display it as required. + // This style is applied to readonly fields (gray text). This just removes it, as + // this is difficult to achieve otherwise by modifying the .readonly property. + if (readonlyField.classList.contains("text-base")) { + readonlyField.classList.remove("text-base") + } }else { readonlyField.innerHTML = inputFieldValue } From ac4b657dbbb9da30a21d5ef7fa47b1df88a7ada0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jul 2024 14:53:01 -0600 Subject: [PATCH 17/71] Add unit test and lint --- src/registrar/forms/user_profile.py | 2 +- src/registrar/tests/test_views.py | 84 +++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/user_profile.py b/src/registrar/forms/user_profile.py index 02bc4e58f..60e67886c 100644 --- a/src/registrar/forms/user_profile.py +++ b/src/registrar/forms/user_profile.py @@ -99,4 +99,4 @@ class FinishSetupProfileForm(UserProfileForm): self.fields["full_name"].initial = full_name # Set full_name as required for styling purposes - self.fields["full_name"].widget.attrs['required'] = 'required' + self.fields["full_name"].widget.attrs["required"] = "required" diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 61bc94a32..f8f4e1775 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -539,6 +539,49 @@ class FinishUserProfileTests(TestWithUser, WebTest): self._set_session_cookie() return page.follow() if follow else page + @less_console_noise_decorator + @override_flag("profile_feature", active=True) + def test_full_name_initial_value(self): + """Test that full_name initial value is empty when first_name or last_name is empty. + This will later be displayed as "unknown" using javascript.""" + self.app.set_user(self.incomplete_regular_user.username) + + # Test when first_name is empty + self.incomplete_regular_user.contact.first_name = "" + self.incomplete_regular_user.contact.last_name = "Doe" + self.incomplete_regular_user.contact.save() + + finish_setup_page = self.app.get(reverse("home")).follow() + form = finish_setup_page.form + self.assertEqual(form["full_name"].value, "") + + # Test when last_name is empty + self.incomplete_regular_user.contact.first_name = "John" + self.incomplete_regular_user.contact.last_name = "" + self.incomplete_regular_user.contact.save() + + finish_setup_page = self.app.get(reverse("home")).follow() + form = finish_setup_page.form + self.assertEqual(form["full_name"].value, "") + + # Test when both first_name and last_name are empty + self.incomplete_regular_user.contact.first_name = "" + self.incomplete_regular_user.contact.last_name = "" + self.incomplete_regular_user.contact.save() + + finish_setup_page = self.app.get(reverse("home")).follow() + form = finish_setup_page.form + self.assertEqual(form["full_name"].value, "") + + # Test when both first_name and last_name are present + self.incomplete_regular_user.contact.first_name = "John" + self.incomplete_regular_user.contact.last_name = "Doe" + self.incomplete_regular_user.contact.save() + + finish_setup_page = self.app.get(reverse("home")).follow() + form = finish_setup_page.form + self.assertEqual(form["full_name"].value, "John Doe") + @less_console_noise_decorator def test_new_user_with_profile_feature_on(self): """Tests that a new user is redirected to the profile setup page when profile_feature is on""" @@ -577,6 +620,47 @@ class FinishUserProfileTests(TestWithUser, WebTest): completed_setup_page = self.app.get(reverse("home")) self.assertContains(completed_setup_page, "Manage your domain") + @less_console_noise_decorator + def test_new_user_with_empty_name_profile_feature_on(self): + """Tests that a new user without a name can still enter this information accordingly""" + self.incomplete_regular_user.contact.first_name = None + self.incomplete_regular_user.contact.last_name = None + self.incomplete_regular_user.save() + self.app.set_user(self.incomplete_regular_user.username) + with override_flag("profile_feature", active=True): + # This will redirect the user to the setup page. + # Follow implicity checks if our redirect is working. + finish_setup_page = self.app.get(reverse("home")).follow() + self._set_session_cookie() + + # Assert that we're on the right page + self.assertContains(finish_setup_page, "Finish setting up your profile") + + finish_setup_page = self._submit_form_webtest(finish_setup_page.form) + + self.assertEqual(finish_setup_page.status_code, 200) + + # We're missing a phone number, so the page should tell us that + self.assertContains(finish_setup_page, "Enter your phone number.") + + # Check for the name of the save button + self.assertContains(finish_setup_page, "contact_setup_save_button") + + # Add a phone number + finish_setup_form = finish_setup_page.form + finish_setup_form["phone"] = "(201) 555-0123" + finish_setup_form["title"] = "CEO" + finish_setup_form["last_name"] = "example" + save_page = self._submit_form_webtest(finish_setup_form, follow=True) + + self.assertEqual(save_page.status_code, 200) + self.assertContains(save_page, "Your profile has been updated.") + + # Try to navigate back to the home page. + # This is the same as clicking the back button. + completed_setup_page = self.app.get(reverse("home")) + self.assertContains(completed_setup_page, "Manage your domain") + @less_console_noise_decorator def test_new_user_goes_to_domain_request_with_profile_feature_on(self): """Tests that a new user is redirected to the domain request page when profile_feature is on""" From 1146d97835cb7fb49581a16718800b7416ff860a Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 3 Jul 2024 16:57:22 -0700 Subject: [PATCH 18/71] Hide the other sections when in rejected state --- src/registrar/templates/domain_request_status.html | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/templates/domain_request_status.html b/src/registrar/templates/domain_request_status.html index ad3dc4069..183a8be81 100644 --- a/src/registrar/templates/domain_request_status.html +++ b/src/registrar/templates/domain_request_status.html @@ -42,10 +42,13 @@

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

+ + {% if DomainRequest.status != 'rejected' %}

{% include "includes/domain_request.html" %}

Withdraw request

+ {% endif %}
From b5d3df6d8579a326797573699fda2f4c91de9a4c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 08:16:58 -0600 Subject: [PATCH 19/71] Update src/registrar/assets/js/get-gov.js --- src/registrar/assets/js/get-gov.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 74f2715b2..7bc19d0d5 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1851,7 +1851,6 @@ document.addEventListener('DOMContentLoaded', function() { let lastName = document.querySelector(`#id_last_name`); if (firstName && lastName) { let values = [firstName.value, middleName.value, lastName.value] - console.log(values) readonlyField.innerHTML = values.join(" "); }else { readonlyField.innerHTML = "Unknown"; From 0641e9ad7255e068a73e3079cfaf6b891b9b1146 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 08:26:32 -0600 Subject: [PATCH 20/71] linting --- src/registrar/admin.py | 5 +++-- src/registrar/models/domain_request.py | 2 +- src/registrar/models/utility/generic_helper.py | 1 - src/registrar/tests/test_admin.py | 8 ++------ 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a1ba6559d..850c385f0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1746,7 +1746,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): obj.action_needed_reason_email = body_text should_save = True - if obj.status == original_obj.status: # If the status hasn't changed, let the base function take care of it return super().save_model(request, obj, form, change) @@ -1968,7 +1967,9 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email_text(domain_request, enum_value, custom_text) + emails[enum_value] = self._get_action_needed_reason_default_email_text( + domain_request, enum_value, custom_text + ) return json.dumps(emails) def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str, custom_text=None): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 387840161..632a16474 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -893,7 +893,7 @@ class DomainRequest(TimeStampedModel): # This is so you can override if you need, or have this taken care of for you. if not email_template_name: email_template_name = f"{self.action_needed_reason}.txt" - + if not email_template_subject_name: email_template_subject_name = f"{self.action_needed_reason}_subject.txt" diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 72dee8204..f9d4303c4 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -2,7 +2,6 @@ import time import logging -import hashlib from urllib.parse import urlparse, urlunparse, urlencode diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f9a107dc3..fecc733e3 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1485,9 +1485,7 @@ class TestDomainRequestAdmin(MockEppLib): domain_request.save() self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_so) - self.assert_email_is_accurate( - "custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL - ) + self.assert_email_is_accurate("custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) def test_save_model_sends_submitted_email(self): @@ -2234,6 +2232,7 @@ class TestDomainRequestAdmin(MockEppLib): self.assertContains(response, "Yes, select ineligible status") def test_readonly_when_restricted_creator(self): + self.maxDiff = None with less_console_noise(): domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) with boto3_mocking.clients.handler_for("sesv2", self.mock_client): @@ -2252,7 +2251,6 @@ class TestDomainRequestAdmin(MockEppLib): "is_election_board", "federal_agency", "status_history", - "action_needed_reason_email", "id", "created_at", "updated_at", @@ -2314,7 +2312,6 @@ class TestDomainRequestAdmin(MockEppLib): "is_election_board", "federal_agency", "status_history", - "action_needed_reason_email", "creator", "about_your_organization", "requested_domain", @@ -2346,7 +2343,6 @@ class TestDomainRequestAdmin(MockEppLib): "is_election_board", "federal_agency", "status_history", - "action_needed_reason_email", ] self.assertEqual(readonly_fields, expected_fields) From 204b907a540586bcdab7e867546e228e70d58294 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 08:30:47 -0600 Subject: [PATCH 21/71] lint --- src/registrar/admin.py | 2 +- src/registrar/tests/test_admin.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 850c385f0..e50b262fb 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1984,7 +1984,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if not custom_text: template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" else: - template_path = f"emails/action_needed_reasons/custom_email.txt" + template_path = "emails/action_needed_reasons/custom_email.txt" template = get_template(template_path) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index fecc733e3..c100477a6 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1464,7 +1464,7 @@ class TestDomainRequestAdmin(MockEppLib): ) self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) - # Test the email sent out for questionable_so + # Test that a custom email is sent out for questionable_so questionable_so = DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_so) self.assert_email_is_accurate( From f18b10d669a8c2b3d9db94bd798dfb20aa9191a8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 08:47:44 -0600 Subject: [PATCH 22/71] Add logic for if just the email is changed --- src/registrar/models/domain_request.py | 7 +++++-- src/registrar/tests/test_admin.py | 27 ++++++++++++++++++++++---- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 632a16474..5740d9504 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -598,6 +598,7 @@ class DomainRequest(TimeStampedModel): def _cache_status_and_action_needed_reason(self): """Maintains a cache of properties so we can avoid a DB call""" self._cached_action_needed_reason = self.action_needed_reason + self._cached_action_needed_reason_email = self.action_needed_reason_email self._cached_status = self.status def __init__(self, *args, **kwargs): @@ -614,7 +615,8 @@ class DomainRequest(TimeStampedModel): # Handle the action needed email. We send one when moving to action_needed, # but we don't send one when we are _already_ in the state and change the reason. - self.sync_action_needed_reason() + if self.status == self.DomainRequestStatus.ACTION_NEEDED and self.action_needed_reason: + self.sync_action_needed_reason() # Update the cached values after saving self._cache_status_and_action_needed_reason() @@ -624,7 +626,8 @@ class DomainRequest(TimeStampedModel): was_already_action_needed = self._cached_status == self.DomainRequestStatus.ACTION_NEEDED reason_exists = self._cached_action_needed_reason is not None and self.action_needed_reason is not None reason_changed = self._cached_action_needed_reason != self.action_needed_reason - if was_already_action_needed and (reason_exists and reason_changed): + reason_email_changed = self._cached_action_needed_reason_email != self.action_needed_reason_email + if was_already_action_needed and (reason_exists and reason_changed or reason_email_changed): # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: self._send_action_needed_reason_email(email_content=self.action_needed_reason_email) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index c100477a6..22bdc8558 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1376,7 +1376,9 @@ class TestDomainRequestAdmin(MockEppLib): self.assertContains(response, "status in [submitted,in review,action needed]", count=1) @less_console_noise_decorator - def transition_state_and_send_email(self, domain_request, status, rejection_reason=None, action_needed_reason=None): + def transition_state_and_send_email( + self, domain_request, status, rejection_reason=None, action_needed_reason=None, action_needed_reason_email=None + ): """Helper method for the email test cases.""" with boto3_mocking.clients.handler_for("sesv2", self.mock_client): @@ -1392,6 +1394,9 @@ class TestDomainRequestAdmin(MockEppLib): if action_needed_reason: domain_request.action_needed_reason = action_needed_reason + if action_needed_reason_email: + domain_request.action_needed_reason_email = action_needed_reason_email + # Use the model admin's save_model method self.admin.save_model(request, domain_request, form=None, change=True) @@ -1481,13 +1486,27 @@ class TestDomainRequestAdmin(MockEppLib): # Tests if an analyst can override existing email content questionable_so = DomainRequest.ActionNeededReasons.QUESTIONABLE_SENIOR_OFFICIAL - domain_request.action_needed_reason_email = "custom email content" - domain_request.save() - self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=questionable_so) + self.transition_state_and_send_email( + domain_request, + action_needed, + action_needed_reason=questionable_so, + action_needed_reason_email="custom email content", + ) self.assert_email_is_accurate("custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) + # Tests if a new email gets sent when just the email is changed + self.transition_state_and_send_email( + domain_request, + action_needed, + action_needed_reason=questionable_so, + action_needed_reason_email="dummy email content", + ) + + self.assert_email_is_accurate("dummy email content", 5, EMAIL, bcc_email_address=BCC_EMAIL) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 6) + def test_save_model_sends_submitted_email(self): """When transitioning to submitted from started or withdrawn on a domain request, an email is sent out. From 173c0b51c0e69a9ca8f04e0becc0b042beec2f3a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 13:55:11 -0600 Subject: [PATCH 23/71] Update src/registrar/assets/js/get-gov.js --- src/registrar/assets/js/get-gov.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 7bc19d0d5..0d450b9e5 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1846,9 +1846,9 @@ document.addEventListener('DOMContentLoaded', function() { let inputFieldValue = inputField.value if (inputFieldValue || fieldName == "full_name"){ if (fieldName == "full_name"){ - let firstName = document.querySelector(`#id_first_name`); - let middleName = document.querySelector(`#id_middle_name`); - let lastName = document.querySelector(`#id_last_name`); + let firstName = document.querySelector("#id_first_name"); + let middleName = document.querySelector("#id_middle_name"); + let lastName = document.querySelector("#id_last_name"); if (firstName && lastName) { let values = [firstName.value, middleName.value, lastName.value] readonlyField.innerHTML = values.join(" "); From 425cfbcb4eae8b5b1e2a4ac2df5691a399ce9886 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 14:13:29 -0600 Subject: [PATCH 24/71] Logic refactor --- src/registrar/admin.py | 57 +++++++++++-------- src/registrar/assets/js/get-gov-admin.js | 6 ++ src/registrar/models/domain_request.py | 21 +------ .../admin/includes/detail_table_fieldset.html | 9 +++ 4 files changed, 48 insertions(+), 45 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e50b262fb..1561b7e1f 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1735,15 +1735,16 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) - if obj.action_needed_reason: - text = self._get_action_needed_reason_default_email_text(obj, obj.action_needed_reason) + if obj.action_needed_reason and obj.action_needed_reason != obj.ActionNeededReasons.OTHER: + text = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) body_text = text.get("email_body_text") - if body_text: - body_text.strip().lstrip("\n") - is_default_email = body_text == obj.action_needed_reason_email - reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - if is_default_email and reason_changed: - obj.action_needed_reason_email = body_text + reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + is_default_email = body_text == obj.action_needed_reason_email + if body_text and is_default_email and reason_changed: + obj.action_needed_reason_email = body_text + should_save = True + elif not obj.action_needed_reason and obj.action_needed_reason_email: + obj.action_needed_reason_email = None should_save = True if obj.status == original_obj.status: @@ -1967,12 +1968,12 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email_text( + emails[enum_value] = self._get_action_needed_reason_default_email( domain_request, enum_value, custom_text ) return json.dumps(emails) - def _get_action_needed_reason_default_email_text(self, domain_request, action_needed_reason: str, custom_text=None): + def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason: str, custom_text=None): """Returns the default email associated with the given action needed reason""" if action_needed_reason is None or action_needed_reason == domain_request.ActionNeededReasons.OTHER: return { @@ -1980,28 +1981,34 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "email_body_text": None, } - # Get the email body - if not custom_text: - template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" - else: - template_path = "emails/action_needed_reasons/custom_email.txt" - - template = get_template(template_path) - - # Get the email subject - template_subject_path = f"emails/action_needed_reasons/{action_needed_reason}_subject.txt" - subject_template = get_template(template_subject_path) - if flag_is_active(None, "profile_feature"): # type: ignore recipient = domain_request.creator else: recipient = domain_request.submitter - # Return the content of the rendered views + # Return the context of the rendered views context = {"domain_request": domain_request, "recipient": recipient} + + # Get the email subject + template_subject_path = f"emails/action_needed_reasons/{action_needed_reason}_subject.txt" + subject_text = get_template(template_subject_path).render(context=context) + + # Get the email body + if not custom_text: + template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" + else: + template_path = "emails/action_needed_reasons/custom_email.txt" + context["custom_email_content"] = custom_text + + email_body_text = get_template(template_path).render(context=context) + + email_body_text_cleaned = None + if email_body_text: + email_body_text_cleaned = email_body_text.strip().lstrip("\n") + return { - "subject_text": subject_template.render(context=context), - "email_body_text": template.render(context=context) if not custom_text else custom_text, + "subject_text": subject_text, + "email_body_text": email_body_text_cleaned, } def process_log_entry(self, log_entry): diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 2a01eb304..3cf20311d 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -531,6 +531,12 @@ function initializeWidgetOnList(list, parentId) { if(actionNeededReasonDropdown && actionNeededEmail) { // Add a change listener to the action needed reason dropdown handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail); + + document.addEventListener('DOMContentLoaded', function() { + if (!actionNeededReasonDropdown.value || actionNeededReasonDropdown.value == "other") { + showNoEmailMessage(actionNeededEmail); + } + }); } function handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail) { diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 5740d9504..25138c54a 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -874,14 +874,10 @@ class DomainRequest(TimeStampedModel): def _send_action_needed_reason_email(self, send_email=True, email_content=None): """Sends out an automatic email for each valid action needed reason provided""" - # Store the filenames of the template and template subject email_template_name: str = "" email_template_subject_name: str = "" - # Check if the current email that we sent out is the same as our defaults. - # If these differ, then that means that we're sending custom content. - default_email = self.get_default_action_needed_reason_email(self.action_needed_reason) - if self.action_needed_reason_email and self.action_needed_reason_email != default_email: + if email_content is not None: email_template_name = "custom_email.txt" # Check for the "type" of action needed reason. @@ -916,21 +912,6 @@ class DomainRequest(TimeStampedModel): wrap_email=True, ) - def get_default_action_needed_reason_email(self, action_needed_reason): - """Returns the default email associated with the given action needed reason""" - if action_needed_reason is None or action_needed_reason == self.ActionNeededReasons.OTHER: - return None - - # Get the email body - template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" - template = get_template(template_path) - - recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter - # Return the content of the rendered views - context = {"domain_request": self, "recipient": recipient} - body_text = template.render(context=context).strip().lstrip("\n") - return body_text - @transition( field="status", source=[ diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 8b8748f80..2aa5c0a60 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -143,6 +143,15 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endwith %} {% endblock field_readonly %} +{% block field_other %} + {% if field.field.name == "action_needed_reason_email" %} + {{ field.field }} +

No email will be sent.

+ {% else %} + {{ field.field }} + {% endif %} +{% endblock field_other %} + {% block after_help_text %} {% if field.field.name == "action_needed_reason_email" %} {% comment %} From 91b9cb43795a61a6083b05a7f9a2ab220ee70466 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 14:29:36 -0600 Subject: [PATCH 25/71] Cleanup --- src/registrar/admin.py | 35 +++++++++++++++++--------- src/registrar/models/domain_request.py | 1 - 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1561b7e1f..7d5708e62 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1735,17 +1735,12 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) + # If the reason is in a state where we can send out an email, + # set the email to a default one if a custom email isn't provided. if obj.action_needed_reason and obj.action_needed_reason != obj.ActionNeededReasons.OTHER: - text = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) - body_text = text.get("email_body_text") - reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - is_default_email = body_text == obj.action_needed_reason_email - if body_text and is_default_email and reason_changed: - obj.action_needed_reason_email = body_text - should_save = True - elif not obj.action_needed_reason and obj.action_needed_reason_email: + obj = self._handle_existing_action_needed_reason_email(obj, original_obj) + else: obj.action_needed_reason_email = None - should_save = True if obj.status == original_obj.status: # If the status hasn't changed, let the base function take care of it @@ -1759,6 +1754,24 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if should_save: return super().save_model(request, obj, form, change) + def _handle_existing_action_needed_reason_email(self, obj, original_obj): + """ + Changes the action_needed_reason to the default email. + This occurs if the email changes and if it is empty. + """ + + # Get the default email + text = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) + body_text = text.get("email_body_text") + + # Set the action_needed_reason_email to the default + reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + is_default_email = body_text == obj.action_needed_reason_email + if body_text and is_default_email and reason_changed: + obj.action_needed_reason_email = body_text + + return obj + def _handle_status_change(self, request, obj, original_obj): """ Checks for various conditions when a status change is triggered. @@ -1968,9 +1981,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email( - domain_request, enum_value, custom_text - ) + emails[enum_value] = self._get_action_needed_reason_default_email(domain_request, enum_value, custom_text) return json.dumps(emails) def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason: str, custom_text=None): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 25138c54a..1d26e0d2f 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1,7 +1,6 @@ from __future__ import annotations from typing import Union import logging -from django.template.loader import get_template from django.apps import apps from django.conf import settings from django.db import models From 72000b4a9b9b51e60d0156b66392b23bac905c06 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jul 2024 14:44:52 -0600 Subject: [PATCH 26/71] Update email.py --- src/registrar/utility/email.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 7f0b997fa..e274893a2 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -46,7 +46,7 @@ def send_templated_email( template = get_template(template_name) email_body = template.render(context=context) - # Do cleanup on the email body. Mostly for emails with custom content. + # Do cleanup on the email body. For emails with custom content. if email_body: email_body.strip().lstrip("\n") From d9f963361f7c3e002ac48b72a3f247b97f6a4c03 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jul 2024 09:27:38 -0600 Subject: [PATCH 27/71] PR suggestion --- src/registrar/tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index f8f4e1775..a610cdf71 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -621,7 +621,7 @@ class FinishUserProfileTests(TestWithUser, WebTest): self.assertContains(completed_setup_page, "Manage your domain") @less_console_noise_decorator - def test_new_user_with_empty_name_profile_feature_on(self): + def test_new_user_with_empty_name_can_add_name(self): """Tests that a new user without a name can still enter this information accordingly""" self.incomplete_regular_user.contact.first_name = None self.incomplete_regular_user.contact.last_name = None From ee517768156c30254b7523f4a013c8ce0759e248 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 9 Jul 2024 14:00:08 -0700 Subject: [PATCH 28/71] Fix show detail section --- src/registrar/admin.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ef7888005..6756db1cf 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1698,8 +1698,34 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): for name, data in fieldsets: fields = data.get("fields", []) fields = tuple(field for field in fields if field not in self.superuser_only_fields) - modified_fieldsets.append((name, {"fields": fields})) + + # Handle Type of organization and its Show details + if name == "Type of organization": + show_details_fields = ( + "federal_type", + "federal_agency", + "tribe_name", + "federally_recognized_tribe", + "state_recognized_tribe", + "about_your_organization", + ) + fields = tuple(field for field in fields if field not in show_details_fields) + modified_fieldsets.append((name, {"fields": fields + show_details_fields})) + # Handle Organization name and mailing address and its Show details + elif name == "Organization name and mailing address": + show_details_address_fields = ( + "address_line1", + "address_line2", + "city", + "zipcode", + "urbanization", + ) + fields = tuple(field for field in fields if field not in show_details_address_fields) + modified_fieldsets.append((name, {"fields": fields + show_details_address_fields})) + else: + modified_fieldsets.append((name, {"fields": fields})) return modified_fieldsets + return fieldsets # Table ordering From c1a083ed734cf3b3b286ece2b527c1628ff1bb42 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:46:01 -0700 Subject: [PATCH 29/71] Add manual deploy workflow --- .../workflows/deploy-branch-to-sandbox.yaml | 90 +++++++++++++++++++ .../test_generate_current_full_report.py | 58 ++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 .github/workflows/deploy-branch-to-sandbox.yaml create mode 100644 src/registrar/management/commands/test_generate_current_full_report.py diff --git a/.github/workflows/deploy-branch-to-sandbox.yaml b/.github/workflows/deploy-branch-to-sandbox.yaml new file mode 100644 index 000000000..a6192a2d7 --- /dev/null +++ b/.github/workflows/deploy-branch-to-sandbox.yaml @@ -0,0 +1,90 @@ +# This workflow + +name: Manual Build and Deploy +run-name: Manually build and deploy branch to sandbox of choice + +on: + workflow_dispatch: + inputs: + environment: + description: 'Environment to deploy' + required: true + default: 'backup' + type: 'choice' + options: + - ab + - backup + - cb + - dk + - es + - gd + - ko + - ky + - nl + - rb + - rh + - rjm + - meoward + - bob + - hotgov + - litterbox + # GitHub Actions has no "good" way yet to dynamically input branches + branch: + description: 'Branch to deploy' + required: true + default: 'main' + type: string + + +jobs: + variables: + runs-on: ubuntu-latest + steps: + - name: Setting global variables + uses: actions/github-script@v6 + id: var + with: + script: | + core.setOutput('environment', '${{ github.head_ref }}'.split("/")[0]); + deploy: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Compile USWDS assets + working-directory: ./src + run: | + docker compose run node npm install && + docker compose run node npx gulp copyAssets && + docker compose run node npx gulp compile + - name: Collect static assets + working-directory: ./src + run: docker compose run app python manage.py collectstatic --no-input + - name: Deploy to cloud.gov sandbox + uses: cloud-gov/cg-cli-tools@main + env: + ENVIRONMENT: ${{ github.event.inputs.environment }} + CF_USERNAME: CF_${{ github.event.inputs.environment }}_USERNAME + CF_PASSWORD: CF_${{ github.event.inputs.environment }}_PASSWORD + with: + cf_username: ${{ secrets[env.CF_USERNAME] }} + cf_password: ${{ secrets[env.CF_PASSWORD] }} + cf_org: cisa-dotgov + cf_space: ${{ env.ENVIRONMENT }} + cf_manifest: ops/manifests/manifest-${{ env.ENVIRONMENT }}.yaml + comment: + runs-on: ubuntu-latest + needs: [deploy] + steps: + - uses: actions/github-script@v6 + env: + ENVIRONMENT: ${{ github.event.inputs.environment }} + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + body: '🥳 Successfully deployed to developer sandbox **[${{ env.ENVIRONMENT }}](https://getgov-${{ env.ENVIRONMENT }}.app.cloud.gov/)**.' + }) + + diff --git a/src/registrar/management/commands/test_generate_current_full_report.py b/src/registrar/management/commands/test_generate_current_full_report.py new file mode 100644 index 000000000..e5552fba1 --- /dev/null +++ b/src/registrar/management/commands/test_generate_current_full_report.py @@ -0,0 +1,58 @@ +"""Generates current-full.csv and current-federal.csv then uploads them to the desired URL.""" + +import logging +import os + +from django.core.management import BaseCommand +from registrar.utility import csv_export +from registrar.utility.s3_bucket import S3ClientHelper + + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = ( + "Generates and uploads a current-full.csv file to our S3 bucket " "which is based off of all existing Domains." + ) + + def add_arguments(self, parser): + """Add our two filename arguments.""" + parser.add_argument("--directory", default="migrationdata", help="Desired directory") + parser.add_argument( + "--checkpath", + default=True, + help="Flag that determines if we do a check for os.path.exists. Used for test cases", + ) + + def handle(self, **options): + """Grabs the directory then creates current-full.csv in that directory""" + file_name = "current-full.csv" + # Ensures a slash is added + directory = os.path.join(options.get("directory"), "") + check_path = options.get("checkpath") + + logger.info("Generating report...") + try: + self.generate_current_full_report(directory, file_name, check_path) + except Exception as err: + # TODO - #1317: Notify operations when auto report generation fails + raise err + else: + logger.info(f"Success! Created {file_name}") + + def generate_current_full_report(self, directory, file_name, check_path): + """Creates a current-full.csv file under the specified directory, + then uploads it to a AWS S3 bucket""" + s3_client = S3ClientHelper() + file_path = os.path.join(directory, file_name) + + # Generate a file locally for upload + with open(file_path, "w") as file: + csv_export.export_data_full_to_csv(file) + + if check_path and not os.path.exists(file_path): + raise FileNotFoundError(f"Could not find newly created file at '{file_path}'") + + # Upload this generated file for our S3 instance + print(file_path, file_name) From 7fe23a98ea2d4d1ffbe4d78ad0e4554acdf92ae6 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:48:49 -0700 Subject: [PATCH 30/71] Remove unkonwn file --- .../test_generate_current_full_report.py | 58 ------------------- 1 file changed, 58 deletions(-) delete mode 100644 src/registrar/management/commands/test_generate_current_full_report.py diff --git a/src/registrar/management/commands/test_generate_current_full_report.py b/src/registrar/management/commands/test_generate_current_full_report.py deleted file mode 100644 index e5552fba1..000000000 --- a/src/registrar/management/commands/test_generate_current_full_report.py +++ /dev/null @@ -1,58 +0,0 @@ -"""Generates current-full.csv and current-federal.csv then uploads them to the desired URL.""" - -import logging -import os - -from django.core.management import BaseCommand -from registrar.utility import csv_export -from registrar.utility.s3_bucket import S3ClientHelper - - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - help = ( - "Generates and uploads a current-full.csv file to our S3 bucket " "which is based off of all existing Domains." - ) - - def add_arguments(self, parser): - """Add our two filename arguments.""" - parser.add_argument("--directory", default="migrationdata", help="Desired directory") - parser.add_argument( - "--checkpath", - default=True, - help="Flag that determines if we do a check for os.path.exists. Used for test cases", - ) - - def handle(self, **options): - """Grabs the directory then creates current-full.csv in that directory""" - file_name = "current-full.csv" - # Ensures a slash is added - directory = os.path.join(options.get("directory"), "") - check_path = options.get("checkpath") - - logger.info("Generating report...") - try: - self.generate_current_full_report(directory, file_name, check_path) - except Exception as err: - # TODO - #1317: Notify operations when auto report generation fails - raise err - else: - logger.info(f"Success! Created {file_name}") - - def generate_current_full_report(self, directory, file_name, check_path): - """Creates a current-full.csv file under the specified directory, - then uploads it to a AWS S3 bucket""" - s3_client = S3ClientHelper() - file_path = os.path.join(directory, file_name) - - # Generate a file locally for upload - with open(file_path, "w") as file: - csv_export.export_data_full_to_csv(file) - - if check_path and not os.path.exists(file_path): - raise FileNotFoundError(f"Could not find newly created file at '{file_path}'") - - # Upload this generated file for our S3 instance - print(file_path, file_name) From 91f5a28716ed7d2ebb8554aa694555e68944561b Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:51:26 -0700 Subject: [PATCH 31/71] Add workflow run on pr to test --- .github/workflows/deploy-branch-to-sandbox.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/deploy-branch-to-sandbox.yaml b/.github/workflows/deploy-branch-to-sandbox.yaml index a6192a2d7..f88d81527 100644 --- a/.github/workflows/deploy-branch-to-sandbox.yaml +++ b/.github/workflows/deploy-branch-to-sandbox.yaml @@ -4,6 +4,8 @@ name: Manual Build and Deploy run-name: Manually build and deploy branch to sandbox of choice on: + # delete on pull_request after testing + pull_request: workflow_dispatch: inputs: environment: From e91630839554394be918cb352151c5694f5f8fd9 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 11 Jul 2024 15:56:07 -0700 Subject: [PATCH 32/71] Fix show details for analyst --- src/registrar/admin.py | 40 +--------------------------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 6756db1cf..426437941 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1636,7 +1636,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): ), ] - # Readonly fields for analysts and superusers readonly_fields = ( "other_contacts", "current_websites", @@ -1647,7 +1646,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "action_needed_reason_email", ) - # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ "creator", "about_your_organization", @@ -1680,52 +1678,16 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "sub_organization", ] - # DEVELOPER's NOTE: - # Normally, to exclude a field from an Admin form, we could simply utilize - # Django's "exclude" feature. However, it causes a "missing key" error if we - # go that route for this particular form. The error gets thrown by our - # custom fieldset.html code and is due to the fact that "exclude" removes - # fields from base_fields but not fieldsets. Rather than reworking our - # custom frontend, it seems more straightforward (and easier to read) to simply - # modify the fieldsets list so that it excludes any fields we want to remove - # based on permissions (eg. superuser_only_fields) or other conditions. def get_fieldsets(self, request, obj=None): fieldsets = super().get_fieldsets(request, obj) - # Create a modified version of fieldsets to exclude certain fields if not request.user.has_perm("registrar.full_access_permission"): modified_fieldsets = [] for name, data in fieldsets: fields = data.get("fields", []) fields = tuple(field for field in fields if field not in self.superuser_only_fields) - - # Handle Type of organization and its Show details - if name == "Type of organization": - show_details_fields = ( - "federal_type", - "federal_agency", - "tribe_name", - "federally_recognized_tribe", - "state_recognized_tribe", - "about_your_organization", - ) - fields = tuple(field for field in fields if field not in show_details_fields) - modified_fieldsets.append((name, {"fields": fields + show_details_fields})) - # Handle Organization name and mailing address and its Show details - elif name == "Organization name and mailing address": - show_details_address_fields = ( - "address_line1", - "address_line2", - "city", - "zipcode", - "urbanization", - ) - fields = tuple(field for field in fields if field not in show_details_address_fields) - modified_fieldsets.append((name, {"fields": fields + show_details_address_fields})) - else: - modified_fieldsets.append((name, {"fields": fields})) + modified_fieldsets.append((name, {**data, "fields": fields})) return modified_fieldsets - return fieldsets # Table ordering From f10581c46ac6853929403e2882ac25d864a08d03 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 11 Jul 2024 15:58:44 -0700 Subject: [PATCH 33/71] Add comments back in --- src/registrar/admin.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 426437941..2890bd3cd 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1043,19 +1043,6 @@ class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().changelist_view(request, extra_context=extra_context) -class SeniorOfficialAdmin(ListHeaderAdmin): - """Custom Senior Official Admin class.""" - - # NOTE: these are just placeholders. Not part of ACs (haven't been defined yet). Update in future tickets. - search_fields = ["first_name", "last_name", "email"] - search_help_text = "Search by first name, last name or email." - list_display = ["first_name", "last_name", "email"] - - # this ordering effects the ordering of results - # in autocomplete_fields for Senior Official - ordering = ["first_name", "last_name"] - - class WebsiteResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" @@ -1636,6 +1623,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): ), ] + # Readonly fields for analysts and superusers readonly_fields = ( "other_contacts", "current_websites", @@ -1646,6 +1634,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "action_needed_reason_email", ) + # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ "creator", "about_your_organization", @@ -1678,9 +1667,19 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): "sub_organization", ] + # DEVELOPER's NOTE: + # Normally, to exclude a field from an Admin form, we could simply utilize + # Django's "exclude" feature. However, it causes a "missing key" error if we + # go that route for this particular form. The error gets thrown by our + # custom fieldset.html code and is due to the fact that "exclude" removes + # fields from base_fields but not fieldsets. Rather than reworking our + # custom frontend, it seems more straightforward (and easier to read) to simply + # modify the fieldsets list so that it excludes any fields we want to remove + # based on permissions (eg. superuser_only_fields) or other conditions. def get_fieldsets(self, request, obj=None): fieldsets = super().get_fieldsets(request, obj) + # Create a modified version of fieldsets to exclude certain fields if not request.user.has_perm("registrar.full_access_permission"): modified_fieldsets = [] for name, data in fieldsets: @@ -2800,7 +2799,6 @@ admin.site.register(models.VerifiedByStaff, VerifiedByStaffAdmin) admin.site.register(models.Portfolio, PortfolioAdmin) admin.site.register(models.DomainGroup, DomainGroupAdmin) admin.site.register(models.Suborganization, SuborganizationAdmin) -admin.site.register(models.SeniorOfficial, SeniorOfficialAdmin) # Register our custom waffle implementations admin.site.register(models.WaffleFlag, WaffleFlagAdmin) From 18c5e34dad28ecb1cd3a1071fc2f211f82983188 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:59:12 -0700 Subject: [PATCH 34/71] Update workflow comment description --- .github/workflows/deploy-branch-to-sandbox.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deploy-branch-to-sandbox.yaml b/.github/workflows/deploy-branch-to-sandbox.yaml index f88d81527..664b9581b 100644 --- a/.github/workflows/deploy-branch-to-sandbox.yaml +++ b/.github/workflows/deploy-branch-to-sandbox.yaml @@ -1,4 +1,4 @@ -# This workflow +# Manually deploy a branch of choice to an environment of choice. name: Manual Build and Deploy run-name: Manually build and deploy branch to sandbox of choice From 66851f191ad29101d47036b296494a04c3289ce5 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 11 Jul 2024 16:00:13 -0700 Subject: [PATCH 35/71] Accidentally removed senioradmin smh --- src/registrar/admin.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 2890bd3cd..6f35d6ba9 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1043,6 +1043,19 @@ class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().changelist_view(request, extra_context=extra_context) +class SeniorOfficialAdmin(ListHeaderAdmin): + """Custom Senior Official Admin class.""" + + # NOTE: these are just placeholders. Not part of ACs (haven't been defined yet). Update in future tickets. + search_fields = ["first_name", "last_name", "email"] + search_help_text = "Search by first name, last name or email." + list_display = ["first_name", "last_name", "email"] + + # this ordering effects the ordering of results + # in autocomplete_fields for Senior Official + ordering = ["first_name", "last_name"] + + class WebsiteResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" @@ -2798,6 +2811,7 @@ admin.site.register(models.TransitionDomain, TransitionDomainAdmin) admin.site.register(models.VerifiedByStaff, VerifiedByStaffAdmin) admin.site.register(models.Portfolio, PortfolioAdmin) admin.site.register(models.DomainGroup, DomainGroupAdmin) +admin.site.register(models.SeniorOfficial, SeniorOfficialAdmin) admin.site.register(models.Suborganization, SuborganizationAdmin) # Register our custom waffle implementations From 7921d959edb2a8902875e21b87104709d71d128c Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 11 Jul 2024 16:01:21 -0700 Subject: [PATCH 36/71] Rearrange --- src/registrar/admin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 6f35d6ba9..f15587f49 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2811,8 +2811,9 @@ admin.site.register(models.TransitionDomain, TransitionDomainAdmin) admin.site.register(models.VerifiedByStaff, VerifiedByStaffAdmin) admin.site.register(models.Portfolio, PortfolioAdmin) admin.site.register(models.DomainGroup, DomainGroupAdmin) -admin.site.register(models.SeniorOfficial, SeniorOfficialAdmin) admin.site.register(models.Suborganization, SuborganizationAdmin) +admin.site.register(models.SeniorOfficial, SeniorOfficialAdmin) + # Register our custom waffle implementations admin.site.register(models.WaffleFlag, WaffleFlagAdmin) From d7c09e277f32c73971405f15aaa11b75ac88fd2d Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Thu, 11 Jul 2024 16:02:00 -0700 Subject: [PATCH 37/71] Remove on pull request workflow prompt --- .github/workflows/deploy-branch-to-sandbox.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/deploy-branch-to-sandbox.yaml b/.github/workflows/deploy-branch-to-sandbox.yaml index 664b9581b..14a3d8ef8 100644 --- a/.github/workflows/deploy-branch-to-sandbox.yaml +++ b/.github/workflows/deploy-branch-to-sandbox.yaml @@ -4,8 +4,6 @@ name: Manual Build and Deploy run-name: Manually build and deploy branch to sandbox of choice on: - # delete on pull_request after testing - pull_request: workflow_dispatch: inputs: environment: From 95f315503583d033b5e5cc5c8ef784bc46332f84 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 11 Jul 2024 16:02:44 -0700 Subject: [PATCH 38/71] Fix linting --- src/registrar/admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index f15587f49..7a09255b0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2814,7 +2814,6 @@ admin.site.register(models.DomainGroup, DomainGroupAdmin) admin.site.register(models.Suborganization, SuborganizationAdmin) admin.site.register(models.SeniorOfficial, SeniorOfficialAdmin) - # Register our custom waffle implementations admin.site.register(models.WaffleFlag, WaffleFlagAdmin) From c428e8a1dd5afe77e2f11be71a6f57989afd8c32 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 08:30:15 -0600 Subject: [PATCH 39/71] Logic to show different messages depending on the state --- src/registrar/assets/js/get-gov-admin.js | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 3cf20311d..eb71d0dd1 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -528,13 +528,19 @@ function initializeWidgetOnList(list, parentId) { (function () { let actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); + let noEmailMessage = document.getElementById("no-email-message"); + const emptyReasonText = "---------" + const noEmailText = "No email will be sent." if(actionNeededReasonDropdown && actionNeededEmail) { // Add a change listener to the action needed reason dropdown handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail); document.addEventListener('DOMContentLoaded', function() { - if (!actionNeededReasonDropdown.value || actionNeededReasonDropdown.value == "other") { - showNoEmailMessage(actionNeededEmail); + if (!actionNeededReasonDropdown.value) { + noEmailMessage.innerHTML = emptyReasonText + showNoEmailMessage(actionNeededEmail, noEmailMessage); + }else if (actionNeededReasonDropdown.value == "other") { + noEmailMessage.innerHTML = noEmailText } }); } @@ -547,8 +553,11 @@ function initializeWidgetOnList(list, parentId) { // You also cannot save the model in this state. // This flow occurs if you switch back to the empty picker state. if(!reason) { - showNoEmailMessage(actionNeededEmail); + noEmailMessage.innerHTML = emptyReasonText + showNoEmailMessage(actionNeededEmail, noEmailMessage); return; + }else if (reason === "other") { + noEmailMessage.innerHTML = noEmailText } let actionNeededEmails = JSON.parse(document.getElementById('action-needed-emails-data').textContent) @@ -557,27 +566,25 @@ function initializeWidgetOnList(list, parentId) { let emailBody = emailData.email_body_text if (emailBody) { actionNeededEmail.value = emailBody - showActionNeededEmail(actionNeededEmail); + showActionNeededEmail(actionNeededEmail, noEmailMessage); }else { - showNoEmailMessage(actionNeededEmail); + showNoEmailMessage(actionNeededEmail, noEmailMessage); } }else { - showNoEmailMessage(actionNeededEmail); + showNoEmailMessage(actionNeededEmail, noEmailMessage); } }); } // Show the text field. Hide the "no email" message. - function showActionNeededEmail(actionNeededEmail){ - let noEmailMessage = document.getElementById("no-email-message"); + function showActionNeededEmail(actionNeededEmail, noEmailMessage){ showElement(actionNeededEmail); hideElement(noEmailMessage); } // Hide the text field. Show the "no email" message. - function showNoEmailMessage(actionNeededEmail) { - let noEmailMessage = document.getElementById("no-email-message"); + function showNoEmailMessage(actionNeededEmail, noEmailMessage) { hideElement(actionNeededEmail); showElement(noEmailMessage); } From ef84ff271c9b40a49b9710de409e6f7a217bb984 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 09:40:37 -0600 Subject: [PATCH 40/71] Update src/registrar/assets/js/get-gov-admin.js Co-authored-by: Rachid Mrad <107004823+rachidatecs@users.noreply.github.com> --- src/registrar/assets/js/get-gov-admin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index eb71d0dd1..f6aee5dbe 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -539,7 +539,7 @@ function initializeWidgetOnList(list, parentId) { if (!actionNeededReasonDropdown.value) { noEmailMessage.innerHTML = emptyReasonText showNoEmailMessage(actionNeededEmail, noEmailMessage); - }else if (actionNeededReasonDropdown.value == "other") { + } else if (actionNeededReasonDropdown.value == "other") { noEmailMessage.innerHTML = noEmailText } }); From 9f5dd551934647d80a2471e86c07cfee37274111 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 09:42:50 -0600 Subject: [PATCH 41/71] Update src/registrar/templates/django/admin/includes/detail_table_fieldset.html Co-authored-by: Rachid Mrad <107004823+rachidatecs@users.noreply.github.com> --- .../django/admin/includes/detail_table_fieldset.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 2aa5c0a60..fffe15ff3 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -144,6 +144,13 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endblock field_readonly %} {% block field_other %} + {% if field.field.name == "action_needed_reason_email" %} + {{ field.field }} +

No email will be sent.

+ {% else %} + {{ field.field }} + {% endif %} +{% endblock field_other %} {% if field.field.name == "action_needed_reason_email" %} {{ field.field }}

No email will be sent.

From 58730501a039ed3d59f4274a90ca9367ddc188f8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 09:49:39 -0600 Subject: [PATCH 42/71] Update detail_table_fieldset.html --- .../django/admin/includes/detail_table_fieldset.html | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index fffe15ff3..2aa5c0a60 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -144,13 +144,6 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endblock field_readonly %} {% block field_other %} - {% if field.field.name == "action_needed_reason_email" %} - {{ field.field }} -

No email will be sent.

- {% else %} - {{ field.field }} - {% endif %} -{% endblock field_other %} {% if field.field.name == "action_needed_reason_email" %} {{ field.field }}

No email will be sent.

From ed653e4562a099d805f9b1e35c37aa631983d782 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 11:11:23 -0600 Subject: [PATCH 43/71] add pr suggestion for spacing --- .../django/admin/includes/detail_table_fieldset.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 2aa5c0a60..f0ff7be3f 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -145,10 +145,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block field_other %} {% if field.field.name == "action_needed_reason_email" %} - {{ field.field }} -

No email will be sent.

+ {{ field.field }} +

No email will be sent.

{% else %} - {{ field.field }} + {{ field.field }} {% endif %} {% endblock field_other %} From 025a128baef4c7a3fc08113df5a529d3ddd491c8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 11:54:27 -0600 Subject: [PATCH 44/71] Refactor --- src/registrar/admin.py | 68 +++++++++++------------- src/registrar/assets/js/get-gov-admin.js | 67 ++++++++++------------- 2 files changed, 58 insertions(+), 77 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7d5708e62..8f00ab519 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1731,46 +1731,37 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if not change: return super().save_model(request, obj, form, change) - # == Handle non-status changes == # # Get the original domain request from the database. original_obj = models.DomainRequest.objects.get(pk=obj.pk) - # If the reason is in a state where we can send out an email, - # set the email to a default one if a custom email isn't provided. - if obj.action_needed_reason and obj.action_needed_reason != obj.ActionNeededReasons.OTHER: - obj = self._handle_existing_action_needed_reason_email(obj, original_obj) - else: + # == Handle action_needed_reason == # + # Store the email that was sent out if one was sent and it isn't saved to a variable yet + if not obj.action_needed_reason or obj.action_needed_reason == obj.ActionNeededReasons.OTHER: + # Reset the action needed email to none if we don't send out an email. obj.action_needed_reason_email = None + else: + # We send out an email -- store which one we send out. + # Set the email to a default one if a custom email isn't provided. + default_email = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) + body_text = default_email.get("email_body_text") + # Set the action_needed_reason_email to the default + reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + is_default_email = body_text == obj.action_needed_reason_email + if body_text and is_default_email and reason_changed: + obj.action_needed_reason_email = body_text + + # == Handle status == # if obj.status == original_obj.status: # If the status hasn't changed, let the base function take care of it return super().save_model(request, obj, form, change) + else: + # Run some checks on the current object for invalid status changes + obj, should_save = self._handle_status_change(request, obj, original_obj) - # == Handle status changes == # - # Run some checks on the current object for invalid status changes - obj, should_save = self._handle_status_change(request, obj, original_obj) - - # We should only save if we don't display any errors in the steps above. - if should_save: - return super().save_model(request, obj, form, change) - - def _handle_existing_action_needed_reason_email(self, obj, original_obj): - """ - Changes the action_needed_reason to the default email. - This occurs if the email changes and if it is empty. - """ - - # Get the default email - text = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) - body_text = text.get("email_body_text") - - # Set the action_needed_reason_email to the default - reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - is_default_email = body_text == obj.action_needed_reason_email - if body_text and is_default_email and reason_changed: - obj.action_needed_reason_email = body_text - - return obj + # We should only save if we don't display any errors in the steps above. + if should_save: + return super().save_model(request, obj, form, change) def _handle_status_change(self, request, obj, original_obj): """ @@ -1976,17 +1967,18 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): for this particular domain request.""" emails = {} for action_needed_reason in domain_request.ActionNeededReasons: - enum_value = action_needed_reason.value - custom_text = None - if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: - custom_text = domain_request.action_needed_reason_email + if action_needed_reason != DomainRequest.ActionNeededReasons.OTHER: + enum_value = action_needed_reason.value + custom_text = None + if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: + custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email(domain_request, enum_value, custom_text) + emails[enum_value] = self._get_action_needed_reason_default_email(domain_request, enum_value, custom_text) return json.dumps(emails) - def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason: str, custom_text=None): + def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason, custom_text=None): """Returns the default email associated with the given action needed reason""" - if action_needed_reason is None or action_needed_reason == domain_request.ActionNeededReasons.OTHER: + if not action_needed_reason: return { "subject_text": None, "email_body_text": None, diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index f6aee5dbe..e41431ba3 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -528,65 +528,54 @@ function initializeWidgetOnList(list, parentId) { (function () { let actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); + let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent); let noEmailMessage = document.getElementById("no-email-message"); const emptyReasonText = "---------" const noEmailText = "No email will be sent." - if(actionNeededReasonDropdown && actionNeededEmail) { + if(actionNeededReasonDropdown && actionNeededEmail && actionNeededEmailData) { // Add a change listener to the action needed reason dropdown - handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail); + handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail, actionNeededEmailData); document.addEventListener('DOMContentLoaded', function() { - if (!actionNeededReasonDropdown.value) { - noEmailMessage.innerHTML = emptyReasonText - showNoEmailMessage(actionNeededEmail, noEmailMessage); - } else if (actionNeededReasonDropdown.value == "other") { - noEmailMessage.innerHTML = noEmailText + let reason = actionNeededReasonDropdown.value; + noEmailMessage.innerHTML = reason ? noEmailText : emptyReasonText; + if (reason && reason != "other") { + // Show the email + showElement(actionNeededEmail); + hideElement(noEmailMessage); + } else { + // Show the no email message + hideElement(actionNeededEmail); + showElement(noEmailMessage); } }); } - function handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail) { + function handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail, actionNeededEmailData) { actionNeededReasonDropdown.addEventListener("change", function() { let reason = actionNeededReasonDropdown.value; + let actionNeededEmailsJson = JSON.parse(actionNeededEmailData) - // If a reason isn't specified, no email will be sent. - // You also cannot save the model in this state. - // This flow occurs if you switch back to the empty picker state. - if(!reason) { - noEmailMessage.innerHTML = emptyReasonText - showNoEmailMessage(actionNeededEmail, noEmailMessage); - return; - }else if (reason === "other") { - noEmailMessage.innerHTML = noEmailText - } - - let actionNeededEmails = JSON.parse(document.getElementById('action-needed-emails-data').textContent) - let emailData = actionNeededEmails[reason]; - if (emailData) { + // Show the "no email will be sent" text only if a reason is actually selected. + noEmailMessage.innerHTML = reason ? noEmailText : emptyReasonText; + if (reason && reason in actionNeededEmailsJson) { + let emailData = actionNeededEmailsJson[reason]; let emailBody = emailData.email_body_text if (emailBody) { + // Show the email actionNeededEmail.value = emailBody - showActionNeededEmail(actionNeededEmail, noEmailMessage); + showElement(actionNeededEmail); + hideElement(noEmailMessage); }else { - showNoEmailMessage(actionNeededEmail, noEmailMessage); + // Show the no email message + hideElement(actionNeededEmail); + showElement(noEmailMessage); } }else { - showNoEmailMessage(actionNeededEmail, noEmailMessage); + // Show the no email message + hideElement(actionNeededEmail); + showElement(noEmailMessage); } - }); } - - // Show the text field. Hide the "no email" message. - function showActionNeededEmail(actionNeededEmail, noEmailMessage){ - showElement(actionNeededEmail); - hideElement(noEmailMessage); - } - - // Hide the text field. Show the "no email" message. - function showNoEmailMessage(actionNeededEmail, noEmailMessage) { - hideElement(actionNeededEmail); - showElement(noEmailMessage); - } - })(); From 6d63c9f868dc4881f0055115f62a5d73b27715e9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 11:59:36 -0600 Subject: [PATCH 45/71] Minor edits to not use kwargs and lint --- src/registrar/admin.py | 4 +++- src/registrar/models/domain_request.py | 25 ++++++++++++++++--------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8f00ab519..870fb900e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1973,7 +1973,9 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: custom_text = domain_request.action_needed_reason_email - emails[enum_value] = self._get_action_needed_reason_default_email(domain_request, enum_value, custom_text) + emails[enum_value] = self._get_action_needed_reason_default_email( + domain_request, enum_value, custom_text + ) return json.dumps(emails) def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason, custom_text=None): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 1d26e0d2f..f2f43be33 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -612,8 +612,8 @@ class DomainRequest(TimeStampedModel): super().save(*args, **kwargs) - # Handle the action needed email. We send one when moving to action_needed, - # but we don't send one when we are _already_ in the state and change the reason. + # Handle the action needed email. + # An email is sent out when action_needed_reason is changed or added. if self.status == self.DomainRequestStatus.ACTION_NEEDED and self.action_needed_reason: self.sync_action_needed_reason() @@ -683,7 +683,15 @@ class DomainRequest(TimeStampedModel): logger.error(f"Can't query an approved domain while attempting {called_from}") def _send_status_update_email( - self, new_status, email_template, email_template_subject, bcc_address="", context=None, **kwargs + self, + new_status, + email_template, + email_template_subject, + bcc_address="", + context=None, + send_email=True, + wrap_email=False, + custom_email_content=None, ): """Send a status update email to the creator. @@ -694,7 +702,11 @@ class DomainRequest(TimeStampedModel): If the waffle flag "profile_feature" is active, then this email will be sent to the domain request creator rather than the submitter - kwargs: + Optional args: + bcc_address: str -> the address to bcc to + + context: dict -> The context sent to the template + send_email: bool -> Used to bypass the send_templated_email function, in the event we just want to log that an email would have been sent, rather than actually sending one. @@ -704,11 +716,6 @@ class DomainRequest(TimeStampedModel): custom_email_content: str -> Renders an email with the content of this string as its body text. """ - # Email config options - wrap_email = kwargs.get("wrap_email", False) - send_email = kwargs.get("send_email", True) - custom_email_content = kwargs.get("custom_email_content", None) - recipient = self.creator if flag_is_active(None, "profile_feature") else self.submitter if recipient is None or recipient.email is None: logger.warning( From 8ff6d526525d20bfb9873c8b364e21afb7acd0f4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:12:04 -0600 Subject: [PATCH 46/71] Simplify logic further --- src/registrar/models/domain_request.py | 48 +++++++------------------- 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index f2f43be33..c8ec643e6 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -614,7 +614,7 @@ class DomainRequest(TimeStampedModel): # Handle the action needed email. # An email is sent out when action_needed_reason is changed or added. - if self.status == self.DomainRequestStatus.ACTION_NEEDED and self.action_needed_reason: + if self.action_needed_reason and self.status == self.DomainRequestStatus.ACTION_NEEDED: self.sync_action_needed_reason() # Update the cached values after saving @@ -625,8 +625,7 @@ class DomainRequest(TimeStampedModel): was_already_action_needed = self._cached_status == self.DomainRequestStatus.ACTION_NEEDED reason_exists = self._cached_action_needed_reason is not None and self.action_needed_reason is not None reason_changed = self._cached_action_needed_reason != self.action_needed_reason - reason_email_changed = self._cached_action_needed_reason_email != self.action_needed_reason_email - if was_already_action_needed and (reason_exists and reason_changed or reason_email_changed): + if was_already_action_needed and reason_exists and reason_changed: # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: self._send_action_needed_reason_email(email_content=self.action_needed_reason_email) @@ -880,43 +879,22 @@ class DomainRequest(TimeStampedModel): def _send_action_needed_reason_email(self, send_email=True, email_content=None): """Sends out an automatic email for each valid action needed reason provided""" - email_template_name: str = "" - email_template_subject_name: str = "" - - if email_content is not None: - email_template_name = "custom_email.txt" - - # Check for the "type" of action needed reason. - can_send_email = True - match self.action_needed_reason: - # Add to this match if you need to pass in a custom filename for these templates. - case self.ActionNeededReasons.OTHER, _: - # Unknown and other are default cases - do nothing - can_send_email = False - - # Assumes that the template name matches the action needed reason if nothing is specified. - # This is so you can override if you need, or have this taken care of for you. - if not email_template_name: - email_template_name = f"{self.action_needed_reason}.txt" - - if not email_template_subject_name: - email_template_subject_name = f"{self.action_needed_reason}_subject.txt" + email_template_name = "custom_email.txt" + email_template_subject_name = f"{self.action_needed_reason}_subject.txt" bcc_address = "" if settings.IS_PRODUCTION: bcc_address = settings.DEFAULT_FROM_EMAIL - # If we can, try to send out an email as long as send_email=True - if can_send_email: - self._send_status_update_email( - new_status="action needed", - email_template=f"emails/action_needed_reasons/{email_template_name}", - email_template_subject=f"emails/action_needed_reasons/{email_template_subject_name}", - send_email=send_email, - bcc_address=bcc_address, - custom_email_content=email_content, - wrap_email=True, - ) + self._send_status_update_email( + new_status="action needed", + email_template=f"emails/action_needed_reasons/{email_template_name}", + email_template_subject=f"emails/action_needed_reasons/{email_template_subject_name}", + send_email=send_email, + bcc_address=bcc_address, + custom_email_content=email_content, + wrap_email=True, + ) @transition( field="status", From 41835d275fd7bb701f215d08128660189b71b09d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:44:14 -0600 Subject: [PATCH 47/71] Further simplification --- src/registrar/admin.py | 58 +++++++----------------- src/registrar/assets/js/get-gov-admin.js | 2 +- 2 files changed, 17 insertions(+), 43 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 870fb900e..fd529b652 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1736,20 +1736,14 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # == Handle action_needed_reason == # # Store the email that was sent out if one was sent and it isn't saved to a variable yet - if not obj.action_needed_reason or obj.action_needed_reason == obj.ActionNeededReasons.OTHER: - # Reset the action needed email to none if we don't send out an email. - obj.action_needed_reason_email = None - else: - # We send out an email -- store which one we send out. - # Set the email to a default one if a custom email isn't provided. - default_email = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) - body_text = default_email.get("email_body_text") - - # Set the action_needed_reason_email to the default + default_email = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) + if default_email: + # Set the action_needed_reason_email to the default. + # Since this check occurs after save, if the user enters a value then + # we won't update. reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - is_default_email = body_text == obj.action_needed_reason_email - if body_text and is_default_email and reason_changed: - obj.action_needed_reason_email = body_text + if reason_changed and default_email == obj.action_needed_reason_email: + obj.action_needed_reason_email = default_email # == Handle status == # if obj.status == original_obj.status: @@ -1967,24 +1961,16 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): for this particular domain request.""" emails = {} for action_needed_reason in domain_request.ActionNeededReasons: - if action_needed_reason != DomainRequest.ActionNeededReasons.OTHER: - enum_value = action_needed_reason.value - custom_text = None - if domain_request.action_needed_reason == enum_value and domain_request.action_needed_reason_email: - custom_text = domain_request.action_needed_reason_email - - emails[enum_value] = self._get_action_needed_reason_default_email( - domain_request, enum_value, custom_text - ) + # Map the action_needed_reason to its default email + emails[action_needed_reason.value] = self._get_action_needed_reason_default_email( + domain_request, action_needed_reason.value + ) return json.dumps(emails) - def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason, custom_text=None): + def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason): """Returns the default email associated with the given action needed reason""" - if not action_needed_reason: - return { - "subject_text": None, - "email_body_text": None, - } + if not action_needed_reason or action_needed_reason == DomainRequest.ActionNeededReasons.OTHER: + return None if flag_is_active(None, "profile_feature"): # type: ignore recipient = domain_request.creator @@ -1994,27 +1980,15 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Return the context of the rendered views context = {"domain_request": domain_request, "recipient": recipient} - # Get the email subject - template_subject_path = f"emails/action_needed_reasons/{action_needed_reason}_subject.txt" - subject_text = get_template(template_subject_path).render(context=context) - # Get the email body - if not custom_text: - template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" - else: - template_path = "emails/action_needed_reasons/custom_email.txt" - context["custom_email_content"] = custom_text + template_path = f"emails/action_needed_reasons/{action_needed_reason}.txt" email_body_text = get_template(template_path).render(context=context) - email_body_text_cleaned = None if email_body_text: email_body_text_cleaned = email_body_text.strip().lstrip("\n") - return { - "subject_text": subject_text, - "email_body_text": email_body_text_cleaned, - } + return email_body_text_cleaned def process_log_entry(self, log_entry): """Process a log entry and return filtered entry dictionary if applicable.""" diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index e41431ba3..b5df21538 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -528,7 +528,7 @@ function initializeWidgetOnList(list, parentId) { (function () { let actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); - let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent); + let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; let noEmailMessage = document.getElementById("no-email-message"); const emptyReasonText = "---------" const noEmailText = "No email will be sent." From f6e82c7a989d2b27e5fab6a16ad01ed16d228fc2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:53:26 -0600 Subject: [PATCH 48/71] tests --- src/registrar/tests/test_admin.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 22bdc8558..34d64190f 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1496,7 +1496,8 @@ class TestDomainRequestAdmin(MockEppLib): self.assert_email_is_accurate("custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) - # Tests if a new email gets sent when just the email is changed + # Tests if a new email gets sent when just the email is changed. + # An email should NOT be sent out if we just modify the email content. self.transition_state_and_send_email( domain_request, action_needed, @@ -1504,7 +1505,22 @@ class TestDomainRequestAdmin(MockEppLib): action_needed_reason_email="dummy email content", ) - self.assert_email_is_accurate("dummy email content", 5, EMAIL, bcc_email_address=BCC_EMAIL) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) + + # Set the request back to in review + domain_request.in_review() + + # no email was sent, so no email should be stored + self.assertEqual(domain_request.action_needed_reason_email, None) + + # Try sending another email when changing states AND including content + self.transition_state_and_send_email( + domain_request, + action_needed, + action_needed_reason=questionable_so, + action_needed_reason_email="custom content when starting anew", + ) + self.assert_email_is_accurate("custom content when starting anew", 5, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 6) def test_save_model_sends_submitted_email(self): From bac9dc94aa52aa1439b7658d45917b329e1a71f8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:59:28 -0600 Subject: [PATCH 49/71] Cleanup --- src/registrar/assets/js/get-gov-admin.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index b5df21538..e5aa495f5 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -530,7 +530,7 @@ function initializeWidgetOnList(list, parentId) { let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; let noEmailMessage = document.getElementById("no-email-message"); - const emptyReasonText = "---------" + const emptyReasonText = "-" const noEmailText = "No email will be sent." if(actionNeededReasonDropdown && actionNeededEmail && actionNeededEmailData) { // Add a change listener to the action needed reason dropdown @@ -558,9 +558,10 @@ function initializeWidgetOnList(list, parentId) { // Show the "no email will be sent" text only if a reason is actually selected. noEmailMessage.innerHTML = reason ? noEmailText : emptyReasonText; + console.log(`reaso: ${reason} vs in ${reason in actionNeededEmailsJson}`) + console.log(noEmailMessage) if (reason && reason in actionNeededEmailsJson) { - let emailData = actionNeededEmailsJson[reason]; - let emailBody = emailData.email_body_text + let emailBody = actionNeededEmailsJson[reason]; if (emailBody) { // Show the email actionNeededEmail.value = emailBody From 58605f11e5d7debd2ec700e7e7339b3bde9b3894 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:11:42 -0600 Subject: [PATCH 50/71] Readonly on send --- src/registrar/admin.py | 13 ++++- src/registrar/assets/js/get-gov-admin.js | 50 +++++++++++++------ .../admin/includes/detail_table_fieldset.html | 1 + 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index fd529b652..42d2872a2 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1742,8 +1742,11 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Since this check occurs after save, if the user enters a value then # we won't update. reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - if reason_changed and default_email == obj.action_needed_reason_email: - obj.action_needed_reason_email = default_email + if reason_changed: + request.session["action_needed_email_sent"] = True + logger.info("added session object") + if default_email == obj.action_needed_reason_email: + obj.action_needed_reason_email = default_email # == Handle status == # if obj.status == original_obj.status: @@ -1953,6 +1956,12 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): extra_context["action_needed_reason_emails"] = self.get_all_action_needed_reason_emails_as_json(obj) extra_context["has_profile_feature_flag"] = flag_is_active(request, "profile_feature") + # Denote if an action needed email was sent or not + email_sent = request.session.get("action_needed_email_sent", False) + extra_context["action_needed_email_sent"] = email_sent + if email_sent: + email_sent = request.session["action_needed_email_sent"] = False + # Call the superclass method with updated extra_context return super().change_view(request, object_id, form_url, extra_context) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index e5aa495f5..07722fdfc 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -36,6 +36,15 @@ function openInNewTab(el, removeAttribute = false){ } }; +// Adds or removes a boolean from our session +function addOrRemoveSessionBoolean(name, add){ + if (add) { + sessionStorage.setItem(name, "true"); + }else { + sessionStorage.removeItem(name); + } +} + // <<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>> // Event handlers. @@ -418,15 +427,6 @@ function initializeWidgetOnList(list, parentId) { object.classList.add("display-none"); } } - - // Adds or removes a boolean from our session - function addOrRemoveSessionBoolean(name, add){ - if (add) { - sessionStorage.setItem(name, "true"); - }else { - sessionStorage.removeItem(name); - } - } })(); /** An IIFE for toggling the submit bar on domain request forms @@ -529,9 +529,12 @@ function initializeWidgetOnList(list, parentId) { let actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); let actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; - let noEmailMessage = document.getElementById("no-email-message"); - const emptyReasonText = "-" - const noEmailText = "No email will be sent." + let noEmailMessage = document.querySelector("#no-email-message"); + const oldDropdownValue = actionNeededReasonDropdown ? actionNeededReasonDropdown.value : null; + const oldEmailValue = actionNeededEmailData ? actionNeededEmailData.value : null; + const emptyReasonText = "-"; + const noEmailText = "No email will be sent."; + const domainRequestId = actionNeededReasonDropdown ? document.querySelector("#domain_request_id").value : null if(actionNeededReasonDropdown && actionNeededEmail && actionNeededEmailData) { // Add a change listener to the action needed reason dropdown handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail, actionNeededEmailData); @@ -548,6 +551,16 @@ function initializeWidgetOnList(list, parentId) { hideElement(actionNeededEmail); showElement(noEmailMessage); } + + let emailWasSent = document.getElementById("action-needed-email-sent") + console.log(`email ${emailWasSent.value} vs session ${sessionStorage.getItem("actionNeededEmailSent")} vs id ${domainRequestId}`) + if (emailWasSent && emailWasSent.value) { + // add the session object + if (sessionStorage.getItem(`actionNeededEmailSent-${domainRequestId}`) === null) { + sessionStorage.setItem(`actionNeededEmailSent-${domainRequestId}`, domainRequestId); + } + actionNeededEmail.readOnly = true + } }); } @@ -558,8 +571,6 @@ function initializeWidgetOnList(list, parentId) { // Show the "no email will be sent" text only if a reason is actually selected. noEmailMessage.innerHTML = reason ? noEmailText : emptyReasonText; - console.log(`reaso: ${reason} vs in ${reason in actionNeededEmailsJson}`) - console.log(noEmailMessage) if (reason && reason in actionNeededEmailsJson) { let emailBody = actionNeededEmailsJson[reason]; if (emailBody) { @@ -567,6 +578,17 @@ function initializeWidgetOnList(list, parentId) { actionNeededEmail.value = emailBody showElement(actionNeededEmail); hideElement(noEmailMessage); + + // Reset the session object on change since change refreshes the email content. + // Only do this if we change the action needed reason, or if we: + // change the reason => modify email content => change back to old reason. + if (oldDropdownValue != actionNeededReasonDropdown.value || oldEmailValue != actionNeededEmail.value) { + let emailSent = sessionStorage.getItem(`actionNeededEmailSent-${domainRequestId}`) + if (emailSent !== null){ + sessionStorage.removeItem(`actionNeededEmailSent-${domainRequestId}`); + } + actionNeededEmail.readOnly = false; + } }else { // Show the no email message hideElement(actionNeededEmail); diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index f0ff7be3f..d477b55a7 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -147,6 +147,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% if field.field.name == "action_needed_reason_email" %} {{ field.field }}

No email will be sent.

+ {% else %} {{ field.field }} {% endif %} From 2bb2af5089ee2c19484400239866626e405eaff0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 15 Jul 2024 08:07:28 -0600 Subject: [PATCH 51/71] Readonly view --- src/registrar/assets/js/get-gov-admin.js | 21 +++++++++++++++++-- .../admin/includes/detail_table_fieldset.html | 20 ++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 07722fdfc..fbbea7003 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -559,7 +559,7 @@ function initializeWidgetOnList(list, parentId) { if (sessionStorage.getItem(`actionNeededEmailSent-${domainRequestId}`) === null) { sessionStorage.setItem(`actionNeededEmailSent-${domainRequestId}`, domainRequestId); } - actionNeededEmail.readOnly = true + hideReadonly(actionNeededEmail.parentElement) } }); } @@ -587,7 +587,7 @@ function initializeWidgetOnList(list, parentId) { if (emailSent !== null){ sessionStorage.removeItem(`actionNeededEmailSent-${domainRequestId}`); } - actionNeededEmail.readOnly = false; + showReadonly(actionNeededEmail.parentElement) } }else { // Show the no email message @@ -601,4 +601,21 @@ function initializeWidgetOnList(list, parentId) { } }); } + + function showReadonly(actionNeededEmailParent) { + let readonlyView = document.querySelector("#action-needed-reason-email-readonly") + if (readonlyView) { + hideElement(readonlyView) + showElement(actionNeededEmailParent) + } + } + + function hideReadonly(actionNeededEmailParent) { + let readonlyView = document.querySelector("#action-needed-reason-email-readonly") + if (readonlyView) { + showElement(readonlyView) + hideElement(actionNeededEmailParent) + } + } + })(); diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index d477b55a7..a23cd8f55 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -145,9 +145,29 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block field_other %} {% if field.field.name == "action_needed_reason_email" %} + + +
{{ field.field }}

No email will be sent.

+
{% else %} {{ field.field }} {% endif %} From 646e37570800ee77404e7114f7f8bd0eda30614d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 15 Jul 2024 09:49:46 -0600 Subject: [PATCH 52/71] Fix bugs --- src/registrar/admin.py | 18 ++++----- src/registrar/assets/js/get-gov-admin.js | 50 ++++++++++++++++-------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 42d2872a2..e15228534 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1735,18 +1735,18 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): original_obj = models.DomainRequest.objects.get(pk=obj.pk) # == Handle action_needed_reason == # - # Store the email that was sent out if one was sent and it isn't saved to a variable yet + default_email = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) - if default_email: + reason_changed = obj.action_needed_reason != original_obj.action_needed_reason + if reason_changed: + # Track that we sent out an email + request.session["action_needed_email_sent"] = True + # Set the action_needed_reason_email to the default. # Since this check occurs after save, if the user enters a value then # we won't update. - reason_changed = obj.action_needed_reason != original_obj.action_needed_reason - if reason_changed: - request.session["action_needed_email_sent"] = True - logger.info("added session object") - if default_email == obj.action_needed_reason_email: - obj.action_needed_reason_email = default_email + if default_email and default_email == obj.action_needed_reason_email: + obj.action_needed_reason_email = default_email # == Handle status == # if obj.status == original_obj.status: @@ -1960,7 +1960,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): email_sent = request.session.get("action_needed_email_sent", False) extra_context["action_needed_email_sent"] = email_sent if email_sent: - email_sent = request.session["action_needed_email_sent"] = False + request.session["action_needed_email_sent"] = False # Call the superclass method with updated extra_context return super().change_view(request, object_id, form_url, extra_context) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index fbbea7003..503055a89 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -535,10 +535,11 @@ function initializeWidgetOnList(list, parentId) { const emptyReasonText = "-"; const noEmailText = "No email will be sent."; const domainRequestId = actionNeededReasonDropdown ? document.querySelector("#domain_request_id").value : null - if(actionNeededReasonDropdown && actionNeededEmail && actionNeededEmailData) { + const emailSentSessionVariableName = `actionNeededEmailSent-${domainRequestId}`; + + if(actionNeededReasonDropdown && actionNeededEmail && actionNeededEmailData && domainRequestId) { // Add a change listener to the action needed reason dropdown handleChangeActionNeededEmail(actionNeededReasonDropdown, actionNeededEmail, actionNeededEmailData); - document.addEventListener('DOMContentLoaded', function() { let reason = actionNeededReasonDropdown.value; noEmailMessage.innerHTML = reason ? noEmailText : emptyReasonText; @@ -552,14 +553,21 @@ function initializeWidgetOnList(list, parentId) { showElement(noEmailMessage); } + let emailWasSent = document.getElementById("action-needed-email-sent") - console.log(`email ${emailWasSent.value} vs session ${sessionStorage.getItem("actionNeededEmailSent")} vs id ${domainRequestId}`) - if (emailWasSent && emailWasSent.value) { - // add the session object - if (sessionStorage.getItem(`actionNeededEmailSent-${domainRequestId}`) === null) { - sessionStorage.setItem(`actionNeededEmailSent-${domainRequestId}`, domainRequestId); - } + if (emailWasSent && emailWasSent.value === "True") { + // An email was sent out - store that information in a session variable + addOrRemoveSessionBoolean(emailSentSessionVariableName, add=true) + } + + if (sessionStorage.getItem(emailSentSessionVariableName) !== null) { + // Show the readonly field, hide the editable field + showReadonly(actionNeededEmail.parentElement) + console.log("adding data") + }else { + // No email was sent out -- show the editable field hideReadonly(actionNeededEmail.parentElement) + console.log("removing data") } }); } @@ -582,13 +590,22 @@ function initializeWidgetOnList(list, parentId) { // Reset the session object on change since change refreshes the email content. // Only do this if we change the action needed reason, or if we: // change the reason => modify email content => change back to old reason. - if (oldDropdownValue != actionNeededReasonDropdown.value || oldEmailValue != actionNeededEmail.value) { - let emailSent = sessionStorage.getItem(`actionNeededEmailSent-${domainRequestId}`) + if (oldDropdownValue !== actionNeededReasonDropdown.value || oldEmailValue !== actionNeededEmail.value) { + let emailSent = sessionStorage.getItem(emailSentSessionVariableName) if (emailSent !== null){ - sessionStorage.removeItem(`actionNeededEmailSent-${domainRequestId}`); + console.log("removing data") + addOrRemoveSessionBoolean(emailSentSessionVariableName, add=false) } - showReadonly(actionNeededEmail.parentElement) } + + if (sessionStorage.getItem(emailSentSessionVariableName) !== null) { + // Show the readonly field, hide the editable field + showReadonly(actionNeededEmail.parentElement) + }else { + // No email was sent out -- show the editable field + hideReadonly(actionNeededEmail.parentElement) + } + }else { // Show the no email message hideElement(actionNeededEmail); @@ -605,17 +622,16 @@ function initializeWidgetOnList(list, parentId) { function showReadonly(actionNeededEmailParent) { let readonlyView = document.querySelector("#action-needed-reason-email-readonly") if (readonlyView) { - hideElement(readonlyView) - showElement(actionNeededEmailParent) + showElement(readonlyView) + hideElement(actionNeededEmailParent) } } function hideReadonly(actionNeededEmailParent) { let readonlyView = document.querySelector("#action-needed-reason-email-readonly") if (readonlyView) { - showElement(readonlyView) - hideElement(actionNeededEmailParent) + hideElement(readonlyView) + showElement(actionNeededEmailParent) } } - })(); From 7364db4e247ece3f7fafab94bfd62579677f84e2 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 15 Jul 2024 09:00:44 -0700 Subject: [PATCH 53/71] Add fix for Domain as well --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7a09255b0..90f393a6e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1383,7 +1383,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): for name, data in fieldsets: fields = data.get("fields", []) fields = tuple(field for field in fields if field not in DomainInformationAdmin.superuser_only_fields) - modified_fieldsets.append((name, {"fields": fields})) + modified_fieldsets.append((name, {**data, "fields": fields})) return modified_fieldsets return fieldsets From b68fe5558449f6898ee8c2978a38f2b768087318 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 15 Jul 2024 11:30:38 -0600 Subject: [PATCH 54/71] Further refinement need to fix unit test --- src/registrar/admin.py | 26 ++++++++++++++++-------- src/registrar/assets/js/get-gov-admin.js | 3 --- src/registrar/models/domain_request.py | 2 +- src/registrar/tests/test_admin.py | 8 +++++--- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e15228534..5c6b480f2 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1736,16 +1736,21 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # == Handle action_needed_reason == # - default_email = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) reason_changed = obj.action_needed_reason != original_obj.action_needed_reason if reason_changed: - # Track that we sent out an email + # Track the fact that we sent out an email request.session["action_needed_email_sent"] = True - # Set the action_needed_reason_email to the default. - # Since this check occurs after save, if the user enters a value then - # we won't update. - if default_email and default_email == obj.action_needed_reason_email: + # Set the action_needed_reason_email to the default if nothing exists. + # Since this check occurs after save, if the user enters a value then we won't update. + + default_email = self._get_action_needed_reason_default_email(obj, obj.action_needed_reason) + if obj.action_needed_reason_email: + emails = self.get_all_action_needed_reason_emails(obj) + is_custom_email = obj.action_needed_reason_email not in emails.values() + if not is_custom_email: + obj.action_needed_reason_email = default_email + else: obj.action_needed_reason_email = default_email # == Handle status == # @@ -1953,7 +1958,8 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Initialize extra_context and add filtered entries extra_context = extra_context or {} extra_context["filtered_audit_log_entries"] = filtered_audit_log_entries - extra_context["action_needed_reason_emails"] = self.get_all_action_needed_reason_emails_as_json(obj) + emails = self.get_all_action_needed_reason_emails(obj) + extra_context["action_needed_reason_emails"] = json.dumps(emails) extra_context["has_profile_feature_flag"] = flag_is_active(request, "profile_feature") # Denote if an action needed email was sent or not @@ -1965,16 +1971,18 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Call the superclass method with updated extra_context return super().change_view(request, object_id, form_url, extra_context) - def get_all_action_needed_reason_emails_as_json(self, domain_request): + def get_all_action_needed_reason_emails(self, domain_request): """Returns a json dictionary of every action needed reason and its associated email for this particular domain request.""" + emails = {} for action_needed_reason in domain_request.ActionNeededReasons: # Map the action_needed_reason to its default email emails[action_needed_reason.value] = self._get_action_needed_reason_default_email( domain_request, action_needed_reason.value ) - return json.dumps(emails) + + return emails def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason): """Returns the default email associated with the given action needed reason""" diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 503055a89..bfee12e79 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -563,11 +563,9 @@ function initializeWidgetOnList(list, parentId) { if (sessionStorage.getItem(emailSentSessionVariableName) !== null) { // Show the readonly field, hide the editable field showReadonly(actionNeededEmail.parentElement) - console.log("adding data") }else { // No email was sent out -- show the editable field hideReadonly(actionNeededEmail.parentElement) - console.log("removing data") } }); } @@ -593,7 +591,6 @@ function initializeWidgetOnList(list, parentId) { if (oldDropdownValue !== actionNeededReasonDropdown.value || oldEmailValue !== actionNeededEmail.value) { let emailSent = sessionStorage.getItem(emailSentSessionVariableName) if (emailSent !== null){ - console.log("removing data") addOrRemoveSessionBoolean(emailSentSessionVariableName, add=false) } } diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index c8ec643e6..c7609c8c9 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -597,7 +597,6 @@ class DomainRequest(TimeStampedModel): def _cache_status_and_action_needed_reason(self): """Maintains a cache of properties so we can avoid a DB call""" self._cached_action_needed_reason = self.action_needed_reason - self._cached_action_needed_reason_email = self.action_needed_reason_email self._cached_status = self.status def __init__(self, *args, **kwargs): @@ -625,6 +624,7 @@ class DomainRequest(TimeStampedModel): was_already_action_needed = self._cached_status == self.DomainRequestStatus.ACTION_NEEDED reason_exists = self._cached_action_needed_reason is not None and self.action_needed_reason is not None reason_changed = self._cached_action_needed_reason != self.action_needed_reason + print(f"was_already_action_needed {was_already_action_needed} reason_exists {reason_exists} and {reason_changed}") if was_already_action_needed and reason_exists and reason_changed: # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 34d64190f..4500546f8 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1385,6 +1385,9 @@ class TestDomainRequestAdmin(MockEppLib): # Create a mock request request = self.factory.post("/admin/registrar/domainrequest/{}/change/".format(domain_request.pk)) + # Create a fake session to hook to + request.session = {} + # Modify the domain request's properties domain_request.status = status @@ -1450,6 +1453,7 @@ class TestDomainRequestAdmin(MockEppLib): # Test the email sent out for already_has_domains already_has_domains = DomainRequest.ActionNeededReasons.ALREADY_HAS_DOMAINS self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=already_has_domains) + self.assert_email_is_accurate("ORGANIZATION ALREADY HAS A .GOV DOMAIN", 0, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 1) @@ -1493,6 +1497,7 @@ class TestDomainRequestAdmin(MockEppLib): action_needed_reason_email="custom email content", ) + domain_request.refresh_from_db() self.assert_email_is_accurate("custom email content", 4, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 5) @@ -1510,9 +1515,6 @@ class TestDomainRequestAdmin(MockEppLib): # Set the request back to in review domain_request.in_review() - # no email was sent, so no email should be stored - self.assertEqual(domain_request.action_needed_reason_email, None) - # Try sending another email when changing states AND including content self.transition_state_and_send_email( domain_request, From c2d8c0eadb5ee49efc3e8759d4ea43850ee6005c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 15 Jul 2024 11:42:05 -0600 Subject: [PATCH 55/71] fix test --- src/registrar/models/domain_request.py | 1 - src/registrar/tests/test_admin.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index c7609c8c9..aac356e38 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -624,7 +624,6 @@ class DomainRequest(TimeStampedModel): was_already_action_needed = self._cached_status == self.DomainRequestStatus.ACTION_NEEDED reason_exists = self._cached_action_needed_reason is not None and self.action_needed_reason is not None reason_changed = self._cached_action_needed_reason != self.action_needed_reason - print(f"was_already_action_needed {was_already_action_needed} reason_exists {reason_exists} and {reason_changed}") if was_already_action_needed and reason_exists and reason_changed: # We don't send emails out in state "other" if self.action_needed_reason != self.ActionNeededReasons.OTHER: diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 4500546f8..3ee5eb4f9 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1519,7 +1519,7 @@ class TestDomainRequestAdmin(MockEppLib): self.transition_state_and_send_email( domain_request, action_needed, - action_needed_reason=questionable_so, + action_needed_reason=eligibility_unclear, action_needed_reason_email="custom content when starting anew", ) self.assert_email_is_accurate("custom content when starting anew", 5, EMAIL, bcc_email_address=BCC_EMAIL) From ea1e8aa5101b93a132526c7d65cb7bb22e1879ca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 15 Jul 2024 11:52:16 -0600 Subject: [PATCH 56/71] lint --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5c6b480f2..efb662859 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1981,7 +1981,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): emails[action_needed_reason.value] = self._get_action_needed_reason_default_email( domain_request, action_needed_reason.value ) - + return emails def _get_action_needed_reason_default_email(self, domain_request, action_needed_reason): From 4aa816cfdf7955f3dae7a06b5439346c4dc02ca0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 15 Jul 2024 12:14:07 -0600 Subject: [PATCH 57/71] Update test_admin.py --- src/registrar/tests/test_admin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 3ee5eb4f9..957fb5509 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2449,6 +2449,8 @@ class TestDomainRequestAdmin(MockEppLib): request = self.factory.post("/admin/registrar/domainrequest/{}/change/".format(domain_request.pk)) request.user = self.superuser + request.session = {} + # Define a custom implementation for is_active def custom_is_active(self): return domain_is_active # Override to return True From bd55995e1c4f297673951cf7f2d9c42ecf7db3dd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 15 Jul 2024 12:29:12 -0600 Subject: [PATCH 58/71] Update detail_table_fieldset.html --- .../templates/django/admin/includes/detail_table_fieldset.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index a23cd8f55..5f451766f 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -145,6 +145,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block field_other %} {% if field.field.name == "action_needed_reason_email" %} +

No email will be sent.

-
{{ field.field }} From f4439925b5b59d2ec958d2bfdec98a32adf01744 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 15 Jul 2024 14:59:11 -0600 Subject: [PATCH 61/71] Remove functions for ease of reading --- src/registrar/assets/js/get-gov-admin.js | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 4fd7c4e6c..28ce93a19 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -531,30 +531,21 @@ function initializeWidgetOnList(list, parentId) { var actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); var readonlyView = document.querySelector("#action-needed-reason-email-readonly"); + let emailWasSent = document.getElementById("action-needed-email-sent"); let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; - const oldDropdownValue = actionNeededReasonDropdown ? actionNeededReasonDropdown.value : null; - const oldEmailValue = actionNeededEmailData ? actionNeededEmailData.value : null; + let actionNeededEmailsJson = JSON.parse(actionNeededEmailData); const domainRequestId = actionNeededReasonDropdown ? document.querySelector("#domain_request_id").value : null const emailSentSessionVariableName = `actionNeededEmailSent-${domainRequestId}`; - const emptyReasonText = "-"; - const noEmailText = "No email will be sent."; + const oldDropdownValue = actionNeededReasonDropdown ? actionNeededReasonDropdown.value : null; + const oldEmailValue = actionNeededEmailData ? actionNeededEmailData.value : null; if(actionNeededReasonDropdown && actionNeededEmail && domainRequestId) { // Add a change listener to dom load - handleDomLoadActionNeededEmail(); - - // Add a change listener to the action needed reason dropdown - let actionNeededEmailsJson = JSON.parse(actionNeededEmailData) - handleChangeActionNeededEmail(actionNeededEmailsJson); - } - - function handleDomLoadActionNeededEmail() { document.addEventListener('DOMContentLoaded', function() { let reason = actionNeededReasonDropdown.value; // Handle the session boolean (to enable/disable editing) - let emailWasSent = document.getElementById("action-needed-email-sent"); if (emailWasSent && emailWasSent.value === "True") { // An email was sent out - store that information in a session variable addOrRemoveSessionBoolean(emailSentSessionVariableName, add=true); @@ -563,9 +554,8 @@ function initializeWidgetOnList(list, parentId) { // Show an editable email field or a readonly one updateActionNeededEmailDisplay(reason) }); - } - function handleChangeActionNeededEmail(actionNeededEmailsJson) { + // Add a change listener to the action needed reason dropdown actionNeededReasonDropdown.addEventListener("change", function() { let reason = actionNeededReasonDropdown.value; let emailBody = reason in actionNeededEmailsJson ? actionNeededEmailsJson[reason] : null; @@ -592,13 +582,13 @@ function initializeWidgetOnList(list, parentId) { // Likewise, if we've sent this email before, we should just display the content. function updateActionNeededEmailDisplay(reason) { let emailHasBeenSentBefore = sessionStorage.getItem(emailSentSessionVariableName) !== null; - let collapseableDiv = readonlyView.querySelector('.collapse--dgsimple'); + let collapseableDiv = readonlyView.querySelector(".collapse--dgsimple"); if ((reason && reason != "other") && !emailHasBeenSentBefore) { showElement(actionNeededEmail.parentElement) hideElement(readonlyView) } else { if (!reason || reason === "other") { - collapseableDiv.innerHTML = reason ? noEmailText : emptyReasonText; + collapseableDiv.innerHTML = reason ? "No email will be sent." : "-"; } hideElement(actionNeededEmail.parentElement) showElement(readonlyView) From 28a4b3a0fb4aeb98af11b022e315acec3861281f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 16 Jul 2024 12:14:03 -0600 Subject: [PATCH 62/71] Add missing override on user --- src/registrar/models/user.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 87b7799d3..c542335ec 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -161,10 +161,13 @@ class User(AbstractUser): """Return count of ineligible requests""" return self.domain_requests_created.filter(status=DomainRequest.DomainRequestStatus.INELIGIBLE).count() - def get_formatted_name(self): + def get_formatted_name(self, return_unknown_when_none=True): """Returns the contact's name in Western order.""" names = [n for n in [self.first_name, self.middle_name, self.last_name] if n] - return " ".join(names) if names else "Unknown" + if names: + return " ".join(names) + else: + return "Unknown" if return_unknown_when_none else None def has_contact_info(self): return bool(self.title or self.email or self.phone) From 74d3baae4440ab7c8eb06a78964a11ee9508d53e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 16 Jul 2024 12:27:00 -0600 Subject: [PATCH 63/71] Fix tests that broke after merge Contact was deleted so this test is no longer applicable --- src/registrar/tests/test_views.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index b02cf8201..f9e20005d 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -546,36 +546,36 @@ class FinishUserProfileTests(TestWithUser, WebTest): self.app.set_user(self.incomplete_regular_user.username) # Test when first_name is empty - self.incomplete_regular_user.contact.first_name = "" - self.incomplete_regular_user.contact.last_name = "Doe" - self.incomplete_regular_user.contact.save() + self.incomplete_regular_user.first_name = "" + self.incomplete_regular_user.last_name = "Doe" + self.incomplete_regular_user.save() finish_setup_page = self.app.get(reverse("home")).follow() form = finish_setup_page.form self.assertEqual(form["full_name"].value, "") # Test when last_name is empty - self.incomplete_regular_user.contact.first_name = "John" - self.incomplete_regular_user.contact.last_name = "" - self.incomplete_regular_user.contact.save() + self.incomplete_regular_user.first_name = "John" + self.incomplete_regular_user.last_name = "" + self.incomplete_regular_user.save() finish_setup_page = self.app.get(reverse("home")).follow() form = finish_setup_page.form self.assertEqual(form["full_name"].value, "") # Test when both first_name and last_name are empty - self.incomplete_regular_user.contact.first_name = "" - self.incomplete_regular_user.contact.last_name = "" - self.incomplete_regular_user.contact.save() + self.incomplete_regular_user.first_name = "" + self.incomplete_regular_user.last_name = "" + self.incomplete_regular_user.save() finish_setup_page = self.app.get(reverse("home")).follow() form = finish_setup_page.form self.assertEqual(form["full_name"].value, "") # Test when both first_name and last_name are present - self.incomplete_regular_user.contact.first_name = "John" - self.incomplete_regular_user.contact.last_name = "Doe" - self.incomplete_regular_user.contact.save() + self.incomplete_regular_user.first_name = "John" + self.incomplete_regular_user.last_name = "Doe" + self.incomplete_regular_user.save() finish_setup_page = self.app.get(reverse("home")).follow() form = finish_setup_page.form @@ -622,8 +622,8 @@ class FinishUserProfileTests(TestWithUser, WebTest): @less_console_noise_decorator def test_new_user_with_empty_name_can_add_name(self): """Tests that a new user without a name can still enter this information accordingly""" - self.incomplete_regular_user.contact.first_name = None - self.incomplete_regular_user.contact.last_name = None + self.incomplete_regular_user.first_name = "" + self.incomplete_regular_user.last_name = "" self.incomplete_regular_user.save() self.app.set_user(self.incomplete_regular_user.username) with override_flag("profile_feature", active=True): @@ -647,6 +647,8 @@ class FinishUserProfileTests(TestWithUser, WebTest): # Add a phone number finish_setup_form = finish_setup_page.form + finish_setup_form["first_name"] = "test" + finish_setup_form["last_name"] = "test2" finish_setup_form["phone"] = "(201) 555-0123" finish_setup_form["title"] = "CEO" finish_setup_form["last_name"] = "example" From 5532aa15161d14f6ce374ec8cdd6d44ade9dda2b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 16 Jul 2024 12:31:58 -0600 Subject: [PATCH 64/71] fix test --- src/registrar/tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index f9e20005d..e5de9d49d 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -643,7 +643,7 @@ class FinishUserProfileTests(TestWithUser, WebTest): self.assertContains(finish_setup_page, "Enter your phone number.") # Check for the name of the save button - self.assertContains(finish_setup_page, "contact_setup_save_button") + self.assertContains(finish_setup_page, "user_setup_save_button") # Add a phone number finish_setup_form = finish_setup_page.form From f0f8bbf26aa6461c8ba9a96174e5926628007195 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 16 Jul 2024 12:44:04 -0600 Subject: [PATCH 65/71] add decorator --- src/registrar/tests/test_admin.py | 121 +++++++++++++++--------------- 1 file changed, 60 insertions(+), 61 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index dc76b44cd..ac49b1bee 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2288,72 +2288,71 @@ class TestDomainRequestAdmin(MockEppLib): self.assertContains(response, "When a domain request is in ineligible status") self.assertContains(response, "Yes, select ineligible status") + @less_console_noise_decorator def test_readonly_when_restricted_creator(self): - self.maxDiff = None - with less_console_noise(): - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - domain_request.creator.status = User.RESTRICTED - domain_request.creator.save() + domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): + domain_request.creator.status = User.RESTRICTED + domain_request.creator.save() - request = self.factory.get("/") - request.user = self.superuser + request = self.factory.get("/") + request.user = self.superuser - readonly_fields = self.admin.get_readonly_fields(request, domain_request) + readonly_fields = self.admin.get_readonly_fields(request, domain_request) - expected_fields = [ - "other_contacts", - "current_websites", - "alternative_domains", - "is_election_board", - "federal_agency", - "status_history", - "id", - "created_at", - "updated_at", - "status", - "rejection_reason", - "action_needed_reason", - "action_needed_reason_email", - "federal_agency", - "portfolio", - "sub_organization", - "creator", - "investigator", - "generic_org_type", - "is_election_board", - "organization_type", - "federally_recognized_tribe", - "state_recognized_tribe", - "tribe_name", - "federal_type", - "organization_name", - "address_line1", - "address_line2", - "city", - "state_territory", - "zipcode", - "urbanization", - "about_your_organization", - "senior_official", - "approved_domain", - "requested_domain", - "submitter", - "purpose", - "no_other_contacts_rationale", - "anything_else", - "has_anything_else_text", - "cisa_representative_email", - "cisa_representative_first_name", - "cisa_representative_last_name", - "has_cisa_representative", - "is_policy_acknowledged", - "submission_date", - "notes", - "alternative_domains", - ] + expected_fields = [ + "other_contacts", + "current_websites", + "alternative_domains", + "is_election_board", + "federal_agency", + "status_history", + "id", + "created_at", + "updated_at", + "status", + "rejection_reason", + "action_needed_reason", + "action_needed_reason_email", + "federal_agency", + "portfolio", + "sub_organization", + "creator", + "investigator", + "generic_org_type", + "is_election_board", + "organization_type", + "federally_recognized_tribe", + "state_recognized_tribe", + "tribe_name", + "federal_type", + "organization_name", + "address_line1", + "address_line2", + "city", + "state_territory", + "zipcode", + "urbanization", + "about_your_organization", + "senior_official", + "approved_domain", + "requested_domain", + "submitter", + "purpose", + "no_other_contacts_rationale", + "anything_else", + "has_anything_else_text", + "cisa_representative_email", + "cisa_representative_first_name", + "cisa_representative_last_name", + "has_cisa_representative", + "is_policy_acknowledged", + "submission_date", + "notes", + "alternative_domains", + ] - self.assertEqual(readonly_fields, expected_fields) + self.assertEqual(readonly_fields, expected_fields) def test_readonly_fields_for_analyst(self): with less_console_noise(): From 0e138bbf0a4f1ec3c9dd9f9a625440a927a662ed Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 16 Jul 2024 12:59:22 -0600 Subject: [PATCH 66/71] Fix edge case --- src/registrar/assets/js/get-gov-admin.js | 8 ++++++++ .../django/admin/includes/detail_table_fieldset.html | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 28ce93a19..0f7219913 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -583,12 +583,20 @@ function initializeWidgetOnList(list, parentId) { function updateActionNeededEmailDisplay(reason) { let emailHasBeenSentBefore = sessionStorage.getItem(emailSentSessionVariableName) !== null; let collapseableDiv = readonlyView.querySelector(".collapse--dgsimple"); + let showMoreButton = document.querySelector("#action_needed_reason_email__show_details"); if ((reason && reason != "other") && !emailHasBeenSentBefore) { showElement(actionNeededEmail.parentElement) hideElement(readonlyView) + hideElement(showMoreButton) } else { if (!reason || reason === "other") { collapseableDiv.innerHTML = reason ? "No email will be sent." : "-"; + hideElement(showMoreButton) + if (collapseableDiv.classList.contains("collapsed")) { + showMoreButton.click() + } + }else { + showElement(showMoreButton) } hideElement(actionNeededEmail.parentElement) showElement(readonlyView) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 0eba6c9b4..51dd9b518 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -149,7 +149,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) -