|
| 1 | +# Issue #202: Instance-Level Subscriptions - Design |
| 2 | + |
| 3 | +## Schema Changes |
| 4 | + |
| 5 | +### Subscription Table |
| 6 | + |
| 7 | +Add two nullable polymorphic columns to the `subscriptions` table: |
| 8 | + |
| 9 | +``` |
| 10 | +notifiable_type :string, index: true, null: true |
| 11 | +notifiable_id :bigint, index: true, null: true (or :string for Mongoid/Dynamoid) |
| 12 | +``` |
| 13 | + |
| 14 | +- `NULL` notifiable fields = key-level subscription (existing behavior) |
| 15 | +- Non-NULL notifiable fields = instance-level subscription |
| 16 | + |
| 17 | +### Unique Constraint |
| 18 | + |
| 19 | +Replace the existing unique index: |
| 20 | +``` |
| 21 | +# Before |
| 22 | +add_index :subscriptions, [:target_type, :target_id, :key], unique: true |
| 23 | +
|
| 24 | +# After |
| 25 | +add_index :subscriptions, [:target_type, :target_id, :key, :notifiable_type, :notifiable_id], |
| 26 | + unique: true, name: 'index_subscriptions_uniqueness' |
| 27 | +``` |
| 28 | + |
| 29 | +This allows: |
| 30 | +- One key-level subscription per (target, key) where notifiable is NULL |
| 31 | +- One instance-level subscription per (target, key, notifiable) combination |
| 32 | + |
| 33 | +**Note:** Most databases treat NULL as distinct in unique indexes, so `(user1, 'comment.default', NULL, NULL)` and `(user1, 'comment.default', 'Post', 1)` are considered different. For databases that don't, a partial/conditional index may be needed. |
| 34 | + |
| 35 | +### Model Validations |
| 36 | + |
| 37 | +Update uniqueness validation in all three ORM implementations: |
| 38 | + |
| 39 | +```ruby |
| 40 | +# ActiveRecord |
| 41 | +validates :key, presence: true, uniqueness: { scope: [:target, :notifiable_type, :notifiable_id] } |
| 42 | + |
| 43 | +# Mongoid |
| 44 | +validates :key, presence: true, uniqueness: { scope: [:target_type, :target_id, :notifiable_type, :notifiable_id] } |
| 45 | + |
| 46 | +# Dynamoid |
| 47 | +validates :key, presence: true, uniqueness: { scope: :target_key } |
| 48 | +# (Dynamoid uses composite keys, needs separate handling) |
| 49 | +``` |
| 50 | + |
| 51 | +## Core Logic Changes |
| 52 | + |
| 53 | +### 1. Subscription Model (All ORMs) |
| 54 | + |
| 55 | +Add `belongs_to :notifiable` polymorphic association (optional): |
| 56 | + |
| 57 | +```ruby |
| 58 | +# ActiveRecord |
| 59 | +belongs_to :notifiable, polymorphic: true, optional: true |
| 60 | + |
| 61 | +# Mongoid |
| 62 | +belongs_to_polymorphic_xdb_record :notifiable, optional: true |
| 63 | + |
| 64 | +# Dynamoid |
| 65 | +belongs_to_composite_xdb_record :notifiable, optional: true |
| 66 | +``` |
| 67 | + |
| 68 | +Add scopes for filtering: |
| 69 | + |
| 70 | +```ruby |
| 71 | +scope :key_level_only, -> { where(notifiable_type: nil) } |
| 72 | +scope :instance_level_only, -> { where.not(notifiable_type: nil) } |
| 73 | +scope :for_notifiable, ->(notifiable) { where(notifiable_type: notifiable.class.name, notifiable_id: notifiable.id) } |
| 74 | +``` |
| 75 | + |
| 76 | +### 2. Subscriber Concern (`models/concerns/subscriber.rb`) |
| 77 | + |
| 78 | +Update `_subscribes_to_notification?` to only check key-level subscriptions: |
| 79 | + |
| 80 | +```ruby |
| 81 | +def _subscribes_to_notification?(key, subscribe_as_default = ...) |
| 82 | + evaluate_subscription( |
| 83 | + subscriptions.where(key: key, notifiable_type: nil).first, |
| 84 | + :subscribing?, |
| 85 | + subscribe_as_default |
| 86 | + ) |
| 87 | +end |
| 88 | +``` |
| 89 | + |
| 90 | +Add new method for instance-level subscription check: |
| 91 | + |
| 92 | +```ruby |
| 93 | +def _subscribes_to_notification_for_instance?(key, notifiable, subscribe_as_default = ...) |
| 94 | + instance_sub = subscriptions.where(key: key, notifiable_type: notifiable.class.name, notifiable_id: notifiable.id).first |
| 95 | + instance_sub.present? && instance_sub.subscribing? |
| 96 | +end |
| 97 | +``` |
| 98 | + |
| 99 | +Update `find_subscription` to support optional notifiable: |
| 100 | + |
| 101 | +```ruby |
| 102 | +def find_subscription(key, notifiable: nil) |
| 103 | + if notifiable |
| 104 | + subscriptions.where(key: key, notifiable_type: notifiable.class.name, notifiable_id: notifiable.id).first |
| 105 | + else |
| 106 | + subscriptions.where(key: key, notifiable_type: nil).first |
| 107 | + end |
| 108 | +end |
| 109 | +``` |
| 110 | + |
| 111 | +### 3. Target Concern (`models/concerns/target.rb`) |
| 112 | + |
| 113 | +Update `subscribes_to_notification?` to accept optional notifiable: |
| 114 | + |
| 115 | +```ruby |
| 116 | +def subscribes_to_notification?(key, subscribe_as_default = ..., notifiable: nil) |
| 117 | + return true unless subscription_allowed?(key) |
| 118 | + _subscribes_to_notification?(key, subscribe_as_default) || |
| 119 | + (notifiable.present? && _subscribes_to_notification_for_instance?(key, notifiable, subscribe_as_default)) |
| 120 | +end |
| 121 | +``` |
| 122 | + |
| 123 | +### 4. Notification API (`apis/notification_api.rb`) |
| 124 | + |
| 125 | +#### `generate_notification` - Add instance-level check |
| 126 | + |
| 127 | +```ruby |
| 128 | +def generate_notification(target, notifiable, options = {}) |
| 129 | + key = options[:key] || notifiable.default_notification_key |
| 130 | + if target.subscribes_to_notification?(key, notifiable: notifiable) |
| 131 | + store_notification(target, notifiable, key, options) |
| 132 | + end |
| 133 | +end |
| 134 | +``` |
| 135 | + |
| 136 | +This is the minimal change. The existing `subscribes_to_notification?` check stays in `generate_notification` (not moved to `notify`), and we extend it to also consider instance-level subscriptions. |
| 137 | + |
| 138 | +#### `notify` - Add instance subscription targets |
| 139 | + |
| 140 | +```ruby |
| 141 | +def notify(target_type, notifiable, options = {}) |
| 142 | + if options[:notify_later] |
| 143 | + notify_later(target_type, notifiable, options) |
| 144 | + else |
| 145 | + targets = notifiable.notification_targets(target_type, options[:pass_full_options] ? options : options[:key]) |
| 146 | + # Merge instance subscription targets, deduplicate |
| 147 | + instance_targets = notifiable.instance_subscription_targets(target_type, options[:key]) |
| 148 | + targets = merge_targets(targets, instance_targets) |
| 149 | + unless targets_empty?(targets) |
| 150 | + notify_all(targets, notifiable, options) |
| 151 | + end |
| 152 | + end |
| 153 | +end |
| 154 | +``` |
| 155 | + |
| 156 | +#### New helper: `merge_targets` |
| 157 | + |
| 158 | +```ruby |
| 159 | +def merge_targets(targets, instance_targets) |
| 160 | + return targets if instance_targets.blank? |
| 161 | + # Convert to array for deduplication |
| 162 | + all_targets = targets.respond_to?(:to_a) ? targets.to_a : Array(targets) |
| 163 | + (all_targets + instance_targets).uniq |
| 164 | +end |
| 165 | +``` |
| 166 | + |
| 167 | +### 5. Notifiable Concern (`models/concerns/notifiable.rb`) |
| 168 | + |
| 169 | +Add `instance_subscription_targets`: |
| 170 | + |
| 171 | +```ruby |
| 172 | +def instance_subscription_targets(target_type, key = nil) |
| 173 | + key ||= default_notification_key |
| 174 | + target_class_name = target_type.to_s.to_model_name |
| 175 | + Subscription.where( |
| 176 | + notifiable_type: self.class.name, |
| 177 | + notifiable_id: self.id, |
| 178 | + key: key, |
| 179 | + subscribing: true |
| 180 | + ).where(target_type: target_class_name) |
| 181 | + .map(&:target) |
| 182 | + .compact |
| 183 | +end |
| 184 | +``` |
| 185 | + |
| 186 | +### 6. Subscription API (`apis/subscription_api.rb`) |
| 187 | + |
| 188 | +Add `key_level_only` and `instance_level_only` scopes. No changes to existing subscribe/unsubscribe methods — they work on individual subscription records regardless of whether they're key-level or instance-level. |
| 189 | + |
| 190 | +### 7. Controllers |
| 191 | + |
| 192 | +Update `subscription_params` in `CommonController` to permit `notifiable_type` and `notifiable_id`. |
| 193 | + |
| 194 | +Update `create` action to pass through notifiable params. |
| 195 | + |
| 196 | +Update `find` action to support optional notifiable filtering. |
| 197 | + |
| 198 | +## Async Path (`notify_later`) |
| 199 | + |
| 200 | +The `notify_later` path serializes arguments and delegates to `NotifyJob`, which calls `notify` synchronously. Since our changes are in `notify` and `generate_notification`, the async path is automatically covered — no separate changes needed for `NotifyJob`. |
| 201 | + |
| 202 | +## Migration Template |
| 203 | + |
| 204 | +Update `lib/generators/templates/migrations/migration.rb` to include the new columns and updated index. |
| 205 | + |
| 206 | +Provide a separate migration generator for existing installations: |
| 207 | +`lib/generators/activity_notification/migration/add_notifiable_to_subscriptions_generator.rb` |
| 208 | + |
| 209 | +## Backward Compatibility |
| 210 | + |
| 211 | +- All existing key-level subscriptions have `notifiable_type = NULL` and `notifiable_id = NULL` |
| 212 | +- `_subscribes_to_notification?` filters by `notifiable_type: nil`, so existing behavior is preserved |
| 213 | +- `subscribes_to_notification?` without `notifiable:` parameter returns the same result as before |
| 214 | +- `find_subscription(key)` without `notifiable:` returns key-level subscription as before |
0 commit comments