Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2355: Custom rejection reason emails - [BACKUP] - MIGRATION #2869

Merged
merged 39 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
61b1cee
Refactor
zandercymatics Sep 24, 2024
48b9206
migration
zandercymatics Sep 27, 2024
9b23262
Initial architecture for rejection reason
zandercymatics Sep 27, 2024
1ce0724
Cleanup + send email logic
zandercymatics Sep 27, 2024
e37a95e
Typo
zandercymatics Sep 27, 2024
c084ec6
check for page
zandercymatics Sep 27, 2024
ff5212f
Remove unhelpful logic
zandercymatics Sep 30, 2024
7a5eda0
Fix back button bug
zandercymatics Sep 30, 2024
cfa1879
cleanup
zandercymatics Sep 30, 2024
7cc5231
cleanup
zandercymatics Sep 30, 2024
6bb907c
Simplify backend logic
zandercymatics Sep 30, 2024
5a96855
Simplify PR
zandercymatics Oct 1, 2024
47d226f
Update domain_request.py
zandercymatics Oct 1, 2024
62156ac
Merge branch 'main' into hotgov/2355-rejection-reason-emails
zandercymatics Oct 1, 2024
162369f
add migration
zandercymatics Oct 1, 2024
0a7d4e4
Exclude other reason for now
zandercymatics Oct 1, 2024
06e4dae
lint
zandercymatics Oct 1, 2024
ad43fab
fix unit tests
zandercymatics Oct 1, 2024
997fc1b
Remove no longer relevant test + linting
zandercymatics Oct 2, 2024
db0d4a1
Update admin.py
zandercymatics Oct 2, 2024
2766202
toggle email wrap
zandercymatics Oct 2, 2024
1424197
Fix remaining tests
zandercymatics Oct 2, 2024
b30e017
rename
zandercymatics Oct 2, 2024
02e18a3
Update get-gov-admin.js
zandercymatics Oct 2, 2024
5fc8d0d
Merge branch 'main' into hotgov/2355-rejection-reason-emails
zandercymatics Oct 7, 2024
ade6733
Remove bad import
zandercymatics Oct 8, 2024
e3d2dc0
Remove old migrations (merging)
zandercymatics Oct 8, 2024
c76e27f
Merge branch 'main' into hotgov/2355-rejection-reason-emails
zandercymatics Oct 8, 2024
081c762
Revert "Remove old migrations (merging)"
zandercymatics Oct 8, 2024
a4ffc75
Fix migrations
zandercymatics Oct 8, 2024
fc8090d
Merge branch 'main' into hotgov/2355-rejection-reason-emails
zandercymatics Oct 9, 2024
7c69205
Update test_models.py
zandercymatics Oct 9, 2024
3ba4293
Update test_models.py
zandercymatics Oct 9, 2024
bd603f6
Remove space
zandercymatics Oct 9, 2024
ec1df25
remove bad test + lint
zandercymatics Oct 9, 2024
dbfc548
Fix test + some minor cleanup stuff
zandercymatics Oct 10, 2024
3e54bb1
simplify javascript
zandercymatics Oct 10, 2024
3fb9ab5
lint
zandercymatics Oct 10, 2024
8b70554
Merge branch 'main' into hotgov/2355-rejection-reason-emails
zandercymatics Oct 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions src/registrar/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@
from registrar.models.user_domain_role import UserDomainRole
from waffle.admin import FlagAdmin
from waffle.models import Sample, Switch
from registrar.utility.admin_helpers import get_all_action_needed_reason_emails, get_action_needed_reason_default_email
from registrar.utility.admin_helpers import (
get_all_action_needed_reason_emails,
get_action_needed_reason_default_email,
get_all_rejection_reason_emails,
get_rejection_reason_default_email,
)
from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial
from registrar.utility.constants import BranchChoices
from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes
Expand Down Expand Up @@ -234,6 +239,7 @@ class Meta:
}
labels = {
"action_needed_reason_email": "Email",
"rejection_reason_email": "Email",
}

def __init__(self, *args, **kwargs):
Expand Down Expand Up @@ -1755,6 +1761,7 @@ def status_history(self, obj):
"status_history",
"status",
"rejection_reason",
"rejection_reason_email",
"action_needed_reason",
"action_needed_reason_email",
"investigator",
Expand Down Expand Up @@ -1937,25 +1944,7 @@ def save_model(self, request, obj, form, change):
# Get the original domain request from the database.
original_obj = models.DomainRequest.objects.get(pk=obj.pk)

