From ab5432d149a5661471ad28b2fc4411033b24bb52 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Thu, 19 Mar 2020 19:31:57 -0500
Subject: [PATCH] pundit: convert pools to pundit.

---
 app/controllers/pool_elements_controller.rb   | 12 +++---
 app/controllers/pool_orders_controller.rb     |  3 +-
 app/controllers/pools_controller.rb           | 39 ++++++-------------
 app/models/pool.rb                            |  6 +--
 app/policies/pool_policy.rb                   | 25 ++++++++++++
 app/views/pool_elements/create.js.erb         |  6 +--
 app/views/pools/_secondary_links.html.erb     | 18 ++++-----
 .../pool_elements_controller_test.rb          |  8 ++--
 8 files changed, 58 insertions(+), 59 deletions(-)
 create mode 100644 app/policies/pool_policy.rb

diff --git a/app/controllers/pool_elements_controller.rb b/app/controllers/pool_elements_controller.rb
index e8e3da95d..cbbf8678b 100644
--- a/app/controllers/pool_elements_controller.rb
+++ b/app/controllers/pool_elements_controller.rb
@@ -1,15 +1,13 @@
 class PoolElementsController < ApplicationController
   respond_to :html, :xml, :json, :js
-  before_action :member_only
 
   def create
     @pool = Pool.find_by_name(params[:pool_name]) || Pool.find_by_id(params[:pool_id])
+    raise ActiveRecord::RecordNotFound if @pool.nil?
+    authorize(@pool, :update?)
 
-    if @pool.present? && !@pool.is_deleted?
-      @post = Post.find(params[:post_id])
-      @pool.add!(@post)
-    else
-      @error = "That pool does not exist"
-    end
+    @post = Post.find(params[:post_id])
+    @pool.add!(@post)
+    respond_with(@pool)
   end
 end
diff --git a/app/controllers/pool_orders_controller.rb b/app/controllers/pool_orders_controller.rb
index 10b5b6a8c..7a6f4a5a6 100644
--- a/app/controllers/pool_orders_controller.rb
+++ b/app/controllers/pool_orders_controller.rb
@@ -1,9 +1,8 @@
 class PoolOrdersController < ApplicationController
   respond_to :html, :xml, :json, :js
-  before_action :member_only
 
   def edit
-    @pool = Pool.find(params[:pool_id])
+    @pool = authorize Pool.find(params[:pool_id])
     respond_with(@pool)
   end
 end
diff --git a/app/controllers/pools_controller.rb b/app/controllers/pools_controller.rb
index 282682ae8..d13ef3b80 100644
--- a/app/controllers/pools_controller.rb
+++ b/app/controllers/pools_controller.rb
@@ -1,23 +1,18 @@
 class PoolsController < ApplicationController
   respond_to :html, :xml, :json, :js
-  before_action :member_only, :except => [:index, :show, :gallery]
-  before_action :builder_only, :only => [:destroy]
 
   def new
-    @pool = Pool.new
+    @pool = authorize Pool.new(permitted_attributes(Pool))
     respond_with(@pool)
   end
 
   def edit
-    @pool = Pool.find(params[:id])
-    if @pool.is_deleted && !@pool.deletable_by?(CurrentUser.user)
-      raise User::PrivilegeError
-    end
+    @pool = authorize Pool.find(params[:id])
     respond_with(@pool)
   end
 
   def index
-    @pools = Pool.paginated_search(params, count_pages: true)
+    @pools = authorize Pool.paginated_search(params, count_pages: true)
 
     respond_with(@pools)
   end
@@ -26,28 +21,29 @@ class PoolsController < ApplicationController
     limit = params[:limit].presence || CurrentUser.user.per_page
     search = search_params.presence || ActionController::Parameters.new(category: "series")
 
-    @pools = Pool.search(search).paginate(params[:page], limit: limit, search_count: params[:search])
+    @pools = authorize Pool.search(search).paginate(params[:page], limit: limit, search_count: params[:search])
     respond_with(@pools)
   end
 
   def show
     limit = params[:limit].presence || CurrentUser.user.per_page
 
