Repository navigation
feat: attach exported files to scheduled report emails with export.attach - #10031
Conversation
- Add an opt-in `export.attach` report property (`export_attach` on `ReportSpec` and `ReportOptions`) and an "Attach file to emails" option in the explore report form. - Only supported for query-based reports run as the creator; the admin API and the parser reject it for recipient mode, canvas PDF and AI reports. - The report reconciler exports the data once with the report owner's attributes and attaches it to every email; Slack notifications keep the download link. - Exports larger than 10 MB are not attached; the email says so and keeps the download link, and the run records a warning. - Extract `queries.ProtoToExportQuery` from the download handler, so attachments and download links build the same query. - The SMTP sender only composes a multipart message when there is an attachment. - Add `testruntime.InstanceOptions.AdminService`, so runtime tests can deliver reports.
AdityaHegde
left a comment
AdityaHegde
left a comment
There was a problem hiding this comment.
The include metadata and attach checkboxes have different info icon positions. We can perhaps move both to match attach's checkbox?
|
|
||
| type Sender interface { | ||
| Send(toEmail, toName, subject, body string) error | ||
| Send(toEmail, toName, subject, body string, attachments ...Attachment) error |
There was a problem hiding this comment.
Any specific reason to use rest param instead of array? Will be easier to add new params in the future?
There was a problem hiding this comment.
Switched to []Attachment; the other callers pass nil. (83f67a8)
| </Tooltip> | ||
| </div> | ||
| <Checkbox | ||
| checked={$data["exportAttach"] && canAttach} |
There was a problem hiding this comment.
nit: adding canAttach to check feels weird. We should either reset exportAttach when we cannot attach or still show the selected state but disabled.
There was a problem hiding this comment.
Went with the reset: exportAttach is cleared when Run as is not Creator, so the checkbox binds it directly. (83f67a8)
…ption outside creator mode, align the metadata info icon
…attachments # Conflicts: # proto/gen/rill/admin/v1/api.pb.go # proto/gen/rill/runtime/v1/resources.pb.go
d6e7c09 to
31c2ca0
Compare
|
Info icons: Include metadata now uses the checkbox |
2 participants
Adds an opt-in option to attach a scheduled report's export file to its emails.
export_attachonReportOptionsandReportSpecandexport.attachin report YAML. Off by default, so existing reports are unchanged.queries.ProtoToExportQueryis extracted from the download handler, so attachments and download links build the same query.testruntime.InstanceOptions.AdminServicelets runtime tests deliver reports (the no-op admin skips them); the new reconciler tests use it.admin_owner_user_id) skip the attachment with a warning, since their download links are issued per recipient.Checklist: