pundit: convert user feedbacks to pundit.

Allow users to delete feedbacks they've given to other users, not just
mods.
This commit is contained in:
evazion
2020-03-17 05:52:38 -05:00
parent 565a6572a7
commit 4cd0b2cbfe
9 changed files with 95 additions and 87 deletions

View File

@@ -1,53 +1,37 @@
class UserFeedbacksController < ApplicationController
before_action :gold_only, :only => [:new, :edit, :create, :update]
respond_to :html, :xml, :json, :js
def new
@user_feedback = UserFeedback.new(user_feedback_params(:create))
@user_feedback = authorize UserFeedback.new(permitted_attributes(UserFeedback))
respond_with(@user_feedback)
end
def edit
@user_feedback = UserFeedback.visible(CurrentUser.user).find(params[:id])
check_privilege(@user_feedback)
@user_feedback = authorize UserFeedback.find(params[:id])
respond_with(@user_feedback)
end
def show
@user_feedback = UserFeedback.visible(CurrentUser.user).find(params[:id])
@user_feedback = authorize UserFeedback.find(params[:id])
respond_with(@user_feedback)
end
def index
@user_feedbacks = UserFeedback.visible(CurrentUser.user).paginated_search(params, count_pages: true)
@user_feedbacks = authorize UserFeedback.visible(CurrentUser.user).paginated_search(params, count_pages: true)
@user_feedbacks = @user_feedbacks.includes(:user, :creator) if request.format.html?
respond_with(@user_feedbacks)
end
def create
@user_feedback = UserFeedback.create(user_feedback_params(:create).merge(creator: CurrentUser.user))
@user_feedback = authorize UserFeedback.new(creator: CurrentUser.user, **permitted_attributes(UserFeedback))
@user_feedback.save
respond_with(@user_feedback)
end
def update
@user_feedback = UserFeedback.visible(CurrentUser.user).find(params[:id])
check_privilege(@user_feedback)
@user_feedback.update(user_feedback_params(:update, @user_feedback))
@user_feedback = authorize UserFeedback.find(params[:id])
@user_feedback.update(permitted_attributes(@user_feedback))
respond_with(@user_feedback)
end
private
def check_privilege(user_feedback)
raise User::PrivilegeError unless user_feedback.editable_by?(CurrentUser.user)
end
def user_feedback_params(context, user_feedback = nil)
permitted_params = %i[body category]
permitted_params += %i[user_id user_name] if context == :create
permitted_params += %i[is_deleted] if context == :update && user_feedback.deletable_by?(CurrentUser.user)
params.fetch(:user_feedback, {}).permit(permitted_params)
end
end

View File

