Commit 5ae2a0b6f4891187024c5fdd7688033ead0ba54f
1 parent
aa6be22b
Exists in
master
and in
29 other branches
[comments-refactor-review] Encapsulating comment destroy check
Showing
3 changed files
with
48 additions
and
2 deletions
Show diff stats
app/controllers/public/comment_controller.rb
| @@ -86,8 +86,7 @@ class CommentController < ApplicationController | @@ -86,8 +86,7 @@ class CommentController < ApplicationController | ||
| 86 | def destroy | 86 | def destroy |
| 87 | comment = profile.comments_received.find(params[:id]) | 87 | comment = profile.comments_received.find(params[:id]) |
| 88 | 88 | ||
| 89 | - could_remove = (user == comment.author || user == comment.profile || user.has_permission?(:moderate_comments, comment.profile)) | ||
| 90 | - if comment && could_remove && comment.destroy | 89 | + if comment && comment.can_be_destroyed_by?(user) && comment.destroy |
| 91 | render :text => {'ok' => true}.to_json, :content_type => 'application/json' | 90 | render :text => {'ok' => true}.to_json, :content_type => 'application/json' |
| 92 | else | 91 | else |
| 93 | session[:notice] = _("The comment was not removed.") | 92 | session[:notice] = _("The comment was not removed.") |
app/models/comment.rb
| @@ -259,4 +259,9 @@ class Comment < ActiveRecord::Base | @@ -259,4 +259,9 @@ class Comment < ActiveRecord::Base | ||
| 259 | article.moderate_comments? && (author.nil? || article.author != author) | 259 | article.moderate_comments? && (author.nil? || article.author != author) |
| 260 | end | 260 | end |
| 261 | 261 | ||
| 262 | + def can_be_destroyed_by?(user) | ||
| 263 | + return if user.nil? | ||
| 264 | + user == author || user == profile || user.has_permission?(:moderate_comments, profile) | ||
| 265 | + end | ||
| 266 | + | ||
| 262 | end | 267 | end |
test/unit/comment_test.rb
| @@ -593,6 +593,48 @@ class CommentTest < ActiveSupport::TestCase | @@ -593,6 +593,48 @@ class CommentTest < ActiveSupport::TestCase | ||
| 593 | assert comment.need_moderation? | 593 | assert comment.need_moderation? |
| 594 | end | 594 | end |
| 595 | 595 | ||
| 596 | + should 'not be able to destroy comment without user' do | ||
| 597 | + comment = Comment.new | ||
| 598 | + | ||
| 599 | + assert !comment.can_be_destroyed_by?(nil) | ||
| 600 | + end | ||
| 601 | + | ||
| 602 | + should 'not be able to destroy comment' do | ||
| 603 | + user = Person.new | ||
| 604 | + profile = Profile.new | ||
| 605 | + article = Article.new(:profile => profile) | ||
| 606 | + comment = Comment.new(:article => article) | ||
| 607 | + user.expects(:has_permission?).with(:moderate_comments, profile).returns(false) | ||
| 608 | + | ||
| 609 | + assert !comment.can_be_destroyed_by?(user) | ||
| 610 | + end | ||
| 611 | + | ||
| 612 | + should 'be able to destroy comment if is the author' do | ||
| 613 | + user = Person.new | ||
| 614 | + comment = Comment.new(:author => user) | ||
| 615 | + | ||
| 616 | + assert comment.can_be_destroyed_by?(user) | ||
| 617 | + end | ||
| 618 | + | ||
| 619 | + should 'be able to destroy comment if is the profile' do | ||
| 620 | + user = Person.new | ||
| 621 | + article = Article.new(:profile => user) | ||
| 622 | + comment = Comment.new(:article => article) | ||
| 623 | + | ||
| 624 | + assert comment.can_be_destroyed_by?(user) | ||
| 625 | + end | ||
| 626 | + | ||
| 627 | + should 'be able to destroy comment if can moderate_comments on the profile' do | ||
| 628 | + user = Person.new | ||
| 629 | + profile = Profile.new | ||
| 630 | + article = Article.new(:profile => profile) | ||
| 631 | + comment = Comment.new(:article => article) | ||
| 632 | + | ||
| 633 | + user.expects(:has_permission?).with(:moderate_comments, profile).returns(true) | ||
| 634 | + | ||
| 635 | + assert comment.can_be_destroyed_by?(user) | ||
| 636 | + end | ||
| 637 | + | ||
| 596 | private | 638 | private |
| 597 | 639 | ||
| 598 | def create_comment(args = {}) | 640 | def create_comment(args = {}) |