Send email on Workflow Run Success/Failure #34982
Conversation
GiteaBot
added
the
lgtm/need 2
github-actions
bot
added
modifies/go
|
|
|
replace #33601 |
services/mailer/mail_workflow_run.go
Outdated
| } | ||
| } | ||
|
|
||
| watchers, err := repo_model.GetRepoWatchers(ctx, repo.ID, db.ListOptionsAll) |
There was a problem hiding this comment.
We need a clear definition about "who should receive the emails"
I don't think the "watchers" should receive, for example: on GitHub, I watched some repositories that I have interests, but it doesn't mean that I should receive their notifications.
There was a problem hiding this comment.
Fair enough. I look around but could not find a solid answer, end up asking AI. Send to watchers that are on "And Your Own Notifications"?
There was a problem hiding this comment.
IMO:
- PR poster
- PR reviewers (maybe)
- Workflow doer
There was a problem hiding this comment.
Yes I am writing about an feature opt out, this would affect me on Gitea.com. My local installation cannot deliver E-Mails.
I am a fan of this global user setting of GitHub Actions (https://github.com/settings/notifications)
Other than that only notify on failure is there as well
If I am not mistaken, this would start sending me E-Mails as long I do not opt out anything else.
Reasoning: On GitHub Actions I have multiple daily failures that are not worth a mail.
I read scheduled workflows on GitHub Actions are sent to the last doer changed the cron setting, idk what exactly happens here.
Otherwise I also read this is full of randomness for this trigger
There was a problem hiding this comment.
Since Gitea currently has no fine-grained control over notification, adding the setting requires a database schema migration.
This comment was marked as resolved.
This comment was marked as resolved.
outdatedAnd by reading the history discussions: I think the concern is not addressed? It looks right, the concerns in "Actions - send Email on Success/Failure #33601" should all be addressed. 🙏 |
| @@ -0,0 +1,90 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
Hmm, I think we can try to introduce a "email template review" function in devtest (in the future), then we can preview the email templates. 🤔
There was a problem hiding this comment.
Refactor mail template and support preview #34990
I think we can use this "mail preview" to help to fine tune mail templates.
(If there is any conflict, I will help to resolve)
| <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> | ||
| <meta name="format-detection" content="telephone=no,date=no,address=no,email=no,url=no"> | ||
| <title>{{.Subject}}</title> | ||
| <style> |
There was a problem hiding this comment.
Although the styles look beautiful, but I think we can't use them (well, AI thinks)
There was a problem hiding this comment.
Legacy code. And that style is quite simple, even it doesn't work, the rendered content still looks fine.
There was a problem hiding this comment.
How about embedding style into tags? e.g. <div style="...">
There was a problem hiding this comment.
Yup, I think it is the only working approach 🤔
(Maybe Refactor mail template and support preview #34990 can help to work on the mail templates.)
There was a problem hiding this comment.
Or maybe we can keep this approach. And we can write a helper function to embed these styles to inline (in the future)
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Closes #23725
When to send notification: Run Status Done
Send to primary email address.