From f63077aa7f529fafe7b2350795645355bf8a8516 Mon Sep 17 00:00:00 2001 From: Jon Jensen Date: Fri, 2 Mar 2012 14:42:23 -0700 Subject: [PATCH] cache notifications in memory test plan: * use canvas and ensure that notifications work Change-Id: Ie46f049aab5dde7167c3ea4228e503c7a686c839 Reviewed-on: https://gerrit.instructure.com/9154 Tested-by: Hudson Reviewed-by: Cody Cutrer --- app/models/alert.rb | 2 +- app/models/delayed_message.rb | 2 +- app/models/notification.rb | 16 ++++++++++++++- lib/canvas/account_reports.rb | 4 ++-- spec/models/notification_spec.rb | 20 +++++++++++++++++++ spec/spec_helper.rb | 2 ++ .../broadcast_policy/lib/broadcast_policy.rb | 3 +-- 7 files changed, 42 insertions(+), 7 deletions(-) diff --git a/app/models/alert.rb b/app/models/alert.rb index eecb6dc2504..e5462370ade 100644 --- a/app/models/alert.rb +++ b/app/models/alert.rb @@ -255,7 +255,7 @@ class Alert < ActiveRecord::Base end def self.send_alert(alert, user_ids, student_enrollment) - notification = Notification.find_by_name("Alert") + notification = Notification.by_name("Alert") notification.create_message(alert, user_ids, {:asset_context => student_enrollment}) end end diff --git a/app/models/delayed_message.rb b/app/models/delayed_message.rb index 438e3c6ecd6..9afc4be0394 100644 --- a/app/models/delayed_message.rb +++ b/app/models/delayed_message.rb @@ -112,7 +112,7 @@ class DelayedMessage < ActiveRecord::Base user = to.user rescue nil context = delayed_messages.select{|m| m.context}.compact.first.try(:context) return nil unless context # the context for this message has already been deleted - notification = Notification.find_by_name('Summaries') + notification = Notification.by_name('Summaries') path = HostUrl.outgoing_email_address message = notification.messages.build( :subject => notification.subject, diff --git a/app/models/notification.rb b/app/models/notification.rb index 019333da01d..70b2637c67f 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -74,7 +74,21 @@ class Notification < ActiveRecord::Base end def self.summary_notification - find_by_name('Summaries') + by_name('Summaries') + end + + def self.by_name(name) + @notifications ||= Notification.all.inject({}){ |h, n| h[n.name] = n; h } + if notification = @notifications[name] + copy = notification.clone + copy.id = notification.id + copy.send(:remove_instance_variable, :@new_record) + copy + end + end + + def self.reset_cache! + @notifications = nil end def infer_default_content diff --git a/lib/canvas/account_reports.rb b/lib/canvas/account_reports.rb index 58e404ee73d..c771e9c025f 100644 --- a/lib/canvas/account_reports.rb +++ b/lib/canvas/account_reports.rb @@ -60,8 +60,8 @@ module Canvas::AccountReports def self.message_recipient(account_report, message, csv=nil) user = account_report.user account = account_report.account - notification = Notification.find_by_name("Report Generated") - notification = Notification.find_by_name("Report Generation Failed") if !csv + notification = Notification.by_name("Report Generated") + notification = Notification.by_name("Report Generation Failed") if !csv attachment = nil if csv require 'action_controller' diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 4d811272d13..c8f9aaf56a1 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -44,6 +44,26 @@ describe Notification do n.subject.should_not be_nil n.sms_body.should_not be_nil end + + context "by_name" do + before do + Notification.create(:name => "foo") + Notification.create(:name => "bar") + end + + it "should look up all notifications once and cache them thereafter" do + Notification.expects(:all).once.returns{ Notification.find(:all) } + Notification.by_name("foo").should eql(Notification.find_by_name("foo")) + Notification.by_name("bar").should eql(Notification.find_by_name("bar")) + end + + it "should give you different object for the same notification" do + n1 = Notification.by_name("foo") + n2 = Notification.by_name("foo") + n1.should eql n2 + n1.should_not equal n2 + end + end context "create_message" do it "should only send dashboard messages for users with non-validated channels" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d42c4715bb9..0e4e2ac7878 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -97,6 +97,7 @@ Spec::Runner.configure do |config| config.before :all do # so before(:all)'s don't get confused Account.clear_special_account_cache! + Notification.after_create { Notification.reset_cache! } end config.before :each do @@ -105,6 +106,7 @@ Spec::Runner.configure do |config| Account.default.update_attribute(:default_time_zone, 'UTC') Setting.reset_cache! HostUrl.reset_cache! + Notification.reset_cache! ActiveRecord::Base.reset_any_instantiation! end diff --git a/vendor/plugins/broadcast_policy/lib/broadcast_policy.rb b/vendor/plugins/broadcast_policy/lib/broadcast_policy.rb index 2b8602a1985..9df61f86443 100644 --- a/vendor/plugins/broadcast_policy/lib/broadcast_policy.rb +++ b/vendor/plugins/broadcast_policy/lib/broadcast_policy.rb @@ -126,8 +126,7 @@ module Instructure #:nodoc: record.messages_failed[self.dispatch] = "Did not meet condition." return false end - notification = record.notifications.find_by_name(self.dispatch) rescue nil - notification ||= Notification.find_by_name(self.dispatch) + notification = Notification.by_name(self.dispatch) # logger.warn "Could not find notification for #{record.inspect}" unless notification unless notification record.messages_failed[self.dispatch] = "Could not find notification: #{self.dispatch}."