Commit df03fb0f7c2ce6214ed870a7a8c743173b49d7f2
Committed by
Antonio Terceiro
1 parent
f30fb216
Exists in
master
and in
28 other branches
ActionItem1217: No pending tasks when following link received by e-mail
Showing
4 changed files
with
33 additions
and
9 deletions
Show diff stats
app/models/approve_article.rb
| @@ -68,6 +68,7 @@ class ApproveArticle < Task | @@ -68,6 +68,7 @@ class ApproveArticle < Task | ||
| 68 | end | 68 | end |
| 69 | 69 | ||
| 70 | def target_notification_message | 70 | def target_notification_message |
| 71 | + return nil if target.organization? && !target.moderated_articles? | ||
| 71 | description + "\n\n" + | 72 | description + "\n\n" + |
| 72 | _('You need to login on %{system} in order to approve or reject this article.') % { :system => target.environment.name } | 73 | _('You need to login on %{system} in order to approve or reject this article.') % { :system => target.environment.name } |
| 73 | end | 74 | end |
app/models/task.rb
| @@ -59,7 +59,7 @@ class Task < ActiveRecord::Base | @@ -59,7 +59,7 @@ class Task < ActiveRecord::Base | ||
| 59 | 59 | ||
| 60 | begin | 60 | begin |
| 61 | target_msg = task.target_notification_message | 61 | target_msg = task.target_notification_message |
| 62 | - TaskMailer.deliver_target_notification(task, target_msg) | 62 | + TaskMailer.deliver_target_notification(task, target_msg) if target_msg |
| 63 | rescue NotImplementedError => ex | 63 | rescue NotImplementedError => ex |
| 64 | RAILS_DEFAULT_LOGGER.info ex.to_s | 64 | RAILS_DEFAULT_LOGGER.info ex.to_s |
| 65 | end | 65 | end |
test/unit/approve_article_test.rb
| @@ -2,8 +2,15 @@ require File.dirname(__FILE__) + '/../test_helper' | @@ -2,8 +2,15 @@ require File.dirname(__FILE__) + '/../test_helper' | ||
| 2 | 2 | ||
| 3 | class ApproveArticleTest < ActiveSupport::TestCase | 3 | class ApproveArticleTest < ActiveSupport::TestCase |
| 4 | 4 | ||
| 5 | + def setup | ||
| 6 | + ActionMailer::Base.delivery_method = :test | ||
| 7 | + ActionMailer::Base.perform_deliveries = true | ||
| 8 | + ActionMailer::Base.deliveries = [] | ||
| 9 | + @profile = create_user('test_user').person | ||
| 10 | + end | ||
| 11 | + attr_reader :profile | ||
| 12 | + | ||
| 5 | should 'have name, reference article and profile' do | 13 | should 'have name, reference article and profile' do |
| 6 | - profile = create_user('test_user').person | ||
| 7 | article = profile.articles.create!(:name => 'test article') | 14 | article = profile.articles.create!(:name => 'test article') |
| 8 | 15 | ||
| 9 | a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile) | 16 | a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile) |
| @@ -14,7 +21,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | @@ -14,7 +21,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | ||
| 14 | end | 21 | end |
| 15 | 22 | ||
| 16 | should 'create published article when finished' do | 23 | should 'create published article when finished' do |
| 17 | - profile = create_user('test_user').person | ||
| 18 | article = profile.articles.create!(:name => 'test article') | 24 | article = profile.articles.create!(:name => 'test article') |
| 19 | a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile) | 25 | a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile) |
| 20 | 26 | ||
| @@ -24,7 +30,7 @@ class ApproveArticleTest < ActiveSupport::TestCase | @@ -24,7 +30,7 @@ class ApproveArticleTest < ActiveSupport::TestCase | ||
| 24 | end | 30 | end |
| 25 | 31 | ||
| 26 | should 'override target notification message method from Task' do | 32 | should 'override target notification message method from Task' do |
| 27 | - p1 = create_user('testuser1').person | 33 | + p1 = profile |
| 28 | p2 = create_user('testuser2').person | 34 | p2 = create_user('testuser2').person |
| 29 | task = AddFriend.new(:person => p1, :friend => p2) | 35 | task = AddFriend.new(:person => p1, :friend => p2) |
| 30 | assert_nothing_raised NotImplementedError do | 36 | assert_nothing_raised NotImplementedError do |
| @@ -33,7 +39,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | @@ -33,7 +39,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | ||
| 33 | end | 39 | end |
| 34 | 40 | ||
| 35 | should 'have parent if defined' do | 41 | should 'have parent if defined' do |
| 36 | - profile = create_user('test_user').person | ||
| 37 | article = profile.articles.create!(:name => 'test article') | 42 | article = profile.articles.create!(:name => 'test article') |
| 38 | folder = profile.articles.create!(:name => 'test folder', :type => 'Folder') | 43 | folder = profile.articles.create!(:name => 'test folder', :type => 'Folder') |
| 39 | 44 | ||
| @@ -43,7 +48,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | @@ -43,7 +48,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | ||
| 43 | end | 48 | end |
| 44 | 49 | ||
| 45 | should 'not have parent if not defined' do | 50 | should 'not have parent if not defined' do |
| 46 | - profile = create_user('test_user').person | ||
| 47 | article = profile.articles.create!(:name => 'test article') | 51 | article = profile.articles.create!(:name => 'test article') |
| 48 | 52 | ||
| 49 | a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile) | 53 | a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile) |
| @@ -52,7 +56,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | @@ -52,7 +56,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | ||
| 52 | end | 56 | end |
| 53 | 57 | ||
| 54 | should 'alert when reference article is removed' do | 58 | should 'alert when reference article is removed' do |
| 55 | - profile = create_user('test_user').person | ||
| 56 | article = profile.articles.create!(:name => 'test article') | 59 | article = profile.articles.create!(:name => 'test article') |
| 57 | 60 | ||
| 58 | a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile) | 61 | a = ApproveArticle.create!(:name => 'test name', :article => article, :target => profile, :requestor => profile) |
| @@ -64,7 +67,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | @@ -64,7 +67,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | ||
| 64 | end | 67 | end |
| 65 | 68 | ||
| 66 | should 'preserve article_parent' do | 69 | should 'preserve article_parent' do |
| 67 | - profile = create_user('test_user').person | ||
| 68 | article = profile.articles.create!(:name => 'test article') | 70 | article = profile.articles.create!(:name => 'test article') |
| 69 | a = ApproveArticle.new(:article_parent => article) | 71 | a = ApproveArticle.new(:article_parent => article) |
| 70 | 72 | ||
| @@ -72,7 +74,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | @@ -72,7 +74,6 @@ class ApproveArticleTest < ActiveSupport::TestCase | ||
| 72 | end | 74 | end |
| 73 | 75 | ||
| 74 | should 'handle blank names' do | 76 | should 'handle blank names' do |
| 75 | - profile = create_user('test_user').person | ||
| 76 | article = profile.articles.create!(:name => 'test article') | 77 | article = profile.articles.create!(:name => 'test article') |
| 77 | community = Community.create!(:name => 'test comm') | 78 | community = Community.create!(:name => 'test comm') |
| 78 | a = ApproveArticle.create!(:name => '', :article => article, :target => community, :requestor => profile) | 79 | a = ApproveArticle.create!(:name => '', :article => article, :target => community, :requestor => profile) |
| @@ -81,4 +82,19 @@ class ApproveArticleTest < ActiveSupport::TestCase | @@ -81,4 +82,19 @@ class ApproveArticleTest < ActiveSupport::TestCase | ||
| 81 | a.finish | 82 | a.finish |
| 82 | end | 83 | end |
| 83 | end | 84 | end |
| 85 | + | ||
| 86 | + should 'notify target if group is moderated' do | ||
| 87 | + article = profile.articles.create!(:name => 'test article') | ||
| 88 | + community = Community.create!(:name => 'test comm', :moderated_articles => true) | ||
| 89 | + a = ApproveArticle.create!(:name => '', :article => article, :target => community, :requestor => profile) | ||
| 90 | + assert !ActionMailer::Base.deliveries.empty? | ||
| 91 | + end | ||
| 92 | + | ||
| 93 | + should 'not notify target if group is not moderated' do | ||
| 94 | + article = profile.articles.create!(:name => 'test article') | ||
| 95 | + community = Community.create!(:name => 'test comm', :moderated_articles => false) | ||
| 96 | + a = ApproveArticle.create!(:name => '', :article => article, :target => community, :requestor => profile) | ||
| 97 | + assert ActionMailer::Base.deliveries.empty? | ||
| 98 | + end | ||
| 99 | + | ||
| 84 | end | 100 | end |
test/unit/task_test.rb
| @@ -207,6 +207,13 @@ class TaskTest < Test::Unit::TestCase | @@ -207,6 +207,13 @@ class TaskTest < Test::Unit::TestCase | ||
| 207 | end | 207 | end |
| 208 | end | 208 | end |
| 209 | 209 | ||
| 210 | + should 'not notify target if message is nil' do | ||
| 211 | + task = Task.new | ||
| 212 | + task.stubs(:target_notification_message).returns(nil) | ||
| 213 | + TaskMailer.expects(:deliver_target_notification).never | ||
| 214 | + task.save! | ||
| 215 | + end | ||
| 216 | + | ||
| 210 | protected | 217 | protected |
| 211 | 218 | ||
| 212 | def sample_user | 219 | def sample_user |