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

Mailer jobs not cleaned up when a notification is destroyed #50

Open
sina-s opened this issue Dec 14, 2017 · 4 comments
Open

Mailer jobs not cleaned up when a notification is destroyed #50

sina-s opened this issue Dec 14, 2017 · 4 comments

Comments

@sina-s
Copy link

sina-s commented Dec 14, 2017

I'm using this awesome gem and I have a notifiable model with dependent_notifications: :destroy enabled. The issue is that if the notifiable is created and shortly destroyed before the mailer job is performed, I get a Couldn't find ActivityNotification::Notification with 'id'=xyz error in the background jobs cause the corresponding notification is destroyed. I know this is an edge case, but there is a real case for it, in my case, the notifiable is a "Like" action and if user clicks on Like and UnLike quickly this happens.
I also couldn't keep the notifications around after the notifiable is destroyed cause the email template then can't find the Notifiable to generate an email.

@sina-s
Copy link
Author

sina-s commented Dec 14, 2017

Also googling around a bit for canceling Active Jobs (DelayedJobs in my case) in a callback monkey-patched to ActivityNotification::Notification, it turns out there is no supported API to do that. A better/simpler way would be to let the mailer job execute but do nothing if the notification is destroyed. Not sure that's doable in ActionMailer::DeliveryJob.

@simukappu
Copy link
Owner

This makes sense. Is it OK to add the option which Mailer will ignore them or just create logs without sending email when Notification record cannot be found?

@sina-s
Copy link
Author

sina-s commented Dec 17, 2017

Yes, that's ideal

@supairish
Copy link

This seems to still be an issue, is there a workaround?

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

3 participants