Merge pull request #2775 from evazion/fix-user-feedbacks

Prevent mods from editing/deleting feedbacks given to themselves.

app/controllers/user_feedbacks_controller.rb

@@ -49,6 +49,6 @@ class UserFeedbacksController < ApplicationController
 
 private
   def check_privilege(user_feedback)
-    raise User::PrivilegeError unless (user_feedback.creator_id == CurrentUser.id || CurrentUser.is_moderator?)
+    raise User::PrivilegeError unless user_feedback.editable_by?(CurrentUser.user)
   end
 end

app/models/user_feedback.rb

@@ -98,4 +98,8 @@ class UserFeedback < ActiveRecord::Base
       return true
     end
   end
+
+  def editable_by?(editor)
+    (editor.is_moderator? && editor != user) || creator == editor
+  end
 end

app/views/user_feedbacks/index.html.erb

@@ -20,7 +20,7 @@
             <td><%= compact_time(feedback.created_at) %></td>
             <td><%= format_text(feedback.body) %></td>
             <td>
-              <% if feedback.creator_id == CurrentUser.id || CurrentUser.is_moderator? %>
+              <% if feedback.editable_by?(CurrentUser.user) %>
                 <%= link_to "edit", edit_user_feedback_path(feedback) %>
                 | <%= link_to "delete", user_feedback_path(feedback), :method => :delete, :data => {:confirm => "Are you sure you want to delete this user feedback?"} %>
               <% end %>

app/views/user_feedbacks/show.html.erb

@@ -9,7 +9,7 @@
       <li><strong>Message</strong> <%= format_text @user_feedback.body %></li>
     </ul>
 
-    <% if @user_feedback.creator_id == CurrentUser.id || CurrentUser.is_moderator? %>
+    <% if @user_feedback.editable_by?(CurrentUser.user) %>
       <p><%= link_to "Edit", edit_user_feedback_path(@user_feedback) %></p>
     <% end %>
   </div>

test/functional/user_feedbacks_controller_test.rb

@@ -5,6 +5,7 @@ class UserFeedbacksControllerTest < ActionController::TestCase
     setup do
       @user = FactoryGirl.create(:user)
       @critic = FactoryGirl.create(:gold_user)
+      @mod = FactoryGirl.create(:moderator_user)
       CurrentUser.user = @critic
       CurrentUser.ip_addr = "127.0.0.1"
     end
@@ -62,7 +63,7 @@ class UserFeedbacksControllerTest < ActionController::TestCase
 
     context "destroy action" do
       setup do
-        @user_feedback = FactoryGirl.create(:user_feedback)
+        @user_feedback = FactoryGirl.create(:user_feedback, user: @user)
       end
 
       should "delete a feedback" do
@@ -70,6 +71,21 @@ class UserFeedbacksControllerTest < ActionController::TestCase
           post :destroy, {:id => @user_feedback.id}, {:user_id => @critic.id}
         end
       end
+
+      context "by a moderator" do
+        should "allow deleting feedbacks given to other users" do
+          assert_difference "UserFeedback.count", -1 do
+            post :destroy, {:id => @user_feedback.id}, {:user_id => @mod.id}
+          end
+        end
+
+        should "not allow deleting feedbacks given to themselves" do
+          @user_feedback = FactoryGirl.create(:user_feedback, user: @mod)
+          assert_difference "UserFeedback.count", 0 do
+            post :destroy, {:id => @user_feedback.id}, {:user_id => @mod.id}
+          end
+        end
+      end
     end
   end
 end