Commit e46d23ce9ab2d42db5119253b039f871e1dbef19
1 parent
ed0554c6
Exists in
api_private_token
and in
2 other branches
proper validations for approve_article task
Showing
2 changed files
with
62 additions
and
6 deletions
Show diff stats
app/models/approve_article.rb
... | ... | @@ -2,8 +2,18 @@ class ApproveArticle < Task |
2 | 2 | validates_presence_of :requestor_id, :target_id |
3 | 3 | |
4 | 4 | validates :requestor, kind_of: {kind: Person} |
5 | - #validates :target, kind_of: {kind: Organization} | |
6 | - #validate :request_is_member_of_target | |
5 | + validate :allowed_requestor | |
6 | + | |
7 | + def allowed_requestor | |
8 | + if target | |
9 | + if target.person? && requestor != target | |
10 | + self.errors.add(:requestor, _('You can not post articles to other users.')) | |
11 | + end | |
12 | + if target.organization? && !target.members.include?(requestor) && target.environment.portal_community != target | |
13 | + self.errors.add(:requestor, _('Only members can post articles on communities.')) | |
14 | + end | |
15 | + end | |
16 | + end | |
7 | 17 | |
8 | 18 | def article_title |
9 | 19 | article ? article.title : _('(The original text was removed)') | ... | ... |
test/unit/approve_article_test.rb
... | ... | @@ -9,6 +9,7 @@ class ApproveArticleTest < ActiveSupport::TestCase |
9 | 9 | @profile = create_user('test_user').person |
10 | 10 | @article = fast_create(TextileArticle, :profile_id => @profile.id, :name => 'test name', :abstract => 'Lead of article', :body => 'This is my article') |
11 | 11 | @community = fast_create(Community) |
12 | + @community.add_member(@profile) | |
12 | 13 | end |
13 | 14 | attr_reader :profile, :article, :community |
14 | 15 | |
... | ... | @@ -251,6 +252,8 @@ class ApproveArticleTest < ActiveSupport::TestCase |
251 | 252 | end |
252 | 253 | |
253 | 254 | should 'not group trackers activity of article\'s creation' do |
255 | + other_community = fast_create(Community) | |
256 | + other_community.add_member(profile) | |
254 | 257 | ActionTracker::Record.delete_all |
255 | 258 | |
256 | 259 | article = fast_create(TextileArticle) |
... | ... | @@ -262,20 +265,20 @@ class ApproveArticleTest < ActiveSupport::TestCase |
262 | 265 | a.finish |
263 | 266 | |
264 | 267 | article = fast_create(TextileArticle) |
265 | - other_community = fast_create(Community) | |
266 | 268 | a = create(ApproveArticle, :name => 'another bar', :article => article, :target => other_community, :requestor => profile) |
267 | 269 | a.finish |
268 | 270 | assert_equal 3, ActionTracker::Record.count |
269 | 271 | end |
270 | 272 | |
271 | 273 | should 'not create trackers activity when updating articles' do |
274 | + other_community = fast_create(Community) | |
275 | + other_community.add_member(profile) | |
272 | 276 | ActionTracker::Record.delete_all |
273 | 277 | article1 = fast_create(TextileArticle) |
274 | 278 | a = create(ApproveArticle, :name => 'bar', :article => article1, :target => community, :requestor => profile) |
275 | 279 | a.finish |
276 | 280 | |
277 | 281 | article2 = fast_create(TinyMceArticle) |
278 | - other_community = fast_create(Community) | |
279 | 282 | a = create(ApproveArticle, :name => 'another bar', :article => article2, :target => other_community, :requestor => profile) |
280 | 283 | a.finish |
281 | 284 | assert_equal 2, ActionTracker::Record.count |
... | ... | @@ -283,7 +286,7 @@ class ApproveArticleTest < ActiveSupport::TestCase |
283 | 286 | assert_no_difference 'ActionTracker::Record.count' do |
284 | 287 | published = article1.class.last |
285 | 288 | published.name = 'foo';published.save! |
286 | - | |
289 | + | |
287 | 290 | published = article2.class.last |
288 | 291 | published.name = 'another foo';published.save! |
289 | 292 | end |
... | ... | @@ -307,7 +310,7 @@ class ApproveArticleTest < ActiveSupport::TestCase |
307 | 310 | person = fast_create(Person) |
308 | 311 | person.stubs(:notification_emails).returns(['target@example.org']) |
309 | 312 | |
310 | - a = create(ApproveArticle, :article => article, :target => person, :requestor => profile) | |
313 | + a = create(ApproveArticle, :article => article, :target => person, :requestor => person) | |
311 | 314 | a.finish |
312 | 315 | |
313 | 316 | approved_article = person.articles.find_by_name(article.name) |
... | ... | @@ -440,4 +443,47 @@ class ApproveArticleTest < ActiveSupport::TestCase |
440 | 443 | assert_equal article, LinkArticle.last.reference_article |
441 | 444 | end |
442 | 445 | |
446 | + should 'not allow non-person requestor' do | |
447 | + task = ApproveArticle.new(:requestor => Community.new) | |
448 | + task.valid? | |
449 | + assert task.invalid?(:requestor) | |
450 | + end | |
451 | + | |
452 | + should 'allow only self requestors when the target is a person' do | |
453 | + person = fast_create(Person) | |
454 | + another_person = fast_create(Person) | |
455 | + | |
456 | + t1 = ApproveArticle.new(:requestor => person, :target => person) | |
457 | + t2 = ApproveArticle.new(:requestor => another_person, :target => person) | |
458 | + | |
459 | + assert t1.valid? | |
460 | + assert !t2.valid? | |
461 | + assert t2.invalid?(:requestor) | |
462 | + end | |
463 | + | |
464 | + should 'allow only members to be requestors when target is a community' do | |
465 | + community = fast_create(Community) | |
466 | + member = fast_create(Person) | |
467 | + community.add_member(member) | |
468 | + non_member = fast_create(Person) | |
469 | + | |
470 | + t1 = ApproveArticle.new(:requestor => member, :target => community) | |
471 | + t2 = ApproveArticle.new(:requestor => non_member, :target => community) | |
472 | + | |
473 | + assert t1.valid? | |
474 | + assert !t2.valid? | |
475 | + assert t2.invalid?(:requestor) | |
476 | + end | |
477 | + | |
478 | + should 'allow any user to be requestor whe the target is the portal community' do | |
479 | + community = fast_create(Community) | |
480 | + environment = community.environment | |
481 | + environment.portal_community = community | |
482 | + environment.save! | |
483 | + person = fast_create(Person) | |
484 | + | |
485 | + task = ApproveArticle.new(:requestor => person, :target => community) | |
486 | + | |
487 | + assert task.valid? | |
488 | + end | |
443 | 489 | end | ... | ... |