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

Updated code to set header on email subject based on desired logic #39

Merged
merged 4 commits into from
Aug 27, 2024

Conversation

jcadam14
Copy link
Contributor

Closes #38

Tested locally in docker by setting ENVIRONMENT: DEV in the docker-compose env variables for the mailing-api, which sent to mailpint an email with "subject": "[DEV BETA] SBL User Request for Institution Profile Change"

Then removed the ENVIRONMENT variable and changed the keycloak user to have both @cfpg.gov and @new-cfpb.gov email domains, both results ended in an email with "subject": "[CFPB BETA] SBL User Request for Institution Profile Change"

Changed account to have an email domain of "citibank.com" and had an email sent with "subject": "[BETA] SBL User Request for Institution Profile Change"

@jcadam14 jcadam14 self-assigned this Aug 27, 2024
@jcadam14 jcadam14 linked an issue Aug 27, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Aug 27, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/regtech_mail_api
  api.py 73
  settings.py
Project Total  

This report was generated by python-coverage-comment-action

subject = f"[DEV BETA] SBL User Request for {type}"

header = "[BETA]"
if "cfpb" in sender_addr:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if "cfpb" in sender_addr:
sender_addr.lower().endswith('cfpb.gov'):

...just to be extra safe and make sure we don't capture some [email protected] type address?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, will update. There are some test CFPB domains we use to verify they’re not associated yet so we’ll just need to make sure they follow a *cfpb.gov format and not @cfpb-test.gov type thing. Or building a configurable list of domains that will get flagged [CFPB BETA]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok changed the check to split on the @ and check for "cfpb" in the domain place. That will allow Bill's current test email accounts to stay the same format but would flag them appropriately.

@jcadam14 jcadam14 requested a review from hkeeler August 27, 2024 23:28
@jcadam14 jcadam14 merged commit 9a76bfd into main Aug 27, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Subject being sent to SBL Help
2 participants