Skip to content

Rp populate sent files table when sending emails with files - #5017

Open
rparke wants to merge 5 commits into
mainfrom
rp-populate-sent-files-table-when-sending-emails-with-files
Open

rparke wants to merge 5 commits into
mainfrom
rp-populate-sent-files-table-when-sending-emails-with-files

Conversation

@rparke

@rparke rparke commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

We should probably merge this first so that the colum rename happens before we actually start populating the table #5035.

@rparke
rparke force-pushed the rp-populate-sent-files-table-when-sending-emails-with-files branch 4 times, most recently from 6d6141b to 39f1a58 Compare October 28, 2026 09:28
to make this as generic as possible we should only care about the placeholder name. Filenames are "optional" when sending via the API, but are set automatically when sent via admin. We should record both cases and therefore use the templates placeholder to identify the file instead of filename.
@rparke
rparke force-pushed the rp-populate-sent-files-table-when-sending-emails-with-files branch 2 times, most recently from 954cb98 to 9bde163 Compare October 29, 2026 10:20
@rparke
rparke marked this pull request as ready for review October 29, 2026 10:21
adds a record to the `sent_files` table, to be updated with the file id we get back from document download as `document_id` and allows us to query file ids for easy deletion if needed.
…s sent via either the api, one-off, or jobs flows.

when sending via the api we need to handle files uploaded via the API and those that are already "attached" to a template.

We heavily mock out calls to `persist_notification` in our one_off tests and it's a bit tricky to test that the table is actually updated. However we test that `persist_notification` is called with the correct args and we separately test taht `persist_notification` adds the record to the table.
@rparke
rparke force-pushed the rp-populate-sent-files-table-when-sending-emails-with-files branch from 4d32e58 to 1916159 Compare October 29, 2026 13:40
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.

1 participant