From e46d23ce9ab2d42db5119253b039f871e1dbef19 Mon Sep 17 00:00:00 2001
From: Rodrigo Souto <rodrigo@colivre.coop.br>
Date: Thu, 9 Jul 2015 16:13:48 -0300
Subject: [PATCH] proper validations for approve_article task

---
 app/models/approve_article.rb     | 14 ++++++++++++--
 test/unit/approve_article_test.rb | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/app/models/approve_article.rb b/app/models/approve_article.rb
index f2f8a9f..8c2d517 100644
--- a/app/models/approve_article.rb
+++ b/app/models/approve_article.rb
@@ -2,8 +2,18 @@ class ApproveArticle < Task
   validates_presence_of :requestor_id, :target_id
 
   validates :requestor, kind_of: {kind: Person}
-  #validates :target, kind_of: {kind: Organization}
-  #validate :request_is_member_of_target
+  validate :allowed_requestor
+
+  def allowed_requestor
+    if target
+      if target.person? && requestor != target
+        self.errors.add(:requestor, _('You can not post articles to other users.'))
+      end
+      if target.organization? && !target.members.include?(requestor) && target.environment.portal_community != target
+        self.errors.add(:requestor, _('Only members can post articles on communities.'))
+      end
+    end
+  end
 
   def article_title
     article ? article.title : _('(The original text was removed)')
diff --git a/test/unit/approve_article_test.rb b/test/unit/approve_article_test.rb
index d7a32ec..9b55e93 100644
--- a/test/unit/approve_article_test.rb
+++ b/test/unit/approve_article_test.rb
@@ -9,6 +9,7 @@ class ApproveArticleTest < ActiveSupport::TestCase
     @profile = create_user('test_user').person
     @article = fast_create(TextileArticle, :profile_id => @profile.id, :name => 'test name', :abstract => 'Lead of article', :body => 'This is my article')
     @community = fast_create(Community)
+    @community.add_member(@profile)
   end
   attr_reader :profile, :article, :community
 
@@ -251,6 +252,8 @@ class ApproveArticleTest < ActiveSupport::TestCase
   end
 
   should 'not group trackers activity of article\'s creation' do
+    other_community = fast_create(Community)
+    other_community.add_member(profile)
     ActionTracker::Record.delete_all
 
     article = fast_create(TextileArticle)
@@ -262,20 +265,20 @@ class ApproveArticleTest < ActiveSupport::TestCase
     a.finish
 
     article = fast_create(TextileArticle)
-    other_community = fast_create(Community)
     a = create(ApproveArticle, :name => 'another bar', :article => article, :target => other_community, :requestor => profile)
     a.finish
     assert_equal 3, ActionTracker::Record.count
   end
 
   should 'not create trackers activity when updating articles' do
+    other_community = fast_create(Community)
+    other_community.add_member(profile)
     ActionTracker::Record.delete_all
     article1 = fast_create(TextileArticle)
     a = create(ApproveArticle, :name => 'bar', :article => article1, :target => community, :requestor => profile)
     a.finish
 
     article2 = fast_create(TinyMceArticle)
-    other_community = fast_create(Community)
     a = create(ApproveArticle, :name => 'another bar', :article => article2, :target => other_community, :requestor => profile)
     a.finish
     assert_equal 2, ActionTracker::Record.count
@@ -283,7 +286,7 @@ class ApproveArticleTest < ActiveSupport::TestCase
     assert_no_difference 'ActionTracker::Record.count' do
       published = article1.class.last
       published.name = 'foo';published.save!
-  
+
       published = article2.class.last
       published.name = 'another foo';published.save!
     end
@@ -307,7 +310,7 @@ class ApproveArticleTest < ActiveSupport::TestCase
     person = fast_create(Person)
     person.stubs(:notification_emails).returns(['target@example.org'])
 
-    a = create(ApproveArticle, :article => article, :target => person, :requestor => profile)
+    a = create(ApproveArticle, :article => article, :target => person, :requestor => person)
     a.finish
 
     approved_article = person.articles.find_by_name(article.name)
@@ -440,4 +443,47 @@ class ApproveArticleTest < ActiveSupport::TestCase
     assert_equal article, LinkArticle.last.reference_article
   end
 
+  should 'not allow non-person requestor' do
+    task = ApproveArticle.new(:requestor => Community.new)
+    task.valid?
+    assert task.invalid?(:requestor)
+  end
+
+  should 'allow only self requestors when the target is a person' do
+    person = fast_create(Person)
+    another_person = fast_create(Person)
+
+    t1 = ApproveArticle.new(:requestor => person, :target => person)
+    t2 = ApproveArticle.new(:requestor => another_person, :target => person)
+
+    assert t1.valid?
+    assert !t2.valid?
+    assert t2.invalid?(:requestor)
+  end
+
+  should 'allow only members to be requestors when target is a community' do
+    community = fast_create(Community)
+    member = fast_create(Person)
+    community.add_member(member)
+    non_member = fast_create(Person)
+
+    t1 = ApproveArticle.new(:requestor => member, :target => community)
+    t2 = ApproveArticle.new(:requestor => non_member, :target => community)
+
+    assert t1.valid?
+    assert !t2.valid?
+    assert t2.invalid?(:requestor)
+  end
+
+  should 'allow any user to be requestor whe the target is the portal community' do
+    community = fast_create(Community)
+    environment = community.environment
+    environment.portal_community = community
+    environment.save!
+    person = fast_create(Person)
+
+    task = ApproveArticle.new(:requestor => person, :target => community)
+
+    assert task.valid?
+  end
 end
--
libgit2 0.21.2