-    @pool = Pool.find(params[:id])
+    @pool = authorize Pool.find(params[:id])
     @posts = @pool.posts.paginate(params[:page], limit: limit, count: @pool.post_count)
     respond_with(@pool)
   end
 
   def create
-    @pool = Pool.create(pool_params)
+    @pool = authorize Pool.new(permitted_attributes(Pool))
+    @pool.save
     flash[:notice] = @pool.valid? ? "Pool created" : @pool.errors.full_messages.join("; ")
     respond_with(@pool)
   end
 
   def update
     # need to do this in order for synchronize! to work correctly
-    @pool = Pool.find(params[:id])
-    @pool.attributes = pool_params
+    @pool = authorize Pool.find(params[:id])
+    @pool.attributes = permitted_attributes(@pool)
     @pool.synchronize
     @pool.save
     unless @pool.errors.any?
@@ -57,10 +53,7 @@ class PoolsController < ApplicationController
   end
 
   def destroy
-    @pool = Pool.find(params[:id])
-    if !@pool.deletable_by?(CurrentUser.user)
-      raise User::PrivilegeError
-    end
+    @pool = authorize Pool.find(params[:id])
     @pool.update_attribute(:is_deleted, true)
     @pool.create_mod_action_for_delete
     flash[:notice] = "Pool deleted"
@@ -68,10 +61,7 @@ class PoolsController < ApplicationController
   end
 
   def undelete
-    @pool = Pool.find(params[:id])
-    if !@pool.deletable_by?(CurrentUser.user)
-      raise User::PrivilegeError
-    end
+    @pool = authorize Pool.find(params[:id])
     @pool.update_attribute(:is_deleted, false)
     @pool.create_mod_action_for_undelete
     flash[:notice] = "Pool undeleted"
@@ -79,7 +69,7 @@ class PoolsController < ApplicationController
   end
 
   def revert
-    @pool = Pool.find(params[:id])
+    @pool = authorize Pool.find(params[:id])
     @version = @pool.versions.find(params[:version_id])
     @pool.revert_to!(@version)
     flash[:notice] = "Pool reverted"
@@ -97,9 +87,4 @@ class PoolsController < ApplicationController
       true
     end
   end
-
-  def pool_params
-    permitted_params = %i[name description category post_ids post_ids_string]
-    params.require(:pool).permit(*permitted_params, post_ids: [])
-  end
 end
diff --git a/app/models/pool.rb b/app/models/pool.rb
index 70496dce9..197d280fa 100644
--- a/app/models/pool.rb
+++ b/app/models/pool.rb
@@ -147,12 +147,8 @@ class Pool < ApplicationRecord
     post_ids.find_index(post_id).to_i + 1
   end
 
-  def deletable_by?(user)
-    user.is_builder?
-  end
-
   def updater_can_edit_deleted
-    if is_deleted? && !deletable_by?(CurrentUser.user)
+    if is_deleted? && !Pundit.policy!([CurrentUser.user, nil], self).update?
       errors[:base] << "You cannot update pools that are deleted"
     end
   end
diff --git a/app/policies/pool_policy.rb b/app/policies/pool_policy.rb
new file mode 100644
index 000000000..cb76b67ed
--- /dev/null
+++ b/app/policies/pool_policy.rb
@@ -0,0 +1,25 @@
+class PoolPolicy < ApplicationPolicy
+  def gallery?
+    index?
+  end
+
+  def update?
+    unbanned? && (!record.is_deleted? || user.is_builder?)
+  end
+
+  def destroy?
+    !record.is_deleted? && user.is_builder?
+  end
+
+  def undelete?
+    record.is_deleted? && user.is_builder?
+  end
+
+  def revert?
+    update?
+  end
+
+  def permitted_attributes
+    [:name, :description, :category, :post_ids, :post_ids_string, post_ids: []]
+  end
+end
diff --git a/app/views/pool_elements/create.js.erb b/app/views/pool_elements/create.js.erb
index 9ec4c14ee..345366b9b 100644
--- a/app/views/pool_elements/create.js.erb
+++ b/app/views/pool_elements/create.js.erb
@@ -1,5 +1 @@
-<% if @error %>
-  Danbooru.error("<%= j @error.to_s %>");
-<% else %>
-  location.reload();
-<% end %>
+location.reload();
diff --git a/app/views/pools/_secondary_links.html.erb b/app/views/pools/_secondary_links.html.erb
index 8f32915a2..618c9070e 100644
--- a/app/views/pools/_secondary_links.html.erb
+++ b/app/views/pools/_secondary_links.html.erb
@@ -2,26 +2,26 @@
   <%= quick_search_form_for(:name_matches, pools_path, "pools", autocomplete: "pool", redirect: true) %>
   <%= subnav_link_to "Gallery", gallery_pools_path %>
   <%= subnav_link_to "Listing", pools_path %>