# == Handle action_needed_reason == #

reason_changed = obj.action_needed_reason != original_obj.action_needed_reason
if reason_changed:
# 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 if nothing exists.
# Since this check occurs after save, if the user enters a value then we won't update.

default_email = get_action_needed_reason_default_email(request, obj, obj.action_needed_reason)
if obj.action_needed_reason_email:
emails = get_all_action_needed_reason_emails(request, 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 allowed emails == #
if obj.status in DomainRequest.get_statuses_that_send_emails() and not settings.IS_PRODUCTION:
self._check_for_valid_email(request, obj)

Expand Down
441 changes: 290 additions & 151 deletions src/registrar/assets/js/get-gov-admin.js

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions src/registrar/config/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
get_senior_official_from_federal_agency_json,
get_federal_and_portfolio_types_from_federal_agency_json,
get_action_needed_email_for_user_json,
get_rejection_email_for_user_json,
)
from registrar.views.domains_json import get_domains_json
from registrar.views.utility import always_404
Expand Down Expand Up @@ -159,6 +160,11 @@
get_action_needed_email_for_user_json,
name="get-action-needed-email-for-user-json",
),
path(
"admin/api/get-rejection-email-for-user-json/",
get_rejection_email_for_user_json,
name="get-rejection-email-for-user-json",
),
path("admin/", admin.site.urls),
path(
"reports/export_data_type_user/",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 4.2.10 on 2024-09-26 21:18

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("registrar", "0128_alter_domaininformation_state_territory_and_more"),
]

operations = [
migrations.AddField(
model_name="domainrequest",
name="rejection_reason_email",
field=models.TextField(blank=True, null=True),
),
]
103 changes: 61 additions & 42 deletions src/registrar/models/domain_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ def get_action_needed_reason_label(cls, action_needed_reason: str):
blank=True,
)

rejection_reason_email = models.TextField(
null=True,
blank=True,
)

action_needed_reason = models.TextField(
choices=ActionNeededReasons.choices,
null=True,
Expand Down Expand Up @@ -635,15 +640,16 @@ def sync_organization_type(self):
# Actually updates the organization_type field
org_type_helper.create_or_update_organization_type()

def _cache_status_and_action_needed_reason(self):
def _cache_status_and_status_reasons(self):
"""Maintains a cache of properties so we can avoid a DB call"""
self._cached_action_needed_reason = self.action_needed_reason
self._cached_rejection_reason = self.rejection_reason
self._cached_status = self.status

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Store original values for caching purposes. Used to compare them on save.
self._cache_status_and_action_needed_reason()
self._cache_status_and_status_reasons()

def save(self, *args, **kwargs):
"""Save override for custom properties"""
Expand All @@ -657,21 +663,42 @@ def save(self, *args, **kwargs):

# Handle the action needed email.
# An email is sent out when action_needed_reason is changed or added.
if self.action_needed_reason and self.status == self.DomainRequestStatus.ACTION_NEEDED:
self.sync_action_needed_reason()
if self.status == self.DomainRequestStatus.ACTION_NEEDED:
self.send_another_status_reason_email(
checked_status=self.DomainRequestStatus.ACTION_NEEDED,
old_reason=self._cached_action_needed_reason,
new_reason=self.action_needed_reason,
excluded_reasons=[DomainRequest.ActionNeededReasons.OTHER],
email_to_send=self.action_needed_reason_email
)
elif self.status == self.DomainRequestStatus.REJECTED:
self.send_another_status_reason_email(
checked_status=self.DomainRequestStatus.REJECTED,
old_reason=self._cached_rejection_reason,
new_reason=self.rejection_reason,
excluded_reasons=[DomainRequest.RejectionReasons.OTHER],
email_to_send=self.rejection_reason_email,
)

# Update the cached values after saving
self._cache_status_and_action_needed_reason()

def sync_action_needed_reason(self):
"""Checks if we need to send another action needed email"""
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:
# 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._cache_status_and_status_reasons()

def send_another_status_reason_email(self, checked_status, old_reason, new_reason, excluded_reasons, email_to_send):
"""Helper function to send out a second status email when the status remains the same,
but the reason has changed."""

# If the status itself changed, then we already sent out an email
if self._cached_status != checked_status or old_reason is None:
return

# We should never send an email if no reason was specified
# Additionally, Don't send out emails for reasons that shouldn't send them
if new_reason is None or self.action_needed_reason in excluded_reasons:
return

# Only send out an email if the underlying email itself changed
if old_reason != new_reason:
self._send_custom_status_update_email(email_content=email_to_send)

def sync_yes_no_form_fields(self):
"""Some yes/no forms use a db field to track whether it was checked or not.
Expand Down Expand Up @@ -798,6 +825,19 @@ def _send_status_update_email(
except EmailSendingError:
logger.warning("Failed to send confirmation email", exc_info=True)

def _send_custom_status_update_email(self, email_content):
"""Wrapper for `_send_status_update_email` that bcc's [email protected]
and sends an email equivalent to the 'email_content' variable."""
bcc_address = settings.DEFAULT_FROM_EMAIL if settings.IS_PRODUCTION else ""
self._send_status_update_email(
new_status="action needed",
email_template=f"emails/includes/custom_email.txt",
email_template_subject=f"emails/status_change_subject.txt",
bcc_address=bcc_address,
custom_email_content=email_content,
wrap_email=True,
)

def investigator_exists_and_is_staff(self):
"""Checks if the current investigator is in a valid state for a state transition"""
is_valid = True
Expand Down Expand Up @@ -901,7 +941,7 @@ def in_review(self):
target=DomainRequestStatus.ACTION_NEEDED,
conditions=[domain_is_not_active, investigator_exists_and_is_staff],
)
def action_needed(self, send_email=True):
def action_needed(self):
"""Send back an domain request that is under investigation or rejected.

This action is logged.
Expand All @@ -924,27 +964,7 @@ def action_needed(self, send_email=True):
# Send out an email if an action needed reason exists
if self.action_needed_reason and self.action_needed_reason != self.ActionNeededReasons.OTHER:
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, email_content=None):
"""Sends out an automatic email for each valid action needed reason provided"""

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

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_custom_status_update_email(email_content)

