Commit ec22524b445e88e7574daf191f0ebfb142d6d3a5
1 parent
35366ee7
Exists in
master
and in
29 other branches
ActionItem14: modularizing notification sending to check for an existing
requestor. Also extended the system to allow other types of notification just by defining a message with the proper name git-svn-id: https://svn.colivre.coop.br/svn/noosfero/trunk@685 3f533792-8f58-4932-b0fe-aaf55b0a4547
Showing
4 changed files
with
53 additions
and
32 deletions
Show diff stats
app/models/task.rb
| @@ -45,7 +45,7 @@ class Task < ActiveRecord::Base | @@ -45,7 +45,7 @@ class Task < ActiveRecord::Base | ||
| 45 | end | 45 | end |
| 46 | 46 | ||
| 47 | after_create do |task| | 47 | after_create do |task| |
| 48 | - TaskMailer.deliver_task_created(task) | 48 | + task.send(:send_notification, :created) |
| 49 | end | 49 | end |
| 50 | 50 | ||
| 51 | # this method finished the task. It calls #perform, which must be overriden | 51 | # this method finished the task. It calls #perform, which must be overriden |
| @@ -57,7 +57,7 @@ class Task < ActiveRecord::Base | @@ -57,7 +57,7 @@ class Task < ActiveRecord::Base | ||
| 57 | self.end_date = Time.now | 57 | self.end_date = Time.now |
| 58 | self.save! | 58 | self.save! |
| 59 | self.perform | 59 | self.perform |
| 60 | - TaskMailer.deliver_task_finished(self) | 60 | + send_notification(:finished) |
| 61 | end | 61 | end |
| 62 | end | 62 | end |
| 63 | 63 | ||
| @@ -68,36 +68,38 @@ class Task < ActiveRecord::Base | @@ -68,36 +68,38 @@ class Task < ActiveRecord::Base | ||
| 68 | self.status = Task::Status::CANCELLED | 68 | self.status = Task::Status::CANCELLED |
| 69 | self.end_date = Time.now | 69 | self.end_date = Time.now |
| 70 | self.save! | 70 | self.save! |
| 71 | - TaskMailer.deliver_task_cancelled(self) | 71 | + send_notification(:cancelled) |
| 72 | end | 72 | end |
| 73 | end | 73 | end |
| 74 | 74 | ||
| 75 | + | ||
| 76 | + # Returns the description of the task. | ||
| 77 | + # | ||
| 78 | + # This method +must+ be overriden in subclasses to return something | ||
| 79 | + # meaningful for each kind of task | ||
| 80 | + def description | ||
| 81 | + _('Generic task') | ||
| 82 | + end | ||
| 83 | + | ||
| 75 | # The message that will be sent to the requestor of the task when the task is | 84 | # The message that will be sent to the requestor of the task when the task is |
| 76 | # created. | 85 | # created. |
| 77 | - def create_message | 86 | + def task_created_message |
| 87 | + # FIXME: use a date properly recorded. | ||
| 78 | _("The task was created at %s") % Time.now | 88 | _("The task was created at %s") % Time.now |
| 79 | end | 89 | end |
| 80 | 90 | ||
| 81 | # The message that will be sent to the requestor of the task when its | 91 | # The message that will be sent to the requestor of the task when its |
| 82 | # finished. | 92 | # finished. |
| 83 | - def finish_message | 93 | + def task_finished_message |
| 84 | _("The task was finished at %s") % (self.end_date.to_s) | 94 | _("The task was finished at %s") % (self.end_date.to_s) |
| 85 | end | 95 | end |
| 86 | 96 | ||
| 87 | # The message that will be sent to the requestor of the task when its | 97 | # The message that will be sent to the requestor of the task when its |
| 88 | # cancelled. | 98 | # cancelled. |
| 89 | - def cancel_message | 99 | + def task_cancelled_message |
| 90 | _("The task was cancelled at %s") % (self.end_date.to_s) | 100 | _("The task was cancelled at %s") % (self.end_date.to_s) |
| 91 | end | 101 | end |
| 92 | 102 | ||
| 93 | - # Returns the description of the task. | ||
| 94 | - # | ||
| 95 | - # This method +must+ be overriden in subclasses to return something | ||
| 96 | - # meaningful for each kind of task | ||
| 97 | - def description | ||
| 98 | - _('Generic task') | ||
| 99 | - end | ||
| 100 | - | ||
| 101 | protected | 103 | protected |
| 102 | 104 | ||
| 103 | # This method must be overrided in subclasses, and its implementation must do | 105 | # This method must be overrided in subclasses, and its implementation must do |
| @@ -110,6 +112,11 @@ class Task < ActiveRecord::Base | @@ -110,6 +112,11 @@ class Task < ActiveRecord::Base | ||
| 110 | def perform | 112 | def perform |
| 111 | end | 113 | end |
| 112 | 114 | ||
| 115 | + # sends notification e-mail about a task, if the task has a requestor. | ||
| 116 | + def send_notification(action) | ||
| 117 | + TaskMailer.send("deliver_task_#{action}", self) if self.requestor | ||
| 118 | + end | ||
| 119 | + | ||
| 113 | class << self | 120 | class << self |
| 114 | 121 | ||
| 115 | # generates a random code string consisting of 36 characters in the ranges | 122 | # generates a random code string consisting of 36 characters in the ranges |
app/models/task_mailer.rb
| 1 | class TaskMailer < ActionMailer::Base | 1 | class TaskMailer < ActionMailer::Base |
| 2 | 2 | ||
| 3 | - def task_finished(task) | ||
| 4 | - send_message(task, task.finish_message) | ||
| 5 | - end | ||
| 6 | - | ||
| 7 | - def task_created(task) | ||
| 8 | - send_message(task, task.create_message) | ||
| 9 | - end | ||
| 10 | - | ||
| 11 | - def task_cancelled(task) | ||
| 12 | - send_message(task, task.cancel_message) | 3 | + def method_missing(name, *args) |
| 4 | + task = args.shift | ||
| 5 | + if task.kind_of?(Task) && task.respond_to?("#{name}_message") | ||
| 6 | + send_message(task, task.send("#{name}_message"), *args) | ||
| 7 | + else | ||
| 8 | + super | ||
| 9 | + end | ||
| 13 | end | 10 | end |
| 14 | 11 | ||
| 15 | protected | 12 | protected |
test/unit/task_mailer_test.rb
| @@ -18,8 +18,8 @@ class TaskMailerTest < Test::Unit::TestCase | @@ -18,8 +18,8 @@ class TaskMailerTest < Test::Unit::TestCase | ||
| 18 | 18 | ||
| 19 | should 'be able to send a "task finished" message' do | 19 | should 'be able to send a "task finished" message' do |
| 20 | 20 | ||
| 21 | - task = mock() | ||
| 22 | - task.expects(:finish_message).returns('the message') | 21 | + task = Task.new |
| 22 | + task.expects(:task_finished_message).returns('the message') | ||
| 23 | task.expects(:description).returns('the task') | 23 | task.expects(:description).returns('the task') |
| 24 | 24 | ||
| 25 | requestor = mock() | 25 | requestor = mock() |
| @@ -39,8 +39,8 @@ class TaskMailerTest < Test::Unit::TestCase | @@ -39,8 +39,8 @@ class TaskMailerTest < Test::Unit::TestCase | ||
| 39 | 39 | ||
| 40 | should 'be able to send a "task cancelled" message' do | 40 | should 'be able to send a "task cancelled" message' do |
| 41 | 41 | ||
| 42 | - task = mock() | ||
| 43 | - task.expects(:cancel_message).returns('the message') | 42 | + task = Task.new |
| 43 | + task.expects(:task_cancelled_message).returns('the message') | ||
| 44 | task.expects(:description).returns('the task') | 44 | task.expects(:description).returns('the task') |
| 45 | 45 | ||
| 46 | requestor = mock() | 46 | requestor = mock() |
| @@ -60,8 +60,9 @@ class TaskMailerTest < Test::Unit::TestCase | @@ -60,8 +60,9 @@ class TaskMailerTest < Test::Unit::TestCase | ||
| 60 | 60 | ||
| 61 | should 'be able to send a "task created" message' do | 61 | should 'be able to send a "task created" message' do |
| 62 | 62 | ||
| 63 | - task = mock() | ||
| 64 | - task.expects(:create_message).returns('the message') | 63 | + task = Task.new |
| 64 | + | ||
| 65 | + task.expects(:task_created_message).returns('the message') | ||
| 65 | task.expects(:description).returns('the task') | 66 | task.expects(:description).returns('the task') |
| 66 | 67 | ||
| 67 | requestor = mock() | 68 | requestor = mock() |
test/unit/task_test.rb
| @@ -33,6 +33,7 @@ class TaskTest < Test::Unit::TestCase | @@ -33,6 +33,7 @@ class TaskTest < Test::Unit::TestCase | ||
| 33 | def test_should_call_perform_in_finish | 33 | def test_should_call_perform_in_finish |
| 34 | TaskMailer.expects(:deliver_task_finished) | 34 | TaskMailer.expects(:deliver_task_finished) |
| 35 | t = Task.create | 35 | t = Task.create |
| 36 | + t.requestor = sample_user | ||
| 36 | t.expects(:perform) | 37 | t.expects(:perform) |
| 37 | t.finish | 38 | t.finish |
| 38 | assert_equal Task::Status::FINISHED, t.status | 39 | assert_equal Task::Status::FINISHED, t.status |
| @@ -41,6 +42,7 @@ class TaskTest < Test::Unit::TestCase | @@ -41,6 +42,7 @@ class TaskTest < Test::Unit::TestCase | ||
| 41 | def test_should_have_cancelled_status_after_cancel | 42 | def test_should_have_cancelled_status_after_cancel |
| 42 | TaskMailer.expects(:deliver_task_cancelled) | 43 | TaskMailer.expects(:deliver_task_cancelled) |
| 43 | t = Task.create | 44 | t = Task.create |
| 45 | + t.requestor = sample_user | ||
| 44 | t.cancel | 46 | t.cancel |
| 45 | assert_equal Task::Status::CANCELLED, t.status | 47 | assert_equal Task::Status::CANCELLED, t.status |
| 46 | end | 48 | end |
| @@ -52,13 +54,19 @@ class TaskTest < Test::Unit::TestCase | @@ -52,13 +54,19 @@ class TaskTest < Test::Unit::TestCase | ||
| 52 | 54 | ||
| 53 | def test_should_notify_finish | 55 | def test_should_notify_finish |
| 54 | t = Task.create | 56 | t = Task.create |
| 57 | + t.requestor = sample_user | ||
| 58 | + | ||
| 55 | TaskMailer.expects(:deliver_task_finished).with(t) | 59 | TaskMailer.expects(:deliver_task_finished).with(t) |
| 60 | + | ||
| 56 | t.finish | 61 | t.finish |
| 57 | end | 62 | end |
| 58 | 63 | ||
| 59 | def test_should_notify_cancel | 64 | def test_should_notify_cancel |
| 60 | t = Task.create | 65 | t = Task.create |
| 66 | + t.requestor = sample_user | ||
| 67 | + | ||
| 61 | TaskMailer.expects(:deliver_task_cancelled).with(t) | 68 | TaskMailer.expects(:deliver_task_cancelled).with(t) |
| 69 | + | ||
| 62 | t.cancel | 70 | t.cancel |
| 63 | end | 71 | end |
| 64 | 72 | ||
| @@ -84,6 +92,8 @@ class TaskTest < Test::Unit::TestCase | @@ -84,6 +92,8 @@ class TaskTest < Test::Unit::TestCase | ||
| 84 | 92 | ||
| 85 | should 'notify just after the task is created' do | 93 | should 'notify just after the task is created' do |
| 86 | task = Task.new | 94 | task = Task.new |
| 95 | + task.requestor = sample_user | ||
| 96 | + | ||
| 87 | TaskMailer.expects(:deliver_task_created).with(task) | 97 | TaskMailer.expects(:deliver_task_created).with(task) |
| 88 | task.save! | 98 | task.save! |
| 89 | end | 99 | end |
| @@ -109,7 +119,7 @@ class TaskTest < Test::Unit::TestCase | @@ -109,7 +119,7 @@ class TaskTest < Test::Unit::TestCase | ||
| 109 | 119 | ||
| 110 | should 'find only in active tasks' do | 120 | should 'find only in active tasks' do |
| 111 | task = Task.new | 121 | task = Task.new |
| 112 | - task.requestor = User.create(:login => 'testfindinactivetask', :password => 'test', :password_confirmation => 'test', :email => 'testfindinactivetask@localhost.localdomain').person | 122 | + task.requestor = sample_user |
| 113 | task.save! | 123 | task.save! |
| 114 | 124 | ||
| 115 | task.cancel | 125 | task.cancel |
| @@ -119,10 +129,16 @@ class TaskTest < Test::Unit::TestCase | @@ -119,10 +129,16 @@ class TaskTest < Test::Unit::TestCase | ||
| 119 | 129 | ||
| 120 | should 'be able to find active tasks ' do | 130 | should 'be able to find active tasks ' do |
| 121 | task = Task.new | 131 | task = Task.new |
| 122 | - task.requestor = User.create(:login => 'testfindinactivetask', :password => 'test', :password_confirmation => 'test', :email => 'testfindinactivetask@localhost.localdomain').person | 132 | + task.requestor = sample_user |
| 123 | task.save! | 133 | task.save! |
| 124 | 134 | ||
| 125 | assert_not_nil Task.find_by_code(task.code) | 135 | assert_not_nil Task.find_by_code(task.code) |
| 126 | end | 136 | end |
| 127 | 137 | ||
| 138 | + protected | ||
| 139 | + | ||
| 140 | + def sample_user | ||
| 141 | + User.create(:login => 'testfindinactivetask', :password => 'test', :password_confirmation => 'test', :email => 'testfindinactivetask@localhost.localdomain').person | ||
| 142 | + end | ||
| 143 | + | ||
| 128 | end | 144 | end |