-  <%= subnav_link_to "New", new_pool_path %>
+  <% if policy(Pool).create? %>
+    <%= subnav_link_to "New", new_pool_path %>
+  <% end %>
   <%= subnav_link_to "Help", wiki_page_path("help:pools") %>
   <% if @pool && !@pool.new_record? %>
     <li>|</li>
     <%= subnav_link_to "Show", pool_path(@pool) %>
     <%= subnav_link_to "Posts", posts_path(:tags => "pool:#{@pool.id}") %>
-    <% if CurrentUser.is_member? %>
+    <% if policy(@pool).update? %>
       <%= subnav_link_to "Edit", edit_pool_path(@pool), "data-shortcut": "e" %>
     <% end %>
-    <% if @pool.deletable_by?(CurrentUser.user) %>
-      <% if @pool.is_deleted? %>
-        <%= subnav_link_to "Undelete", undelete_pool_path(@pool), :method => :post, :remote => true %>
-      <% else %>
-        <%= subnav_link_to "Delete", pool_path(@pool), :method => :delete, :"data-shortcut" => "shift+d", :"data-confirm" => "Are you sure you want to delete this pool?", :remote => true %>
-      <% end %>
+    <% if policy(@pool).undelete? %>
+      <%= subnav_link_to "Undelete", undelete_pool_path(@pool), :method => :post, :remote => true %>
+    <% elsif policy(@pool).destroy? %>
+      <%= subnav_link_to "Delete", pool_path(@pool), :method => :delete, :"data-shortcut" => "shift+d", :"data-confirm" => "Are you sure you want to delete this pool?", :remote => true %>
     <% end %>
     <% if PoolVersion.enabled? %>
       <%= subnav_link_to "History", pool_versions_path(:search => {:pool_id => @pool.id}) %>
     <% end %>
-    <% if CurrentUser.is_member? %>
+    <% if policy(@pool).update? %>
       <%= subnav_link_to "Order", edit_pool_order_path(@pool) %>
     <% end %>
   <% end %>
diff --git a/test/functional/pool_elements_controller_test.rb b/test/functional/pool_elements_controller_test.rb
index 2809c42f5..bde89265f 100644
--- a/test/functional/pool_elements_controller_test.rb
+++ b/test/functional/pool_elements_controller_test.rb
@@ -20,15 +20,15 @@ class PoolElementsControllerTest < ActionDispatch::IntegrationTest
     context "create action" do
       should "add a post to a pool" do
         post_auth pool_element_path, @user, params: {:pool_id => @pool.id, :post_id => @post.id, :format => "json"}
-        @pool.reload
-        assert_equal([@post.id], @pool.post_ids)
+        assert_response :success
+        assert_equal([@post.id], @pool.reload.post_ids)
       end
 
       should "add a post to a pool once and only once" do
         as_user { @pool.add!(@post) }
         post_auth pool_element_path, @user, params: {:pool_id => @pool.id, :post_id => @post.id, :format => "json"}
-        @pool.reload
-        assert_equal([@post.id], @pool.post_ids)
+        assert_response :success
+        assert_equal([@post.id], @pool.reload.post_ids)
       end
     end
   end