diff options
author | Sean McGivern <sean@gitlab.com> | 2017-06-08 19:10:35 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2017-06-15 17:15:13 +0300 |
commit | e94c1028c1e829f899b0d0f56313cc62df6f9a0a (patch) | |
tree | d69f40f69e1ed69ce5c2c6d7fe2367ccca7cdce1 /app | |
parent | f4b5fcbca100769b89e6b06e0df180012d16a7a8 (diff) |
Deserialise existing custom notification settings
Create a post-deployment migration to update all existing notification settings
with at least one custom level enabled to the new format. Also handle the same
conversion when updating settings, to catch any stragglers.
Diffstat (limited to 'app')
-rw-r--r-- | app/models/notification_setting.rb | 26 | ||||
-rw-r--r-- | app/services/notification_recipient_service.rb | 8 | ||||
-rw-r--r-- | app/services/notification_service.rb | 2 |
3 files changed, 29 insertions, 7 deletions
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index 2a53484f96f..b0df7aeb323 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -42,6 +42,7 @@ class NotificationSetting < ActiveRecord::Base ].freeze store :events, coder: JSON + before_save :convert_events def self.find_or_create_for(source) setting = find_or_initialize_by(source: source) @@ -53,21 +54,42 @@ class NotificationSetting < ActiveRecord::Base setting end - EMAIL_EVENTS.each do |event| + # 1. Check if this event has a value stored in its database column. + # 2. If it does, return that value. + # 3. If it doesn't (the value is nil), return the value from the serialized + # JSON hash in `events`. + (EMAIL_EVENTS - [:failed_pipeline]).each do |event| define_method(event) do bool = super() bool.nil? ? !!events[event] : bool end + + alias_method :"#{event}?", event end # Allow people to receive failed pipeline notifications if they already have # custom notifications enabled, as these are more like mentions than the other # custom settings. def failed_pipeline - bool = read_attribute(:failed_pipeline) + bool = super bool = events[:failed_pipeline] if bool.nil? bool.nil? || bool end + alias_method :failed_pipeline?, :failed_pipeline + + def event_enabled?(event) + respond_to?(event) && public_send(event) + end + + def convert_events + return if events_before_type_cast.nil? + + EMAIL_EVENTS.each do |event| + write_attribute(event, public_send(event)) + end + + write_attribute(:events, nil) + end end diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 988bd0a7cdb..8d1820bc504 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -8,7 +8,7 @@ class NotificationRecipientService @project = project end - def build_recipients(target, current_user, action: nil, previous_assignee: nil, skip_current_user: true) + def build_recipients(target, current_user, action:, previous_assignee: nil, skip_current_user: true) custom_action = build_custom_key(action, target) recipients = target.participants(current_user) @@ -59,7 +59,7 @@ class NotificationRecipientService return [] if notification_setting.mention? || notification_setting.disabled? - return [] if notification_setting.custom? && !notification_setting.public_send(custom_action) + return [] if notification_setting.custom? && !notification_setting.event_enabled?(custom_action) return [] if (notification_setting.watch? || notification_setting.participating?) && NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(custom_action) @@ -176,7 +176,7 @@ class NotificationRecipientService if notification_level settings = resource.notification_settings.where(level: NotificationSetting.levels[notification_level]) - settings = settings.select { |setting| setting.events[action] } if action.present? + settings = settings.select { |setting| setting.event_enabled?(action) } if action.present? settings.map(&:user_id) else resource.notification_settings.pluck(:user_id) @@ -225,7 +225,7 @@ class NotificationRecipientService def user_ids_with_global_level_custom(ids, action) settings = settings_with_global_level_of(:custom, ids) - settings = settings.select { |setting| setting.events[action] } + settings = settings.select { |setting| setting.event_enabled?(action) } settings.map(&:user_id) end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 646ccbdb2bf..3a98a5f6b64 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -273,7 +273,7 @@ class NotificationService end def issue_moved(issue, new_issue, current_user) - recipients = NotificationRecipientService.new(issue.project).build_recipients(issue, current_user) + recipients = NotificationRecipientService.new(issue.project).build_recipients(issue, current_user, action: 'moved') recipients.map do |recipient| email = mailer.issue_moved_email(recipient, issue, new_issue, current_user) |