CROSSLINK-269 Email pullslips#624
Open
JanisSaldabols wants to merge 6 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new scheduled batch action (email-pullslips) that sends pull-slip emails (optionally with a merged PDF attachment) via AWS SES, and extends batch actions to persist arbitrary actionParams in the database and API.
Changes:
- Added an
EmailSenderServicetask handler that builds and sends raw MIME emails through AWS SES, optionally attaching a merged pull-slip PDF. - Extended scheduler batch actions to accept/store
actionParams(JSONB) and renamed the email batch action fromemail/send-emailtoemail-pullslipsacross API, events, and migrations. - Added a simple SES mock service for local testing and introduced AWS SDK v2 SES dependencies.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
illmock/ses/ses_mock.go |
Adds a local SES mock HTTP server to support testing with endpoint override. |
broker/sqlc/sqlc.yaml |
Overrides batch_action.action_params Go type for sqlc generation. |
broker/sqlc/sched_schema.sql |
Adds action_params JSONB column to batch_action. |
broker/sqlc/sched_query.sql |
Persists action_params in SaveBatchAction. |
broker/scheduler/service/email_sender.go |
Implements SES raw-email sending + MIME building + optional PDF attachment. |
broker/scheduler/service/email_sender_test.go |
Adds unit tests for email data extraction, MIME building, and task handling. |
broker/scheduler/api/api_handler.go |
Wires batch-action creation to email-pullslips and passes actionParams into task payload + DB record. |
broker/scheduler/api/api_handler_test.go |
Updates tests to use email-pullslips action name. |
broker/pullslip/service/pdf.go |
Refactors PDF service into an interface + concrete implementation for easier injection/mocking. |
broker/pullslip/service/pdf_test.go |
Updates tests for the PDF service refactor. |
broker/oapi/open-api.yaml |
Updates batch-action enum and adds actionParams schema. |
broker/migrations/042_add_action_params.up.sql |
Adds action_params column and attempts to rename event name. |
broker/migrations/042_add_action_params.down.sql |
Drops action_params column and attempts to revert event name. |
broker/go.mod / broker/go.sum |
Adds AWS SDK v2 dependencies for SES integration. |
broker/events/eventmodels.go |
Renames the email event constant to email-pullslips. |
broker/app/app.go |
Initializes EmailSenderService and conditionally registers the new event handler. |
JanisSaldabols
force-pushed
the
CROSSLINK-269
branch
from
e513ee8 to
08d8d8d
Compare
jakub-id
reviewed
Jun 1, 2026
jakub-id
requested changes
Jun 2, 2026
| EventNameSendNotification EventName = "send-notification" | ||
| EventNameCheckAvailability EventName = "check-availability" | ||
| EventNameSendEmail EventName = "send-email" | ||
| EventNameEmailPullslips EventName = "email-pullslips" |
Contributor
There was a problem hiding this comment.
@JanisSaldabols this is not wrong, but I sorta expected that all batch actions would have a generic invoke-batch-action event type like the regular actions have invoke-action and action name would go to the action custom data.
| | `SMTP_PORT` | SMTP server port | `2525` | | ||
| | `SMTP_USERNAME` | Username for SMTP authentication | (empty value) | | ||
| | `SMTP_PASSWORD` | Password for SMTP authentication | (empty value) | | ||
| | `SMTP_FROM_ADDR` | Default sender address for outgoing emails | (empty value) | |
Contributor
There was a problem hiding this comment.
@JanisSaldabols I don't think this makes sense because a single broken deployment will be shared across all tenants and each will likely want their from address. Use the directory email field instead.
| ctx.Logger().Warn("email batch truncated: selector matched more records than the per-email limit", | ||
| "matched", fullCount, "limit", maxRecordsPerEmail) | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
@JanisSaldabols please replaceAll {{fullCount}}, {{actualCount}} and {{batchQuery}} in the email body with fullCount, limit and selector so we can template the email.
jakub-id
reviewed
Jun 2, 2026
|
|
||
| // maxRecordsPerEmail caps the number of patron requests fetched for a single | ||
| // email batch. Selectors matching more records will be truncated with a warning. | ||
| const maxRecordsPerEmail = 100 |
Contributor
There was a problem hiding this comment.
@JanisSaldabols can you make this configurable via env as BATCH_PULLSLIP_MAX_COUNT?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.