Commit e0b80758fa43ac11ad29237fac016faa78b48059

Authored by Victor Costa
2 parents afb081cb dde55d70

Merge branch 'fix_person_notifier' into stable

Conflicts:
	app/models/person_notifier.rb
	test/unit/person_notifier_test.rb
app/helpers/person_notifier_helper.rb
... ... @@ -1,15 +0,0 @@
1   -module PersonNotifierHelper
2   -
3   - include ApplicationHelper
4   -
5   - private
6   -
7   - def path_to_image(source)
8   - top_url + source
9   - end
10   -
11   - def top_url
12   - top_url = @profile.environment ? @profile.environment.top_url : ''
13   - end
14   -
15   -end
app/models/person_notifier.rb
... ... @@ -58,17 +58,28 @@ class PersonNotifier
58 58 Person.find(person_id).notifier.notify
59 59 end
60 60  
  61 + def failure(job)
  62 + begin
  63 + person = Person.find(person_id)
  64 + person.notifier.dispatch_notification_mail
  65 + rescue
  66 + Rails.logger.error "PersonNotifier::NotifyJob: Cannot recover from failure"
  67 + end
  68 + end
  69 +
61 70 end
62 71  
63 72 class Mailer < ActionMailer::Base
64 73  
65   - add_template_helper(PersonNotifierHelper)
  74 + add_template_helper(ApplicationHelper)
66 75  
67 76 def session
68 77 {:theme => nil}
69 78 end
70 79  
71 80 def content_summary(person, notifications)
  81 + ActionMailer::Base.asset_host = person.environment.top_url if person.environment
  82 +
72 83 @current_theme = 'default'
73 84 @profile = person
74 85 @recipient = @profile.nickname || @profile.name
... ...
test/unit/person_notifier_helper_test.rb
... ... @@ -1,23 +0,0 @@
1   -require File.dirname(__FILE__) + '/../test_helper'
2   -
3   -class PersonNotifierHelperTest < ActionView::TestCase
4   -
5   - include PersonNotifierHelper
6   -
7   - def setup
8   - @profile = mock
9   - @env = Environment.new
10   - end
11   - attr_reader :profile, :env
12   -
13   - should 'append top url of environment at image path' do
14   - profile.expects(:environment).returns(env).at_least_once
15   - assert_match /src="http:\/\/localhost\/image.png"/, image_tag("/image.png")
16   - end
17   -
18   - should 'return original path if do not have an environment' do
19   - profile.expects(:environment).returns(nil).at_least_once
20   - assert_match /src="\/image.png"/, image_tag("/image.png")
21   - end
22   -
23   -end
test/unit/person_notifier_test.rb
... ... @@ -214,6 +214,29 @@ class PersonNotifierTest &lt; ActiveSupport::TestCase
214 214 assert_equal 1, Delayed::Job.count
215 215 end
216 216  
  217 + should 'NotifyJob failed jobs create a new NotifyJob on failure' do
  218 + Delayed::Worker.max_attempts = 1
  219 + Delayed::Job.enqueue(PersonNotifier::NotifyJob.new(@member.id))
  220 +
  221 + PersonNotifier.any_instance.stubs(:notify).raises('error')
  222 +
  223 + process_delayed_job_queue
  224 + jobs = PersonNotifier::NotifyJob.find(@member.id)
  225 + assert !jobs.select {|j| !j.failed? && j.last_error.nil? }.empty?
  226 + end
  227 +
  228 + should 'do not raise errors in NotifyJob failure to avoid loop' do
  229 + Delayed::Worker.max_attempts = 1
  230 + Delayed::Job.enqueue(PersonNotifier::NotifyJob.new(@member.id))
  231 +
  232 + PersonNotifier.any_instance.stubs(:notify).raises('error')
  233 + PersonNotifier.any_instance.stubs(:dispatch_notification_mail).raises('error')
  234 +
  235 + process_delayed_job_queue
  236 + jobs = PersonNotifier::NotifyJob.find(@member.id)
  237 + assert jobs.select {|j| !j.failed? && j.last_error.nil? }.empty?
  238 + end
  239 +
217 240 private
218 241  
219 242 def notify
... ...