fix duplicate Notifications (and add unique constraint)

also avoid unnecessary row updates when processing notifications

Change-Id: Ie25b83b72cdf56fa75b6bb415b27f1720ab1bdc1
Reviewed-on: https://gerrit.instructure.com/12540
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>

app/models/attachment.rb

@@ -828,7 +828,7 @@ class Attachment < ActiveRecord::Base
       file_batches.each do |count, attachment_id, last_updated_at, context_id, context_type|
         # clear the need_notify flag for this batch
         Attachment.update_all("need_notify = NULL",
-            ["updated_at <= ? AND context_id = ? AND context_type = ?", last_updated_at, context_id, context_type])
+            ["need_notify AND updated_at <= ? AND context_id = ? AND context_type = ?", last_updated_at, context_id, context_type])
 
         # skip the notification if this batch is too old to be timely
         next if last_updated_at.to_time < discard_older_than

db/migrate/20120717140514_add_file_notifications.rb

@@ -2,11 +2,14 @@ class AddFileNotifications < ActiveRecord::Migration
   tag :predeploy
 
   def self.up
-    Notification.create(:name => "New File Added", :category => "Files")
-    Notification.create(:name => "New Files Added", :category => "Files")
+    # (shard check added later; dupes removed in AddUniqueIndexOnNotifications)
+    return unless Shard.current.default?
+    Notification.create!(:name => "New File Added", :category => "Files")
+    Notification.create!(:name => "New Files Added", :category => "Files")
   end
 
   def self.down
+    # (try on each shard, because there may be duplicates due to the above)
     Notification.find_by_name("New File Added").try(:destroy)
     Notification.find_by_name("New Files Added").try(:destroy)
   end

db/migrate/20120727145851_add_unique_index_on_notifications.rb

@@ -0,0 +1,15 @@
+class AddUniqueIndexOnNotifications < ActiveRecord::Migration
+  tag :postdeploy
+
+  def self.up
+    return unless Shard.current.default?
+    # the excess subquery is necessary to avoid error 1093 on mysql
+    Notification.delete_all("id NOT IN (SELECT * FROM (SELECT MIN(id) FROM notifications GROUP BY name) x)")
+    add_index :notifications, [:name], :unique => true, :name => "index_notifications_unique_on_name"
+  end
+
+  def self.down
+    return unless Shard.current.default?
+    remove_index :notifications, :name => "index_notifications_unique_on_name"
+  end
+end

spec/messages/messages_helper.rb

@@ -18,7 +18,7 @@
 
 def generate_message(notification_name, path_type, asset, options = {})
   raise "options must be a hash!" unless options.is_a? Hash
-  @notification = Notification.create!(:name => notification_name.to_s)
+  @notification = Notification.find_by_name(notification_name.to_s) || Notification.create!(:name => notification_name.to_s)
   user = options[:user]
   asset_context = options[:asset_context]
   data = options[:data] || {}