Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Email sending condition relies on action, not parsed action #213

Open
PVince81 opened this issue Jul 10, 2018 · 1 comment
Open

Email sending condition relies on action, not parsed action #213

PVince81 opened this issue Jul 10, 2018 · 1 comment

Comments

@PVince81
Copy link
Contributor

Whenever a notification is sent by API with one or multiple actions, the notification needs to be processed by a INotifier instance. If not INotifier instance exists or if it doesn't return a parsed action, the web UI will not display any actions.

The decision whether to send out an email for notifications is based whether said notification contains at least an action. Currently it is checking if there is an "action" and not a "parsedAction".

We should probably make this consistent to avoid confusing behavior.

@jvillafanez

@jvillafanez
Copy link
Member

I think this is a difficult decision. The reason to split the NotificationMailer in 2 classes was to simplify the dependencies instead of having one class depending on 7 others.

At the point where we check the notification we can't rely on parsedActions. We have to prepare the notification before checking. This would imply to move the NotificationManager and OptionsStorage up to the adapter, and the actual mailer will be left with very little things to do. We might want to merge both classes into one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants