Send email on Workflow Run Success/Failure #34982
Conversation
GiteaBot
added
the
lgtm/need 2
github-actions
bot
added
modifies/go
|
|
|
replace #33601 |
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. 🙏 |
|
Is the new table |
|
@lunny The use of |
|
I also think it should reuse the existing setting system, instead of introducing a new system. And the existing system is indeed not clear, it needs refactoring and improvements. |
probably just add a "previousRunFailed" property to the event then it should be easy to make conditions for it based on user configuration |
|
|
||
| recipients := make([]*user_model.User, 0) | ||
|
|
||
| if !sender.IsGiteaActions() && !sender.IsGhost() && sender.IsMailable() { |
There was a problem hiding this comment.
Why checking sender's preference rather than receivers?
There was a problem hiding this comment.
@lunny Currently in the code the sender is the receiver. Only the sender goes receiver list.
#34982 (comment)
#34982 (comment)
|
Any further review please? |
I will complete my review by Monday. |
GiteaBot
added
lgtm/need 1
| ctx.ServerError("GetUserNotificationSettings", err) | ||
| return | ||
| } | ||
| actionsNotify := fineGrainedPreference[user_model.SettingsEmailNotificationGiteaActions].SettingValue |
There was a problem hiding this comment.
It will panic if the setting key doesn't exist.
The user setting system really needs a complete refactor
| } | ||
| } | ||
|
|
||
| func SendActionsWorkflowRunStatusEmail(ctx context.Context, sender *user_model.User, repo *repo_model.Repository, run *actions_model.ActionRun) { |
There was a problem hiding this comment.
Why there are two SendActionsWorkflowRunStatusEmail sendActionsWorkflowRunStatusEmail for different purposes ...
Is it possible to make them have different names?
Closes #23725
/claim #23725