Skip to content

Add failed-job email notifications using CDA user lists - #186

Open
krowvin wants to merge 29 commits into
cwbi-devfrom
157-add-alert-ad-hoc-email-queue
Open

Add failed-job email notifications using CDA user lists#186
krowvin wants to merge 29 commits into
cwbi-devfrom
157-add-alert-ad-hoc-email-queue

Conversation

@krowvin

@krowvin krowvin commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add durable, office-scoped failed-job notification templates and per-script alert rules
  • resolve current recipients from CDA user lists and expose list selection in Scripts Manager
  • enqueue rendered email envelopes for a separate local/AWS notification worker, with retry/DLQ-compatible behavior
  • document the notification contract, local flow, and future producer path

Why this follow-up is needed

PR #160 added API-key authentication for clients calling Batch Events, but it did not implement notification configuration, recipient resolution, queueing, delivery, or the management UI. This PR provides that missing notification workflow for part of issue #157.

Recipient lists remain owned and maintained by CDA to avoid duplicating user membership in Batch Events. This work therefore depends on the office-scoped user-list API and UI in USACE/cwms-data-api#1734.

User impact

Script administrators can enable email-on-failure for an individual script, select a reusable email template, and select an office-scoped CDA user list. Batch Events resolves current list membership when the failure occurs and queues a rendered message for delivery. The queue contract is intentionally generic enough for later authorized ad-hoc or event-driven email producers.

Validation

  • python -m pytest -q — 133 passed
  • npm run lint — passed
  • npm run build — passed (existing unresolved government-banner SVG and large-chunk warnings remain)

@krowvin krowvin linked an issue Aug 4, 2026 that may be closed by this pull request
@krowvin
krowvin requested a review from MikeNeilson August 5, 2026 17:01
@krowvin
krowvin marked this pull request as ready for review August 5, 2026 17:01
krowvin added 3 commits August 5, 2026 12:17
Signed-off-by: Charles Graham, SWT <charles.r.graham@usace.army.mil>
Signed-off-by: Charles Graham, SWT <charles.r.graham@usace.army.mil>
Signed-off-by: Charles Graham, SWT <charles.r.graham@usace.army.mil>
@krowvin
krowvin removed the request for review from MikeNeilson August 5, 2026 17:25
@krowvin

krowvin commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

I caught a few things i'd like to improve on in the UI. Will request review again when i get those in with some screenshots!

krowvin added 6 commits August 5, 2026 12:27
Signed-off-by: Charles Graham, SWT <charles.r.graham@usace.army.mil>
Signed-off-by: Charles Graham, SWT <charles.r.graham@usace.army.mil>
Signed-off-by: Charles Graham, SWT <charles.r.graham@usace.army.mil>
Signed-off-by: Charles Graham, SWT <charles.r.graham@usace.army.mil>
@krowvin

krowvin commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Screenshots

/scripts-manager - Script Details tab

image

/scripts-manager - Script Failure Email (On)

image

/scripts-manager - Script Failure Email (Off)

image

/scripts-manager - Making a new alert for that script

Manual email

image

CDA User List

image

Empty Office / one that we do not have permissions to create lists for
image

When you click "manage user lists" it takes you to the new CDA UI tool for creating these managing them. Screenshots here:
USACE/cwms-data-api#1734 (comment)

/scripts-manager - Preview email/selected template

image image

/events/setup (Edit button above will target a URI to get you to that template)

image

@krowvin

krowvin commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author
image

Updated the way the manage user list button looks

@krowvin
krowvin requested a review from MikeNeilson August 5, 2026 20:43

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what's with the numbering?

sorry, might make sense if I saw the other ones, just too used to doing calendar versioning in these.

@krowvin krowvin Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Brandon had setup the repo to be

V - Version migration
1_02_04 - semver
__ - sep the version from description
etc - the rest is any human readable note

My understanding is that flyway uses the values to apply the migrations in order and record them in the schema history.

Looks like he got it from a flyway migration template that CWBI had

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh yeah, my comment was on the specific version number, not that it was a version.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Okay names work they are just... weird. but the thing is live so we live with it.

@@ -0,0 +1,25 @@
const EMAIL_PATTERN =
/[a-z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)+/gi;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might want to put a comment in here that while this by definition has to be "wrong" (the valid forms of email being rather near infinite), that it will match > 99% of what we would normally expect.... you know for the next person like me who will point out these never fully work.

@krowvin krowvin Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed it's a fools errand

MDN takes a crack at it too

the parser is a modified version of what MDN has

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done here:
183a9d6

Comment thread ui/.env.dev

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

still don't like .env files being in the repo... I've said it, that is all.

@krowvin krowvin Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell it's common practice with vite https://vite.dev/guide/env-and-mode#env-files

To have the env files committed but to make sure you prefix with VITE_ to ensure there's a clear line between frontend env variables and otherwise.

But to your point I try not to commit a ".env" file.

Not saying don't feel that way, just pointing this out!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Okay names work they are just... weird. but the thing is live so we live with it.

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.

2 participants