clean up and stop creating duplication notification policies

fixes #5196

test plan:
 * perform an action that causes lots more than
   User.max_messages_per_day notifications to be sent out
 * in the db, there should only be one entry with notification_id
   is null for that communication channel

Change-Id: Ibd1c5f5273b171bd4ebb6252e942e23dac4aa7e3
Reviewed-on: https://gerrit.instructure.com/9260
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>

app/models/notification.rb

@@ -117,7 +117,9 @@ class Notification < ActiveRecord::Base
     # throttled, so it definitely needs to go to at least one communication
     # channel with 'daily' as the frequency.
     if !policies.any?{|p| p.frequency == 'daily'} && opts[:fallback_channel]
-      policies << NotificationPolicy.new(:communication_channel => opts[:fallback_channel], :frequency => 'daily')
+      fallback_policy = opts[:fallback_channel].notification_policies.by(:daily).find(:first, :conditions => { :notification_id => nil })
+      fallback_policy ||= NotificationPolicy.new(:communication_channel => opts[:fallback_channel], :frequency => 'daily')
+      policies << fallback_policy
     end
 
     return false if (!opts[:fallback_channel] && cc && !cc.active?) || policies.empty? || self.registration?

db/migrate/20120307154947_remove_duplicate_notification_policies.rb

@@ -0,0 +1,7 @@
+class RemoveDuplicateNotificationPolicies < ActiveRecord::Migration
+  tag :postdeploy
+
+  def self.up
+    DataFixup::RemoveDuplicateNotificationPolicies.send_later_if_production(:run)
+  end
+end

lib/data_fixup/remove_duplicate_notification_policies.rb

@@ -0,0 +1,19 @@
+module DataFixup::RemoveDuplicateNotificationPolicies
+  def self.run
+    while true
+      ccs = NotificationPolicy.connection.select_rows("
+          SELECT communication_channel_id
+          FROM notification_policies
+          WHERE notification_id IS NULL
+            AND frequency='daily'
+          GROUP BY communication_channel_id
+          HAVING count(*) > 1 LIMIT 50000")
+      break if ccs.empty?
+      ccs.each do |cc_id|
+        scope = NotificationPolicy.scoped(:conditions => { :communication_channel_id => cc_id, :notification_id => nil, :frequency => 'daily' })
+        keeper = scope.first(:select => "id")
+        scope.delete_all(["id<>?", keeper.id]) if keeper
+      end
+    end
+  end
+end

spec/models/notification_spec.rb

@@ -219,6 +219,7 @@ describe Notification do
     
     it "should not send messages after the user's limit" do
       notification_set
+      NotificationPolicy.count.should == 1
       Rails.cache.delete(['recent_messages_for', @user.id].cache_key)
       User.stubs(:max_messages_per_day).returns(1)
       User.max_messages_per_day.times do 
@@ -229,6 +230,11 @@ describe Notification do
       messages = @notification.create_message(@assignment, @user)
       messages.select{|m| m.to != 'dashboard'}.should be_empty
       DelayedMessage.count.should eql(1)
+      NotificationPolicy.count.should == 2
+      # should not create more dummy policies
+      messages = @notification.create_message(@assignment, @user)
+      messages.select{|m| m.to != 'dashboard'}.should be_empty
+      NotificationPolicy.count.should == 2
     end
     
     it "should not use notification policies for unconfirmed communication channels" do