V2 #313
V2 #313
Conversation
…rs can edit migration to jsonb if needed.
|
Readd support for delays |
|
Woop woop!! |
| @@ -0,0 +1,9 @@ | |||
| module Noticed | |||
| class ApplicationJob < ActiveJob::Base | |||
There was a problem hiding this comment.
I'm not sure we called it out anywhere, but this gem does have a hard dependence on ActiveJob 🤔 I know there are plenty of apps out there using Sidekiq directly (for example), so this may be worth noting.
There was a problem hiding this comment.
We could add an explicit require for it if needed. Could also replace the dependency on Rails with ActiveRecord + ActiveJob.
| return false if config.has_key?(:if) && !evaluate_option(:if) | ||
| return false if config.has_key?(:unless) && evaluate_option(:unless) | ||
|
|
||
| deliver |
There was a problem hiding this comment.
🤔 I wish there was a way to wrap this in with_lock and ensure that this particular [notification, delivery_method] hasn't already sent. I've definitely had cases where someone got multiple messages sent due to a retrying job. I know ultimately it boils down to never truly being able to sync up the web/email POST (or etc) with the database's state... but just wanted to throw it out there!
IIRC we're not retrying jobs anyway, so maybe it's not a big deal!
There was a problem hiding this comment.
Long-term we can add the ability for storing the results of delivery methods on the Noticed::Notification so the delivery methods can check if they were completed already before they run.
This introduced a new major version of Noticed with some big improvements.
record{polymorphic}association to Notification model to improve querying ergonomics.NotificationRecipientmodel to reduce data duplication.httpgem dependency and replace with net/http