@@ -6,8 +6,6 @@ class UserFeedback < ApplicationRecord
attr_accessor :disable_dmail_notification
validates_presence_of :body, :category
validates_inclusion_of :category, :in => %w(positive negative neutral)
validate :creator_is_gold
validate :user_is_not_creator
after_create :create_dmail, unless: :disable_dmail_notification
after_update(:if => ->(rec) { CurrentUser.id != rec.creator_id}) do |rec|
ModAction.log(%{#{CurrentUser.name} updated user feedback for "#{rec.user.name}":/users/#{rec.user_id}}, :user_feedback_update)
@@ -67,26 +65,6 @@ class UserFeedback < ApplicationRecord
Dmail.create_automated(:to_id => user_id, :title => "Your user record has been updated", :body => body)
end
def creator_is_gold
if !creator.is_gold?
errors[:creator] << "must be gold"
end
end
def user_is_not_creator
if user_id == creator_id
errors[:creator] << "cannot submit feedback for yourself"
end
end
def deletable_by?(deleter)
deleter.is_moderator? && deleter != user
end
def editable_by?(editor)
(editor.is_moderator? && editor != user) || (creator == editor && !is_deleted?)
end
def self.available_includes
[:creator, :user]
end

View File

@@ -0,0 +1,25 @@
class UserFeedbackPolicy < ApplicationPolicy
def create?
unbanned? && user.is_gold? && record.user_id != user.id
end
def update?
create? && (user.is_moderator? || (record.creator_id == user.id && !record.is_deleted?))
end
def show?
!record.is_deleted? || can_view_deleted?
end
def can_view_deleted?
user.is_moderator?
end
def permitted_attributes_for_create
[:body, :category, :user_id, :user_name]
end
def permitted_attributes_for_update
[:body, :category, :is_deleted]
end
end

View File

@@ -1,6 +1,6 @@
<% content_for(:secondary_links) do %>
<% if CurrentUser.is_gold? %>
<% if @user_feedback %>
<% if policy(UserFeedback.new).create? %>
<% if @user_feedback.present? && policy(@user_feedback).create? %>
<%= subnav_link_to "New", new_user_feedback_path(:user_feedback => {:user_id => @user_feedback.user_id}) %>
<% elsif params[:search] && params[:search][:user_id] %>
<%= subnav_link_to "New", new_user_feedback_path(:user_feedback => {:user_id => params[:search][:user_id]}) %>

View File

@@ -10,9 +10,7 @@
<%= edit_form_for(@user_feedback) do |f| %>
<%= f.input :category, :collection => ["positive", "neutral", "negative"], :include_blank => false %>
<%= dtext_field "user_feedback", "body" %>
<% if @user_feedback.deletable_by?(CurrentUser.user) %>
<%= f.input :is_deleted, label: "Deleted" %>
<% end %>
<%= f.input :is_deleted, label: "Deleted" %>
<%= f.button :submit, "Submit" %>
<%= dtext_preview_button "user_feedback", "body" %>
<% end %>

View File

@@ -7,7 +7,7 @@
<%= f.input :creator_name, input_html: { value: params.dig(:search, :creator_name), "data-autocomplete": "user" } %>
<%= f.input :body_matches, label: "Message", input_html: { value: params.dig(:search, :body_matches) } %>
<%= f.input :category, collection: %w[positive negative neutral], include_blank: true, selected: params.dig(:search, :category) %>
<% if CurrentUser.is_moderator? %>
<% if policy(UserFeedback).can_view_deleted? %>
<%= f.input :is_deleted, label: "Deleted", collection: %w[Yes No], include_blank: true, selected: params.dig(:search, :is_deleted) %>
<% end %>
<%= f.submit "Search" %>
@@ -24,9 +24,9 @@
</div>
<%= render "application/update_notice", record: feedback, interval: 0.minutes %>
<% end %>
<% if CurrentUser.is_moderator? %>
<% if policy(UserFeedback).can_view_deleted? %>
<% t.column "Status" do |feedback| %>
<%= "deleted" if feedback.is_deleted? %>
<%= "Deleted" if feedback.is_deleted? %>
<% end %>
<% end %>
<% t.column "Category" do |feedback| %>
@@ -38,12 +38,12 @@
<div><%= time_ago_in_words_tagged(feedback.created_at) %></div>
<% end %>
<% t.column column: "control" do |feedback| %>
<% if feedback.editable_by?(CurrentUser.user) %>
<%= link_to "edit", edit_user_feedback_path(feedback) %>
<% if feedback.deletable_by?(CurrentUser.user) && !feedback.is_deleted? %>
| <%= link_to "delete", user_feedback_path(feedback), method: :put, remote: true, "data-params": "user_feedback[is_deleted]=true", "data-confirm": "Are you sure you want to delete this user feedback?" %>
<% elsif feedback.deletable_by?(CurrentUser.user) && feedback.is_deleted? %>
| <%= link_to "undelete", user_feedback_path(feedback), method: :put, remote: true, "data-params": "user_feedback[is_deleted]=false" %>
<% if policy(feedback).update? %>
<%= link_to "Edit", edit_user_feedback_path(feedback) %>
<% if feedback.is_deleted? %>
| <%= link_to "Undelete", user_feedback_path(feedback), method: :put, remote: true, "data-params": "user_feedback[is_deleted]=false" %>
<% else %>
| <%= link_to "Delete", user_feedback_path(feedback), method: :put, remote: true, "data-params": "user_feedback[is_deleted]=true", "data-confirm": "Are you sure you want to delete this user feedback?" %>
<% end %>
<% end %>
<% end %>

View File

@@ -17,7 +17,7 @@
</li>
</ul>
<% if @user_feedback.editable_by?(CurrentUser.user) %>
<% if policy(@user_feedback).update? %>
<p><%= link_to "Edit", edit_user_feedback_path(@user_feedback) %></p>
<% end %>
</div>

View File

@@ -23,12 +23,33 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest
end
end
context "show action" do
should "allow all users to see undeleted feedbacks" do
get user_feedback_path(@user_feedback)
assert_response :success
end
should "allow moderators to see deleted feedbacks" do
as(@user) { @user_feedback.update!(is_deleted: true) }
get_auth user_feedback_path(@user_feedback), @mod
assert_response :success
end
end
context "index action" do
should "render" do
get_auth user_feedbacks_path, @user
assert_response :success
end
should "not allow members to see deleted feedbacks" do
as(@user) { @user_feedback.update!(is_deleted: true) }
get_auth user_feedbacks_path, @user
assert_response :success
assert_select "tr#user-feedback-#{@user_feedback.id}", false
end
context "with search parameters" do
should "render" do
get_auth user_feedbacks_path, @critic, params: {:search => {:user_id => @user.id}}
@@ -38,31 +59,53 @@ class UserFeedbacksControllerTest < ActionDispatch::IntegrationTest
end
context "create action" do
should "create a new feedback" do
should "allow gold users to create new feedbacks" do
assert_difference("UserFeedback.count", 1) do
post_auth user_feedbacks_path, @critic, params: {:user_feedback => {:category => "positive", :user_name => @user.name, :body => "xxx"}}
assert_response :redirect
end
end
should "not allow users to create feedbacks for themselves" do
assert_no_difference("UserFeedback.count") do
post_auth user_feedbacks_path, @critic, params: { user_feedback: { user_id: @critic.id, category: "positive", body: "xxx" }}
assert_response 403
end
end
end
context "update action" do
should "update the feedback" do
should "allow updating undeleted feedbacks" do
put_auth user_feedback_path(@user_feedback), @critic, params: { user_feedback: { category: "positive" }}
assert_redirected_to(@user_feedback)
assert("positive", @user_feedback.reload.category)
assert_equal("positive", @user_feedback.reload.category)
end
should "not allow updating deleted feedbacks" do
as(@user) { @user_feedback.update!(is_deleted: true) }
put_auth user_feedback_path(@user_feedback), @critic, params: { user_feedback: { body: "test" }}
assert_response 403
end
should "allow deleting feedbacks given to others" do
put_auth user_feedback_path(@user_feedback), @critic, params: { user_feedback: { is_deleted: true }}
assert_response :redirect
assert_equal(true, @user_feedback.reload.is_deleted)
end
context "by a moderator" do
should "allow deleting feedbacks given to other users" do
should "allow updating feedbacks given to other users" do
put_auth user_feedback_path(@user_feedback), @mod, params: { user_feedback: { is_deleted: "true" }}
assert_redirected_to @user_feedback
assert(@user_feedback.reload.is_deleted?)
end
should "not allow deleting feedbacks given to themselves" do
@user_feedback = as(@critic) { create(:user_feedback, user: @mod) }
should "not allow updating feedbacks given to themselves" do
@user_feedback = create(:user_feedback, user: @mod, creator: @mod)
put_auth user_feedback_path(@user_feedback), @mod, params: { id: @user_feedback.id, user_feedback: { is_deleted: "true" }}
assert_response 403

View File

@@ -17,25 +17,5 @@ class UserFeedbackTest < ActiveSupport::TestCase
assert_equal(dmail, user.dmails.last.body)
end
end
should "not validate if the creator is the user" do
user = FactoryBot.create(:gold_user)
feedback = build(:user_feedback, creator: user, user: user)
feedback.save
assert_equal(["You cannot submit feedback for yourself"], feedback.errors.full_messages)
end
should "not validate if the creator is not gold" do
user = FactoryBot.create(:user)
gold = FactoryBot.create(:gold_user)
member = FactoryBot.create(:user)
feedback = FactoryBot.create(:user_feedback, creator: gold, user: user)
assert(feedback.errors.empty?)
feedback = build(:user_feedback, creator: member, user: user)
feedback.save
assert_equal(["You must be gold"], feedback.errors.full_messages)
end
end
end