@transition(
field="status",
Expand Down Expand Up @@ -1045,11 +1065,10 @@ def reject(self):
if self.status == self.DomainRequestStatus.APPROVED:
self.delete_and_clean_up_domain("reject")

self._send_status_update_email(
"action needed",
"emails/status_change_rejected.txt",
"emails/status_change_rejected_subject.txt",
)
# Send out an email if a rejection reason exists
if self.rejection_reason:
email_content = self.rejection_reason_email
self._send_custom_status_update_email(email_content)

@transition(
field="status",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
<input id="has_audit_logs" class="display-none" value="{%if filtered_audit_log_entries %}true{% else %}false{% endif %}"/>
{% url 'get-action-needed-email-for-user-json' as url %}
<input id="get-action-needed-email-for-user-json" class="display-none" value="{{ url }}" />
{% url 'get-rejection-email-for-user-json' as url_2 %}
<input id="get-rejection-email-for-user-json" class="display-none" value="{{ url_2 }}" />
{% for fieldset in adminform %}
{% comment %}
TODO: this will eventually need to be changed to something like this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,99 @@ <h2 class="usa-modal__heading">
</div>

{% if original_object.action_needed_reason_email %}
<input id="last-sent-email-content" class="display-none" value="{{original_object.action_needed_reason_email}}">
<input id="last-sent-action-needed-email-content" class="display-none" value="{{original_object.action_needed_reason_email}}">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NITPICK: Fix the tabbing

{% else %}
<input id="last-sent-email-content" class="display-none" value="None">
<input id="last-sent-action-needed-email-content" class="display-none" value="None">
{% endif %}

{% elif field.field.name == "rejection_reason_email" %}
<div class="margin-top-05 text-faded field-rejection_reason_email__placeholder">
&ndash;
</div>

{{ field.field }}

<button
aria-label="Edit email in textarea"
type="button"
class="usa-button usa-button--unstyled usa-button--dja-link-color usa-button__small-text margin-left-1 text-no-underline field-rejection_reason_email__edit flex-align-self-start"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these buttons/ modals will never exist on the same page at the same time, I think there's an opportunity to significantly simplify the HTML and the JS by using generic handles (email-edit instead of field-rejection_reason_email__edit).

On the JS: You can now trim down config by moving a lot of the handles to the parent class

On the template: You can probably pull out the modals into an include that's a variant of modal.html, if not modal.html itself (that would be ideal)

It's not insignificant and your code has nothing wrong with it, so I won' block if you decide not to do it, but we should capture this plan for the next custom email ticket if it exists.

Copy link
Contributor Author

@zandercymatics zandercymatics Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these buttons/ modals will never exist on the same page at the same time

Ah, I think they do actually! Basically the blocks for detail_table_fieldset.html are nested inside in a for loop on each field present on the page. You can verify this via inspect element or by breaking the js somewhere to prevent hiding dom elements.

However, I do really like your train of thought here. We could achieve what your describing by assigning those generic names as classes and having the js infer what the right values are given their expected dom position

You can probably pull out the modals into an include that's a variant of modal.html, if not modal.html itself

You're totally right about that. I'll look at it and see if this is easily achievable (it might be)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I went through and simplified the javascript (though more can be done with the formgroups). Let me know what you think.

For the templates + further refinement we can definitely do that in a follow-on like you said

><img src="/public/admin/img/icon-changelink.svg" alt="Change"> Edit email</button
>
<a
href="#email-already-sent-modal"
class="usa-button usa-button--unstyled usa-button--dja-link-color usa-button__small-text text-no-underline margin-left-1 field-rejection_reason_email__modal-trigger flex-align-self-start"
aria-controls="email-already-sent-modal"
data-open-modal
><img src="/public/admin/img/icon-changelink.svg" alt="Change"> Edit email</a
>
<div
class="usa-modal"
id="email-already-sent-modal"
aria-labelledby="Are you sure you want to edit this email?"
aria-describedby="The creator of this request already received an email"
>
<div class="usa-modal__content">
<div class="usa-modal__main">
<h2 class="usa-modal__heading">
Are you sure you want to edit this email?
</h2>
<div class="usa-prose">
<p>
The creator of this request already received an email for this status/reason:
</p>
<ul>
<li class="font-body-sm">Status: <b>Rejected</b></li>
<li class="font-body-sm">Reason: <b>{{ original_object.get_rejection_reason_display }}</b></li>
</ul>
<p>
If you edit this email's text, <b>the system will send another email</b> to
the creator after you “save” your changes. If you do not want to send another email, click “cancel” below.
</p>
</div>

<div class="usa-modal__footer">
<ul class="usa-button-group">
<li class="usa-button-group__item">
<button
type="submit"
class="usa-button"
id="confirm-edit-email"
data-close-modal
>
Yes, continue editing
</button>
</li>
<li class="usa-button-group__item">
<button
type="button"
class="usa-button usa-button--unstyled padding-105 text-center"
name="_cancel_edit_email"
data-close-modal
>
Cancel
</button>
</li>
</ul>
</div>
</div>
<button
type="button"
class="usa-button usa-modal__close"
aria-label="Close this window"
data-close-modal
>
<svg class="usa-icon" aria-hidden="true" focusable="false" role="img">
<use xlink:href="{%static 'img/sprite.svg'%}#close"></use>
</svg>
</button>
</div>
</div>

{% if original_object.rejection_reason_email %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NITPICK: Tabbing

<input id="last-sent-rejection-email-content" class="display-none" value="{{original_object.rejection_reason_email}}">
{% else %}
<input id="last-sent-rejection-email-content" class="display-none" value="None">
{% endif %}
{% else %}
{{ field.field }}
{% endif %}
Expand Down
10 changes: 10 additions & 0 deletions src/registrar/templates/emails/includes/email_footer.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
THANK YOU
.Gov helps the public identify official, trusted information. Thank you for requesting a .gov domain.

----------------------------------------------------------------

The .gov team
Contact us: <https://get.gov/contact/>
Learn about .gov <https://get.gov>

The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) <https://cisa.gov/>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Hi, {{ recipient.first_name }}.

Your .gov domain request has been rejected.

DOMAIN REQUESTED: {{ domain_request.requested_domain.name }}
REQUEST RECEIVED ON: {{ domain_request.last_submitted_date|date }}
STATUS: Rejected
----------------------------------------------------------------
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #}
{% include "emails/includes/status_change_rejected_header.txt" %}
REJECTION REASON
Your domain request was rejected because we could not verify the organizational
contacts you provided. If you have questions or comments, reply to this email.

{% include "emails/includes/email_footer.txt" %}
{% endautoescape %}
Loading
Loading