pundit: convert wiki pages to pundit.

app/controllers/wiki_pages_controller.rb

@@ -1,31 +1,32 @@
 class WikiPagesController < ApplicationController
   respond_to :html, :xml, :json, :js
-  before_action :member_only, :except => [:index, :search, :show, :show_or_new]
   before_action :normalize_search_params, :only => [:index]
   layout "sidebar"
 
   def new
-    @wiki_page = WikiPage.new(wiki_page_params(:create))
+    @wiki_page = authorize WikiPage.new(permitted_attributes(WikiPage))
     respond_with(@wiki_page)
   end
 
   def edit
     @wiki_page, _found_by = WikiPage.find_by_id_or_title(params[:id])
+    authorize @wiki_page
     respond_with(@wiki_page)
   end
 
   def index
-    @wiki_pages = WikiPage.paginated_search(params)
-
+    @wiki_pages = authorize WikiPage.paginated_search(params)
     respond_with(@wiki_pages)
   end
 
   def search
+    authorize WikiPage
     render layout: "default"
   end
 
   def show
     @wiki_page, found_by = WikiPage.find_by_id_or_title(params[:id])
+
     if request.format.html? && @wiki_page.blank? && found_by == :title
       @wiki_page = WikiPage.new(title: params[:id])
       respond_with @wiki_page, status: 404
@@ -39,13 +40,16 @@ class WikiPagesController < ApplicationController
   end
 
   def create
-    @wiki_page = WikiPage.create(wiki_page_params(:create))
+    @wiki_page = authorize WikiPage.new(permitted_attributes(WikiPage))
+    @wiki_page.save
     respond_with(@wiki_page)
   end
 
   def update
     @wiki_page, _found_by = WikiPage.find_by_id_or_title(params[:id])
-    @wiki_page.update(wiki_page_params(:update))
+    authorize @wiki_page
+
+    @wiki_page.update(permitted_attributes(@wiki_page))
     flash[:notice] = @wiki_page.warnings.full_messages.join(".\n \n") if @wiki_page.warnings.any?
 
     respond_with(@wiki_page)
@@ -53,12 +57,16 @@ class WikiPagesController < ApplicationController
 
   def destroy
     @wiki_page, _found_by = WikiPage.find_by_id_or_title(params[:id])
+    authorize @wiki_page
+
     @wiki_page.update(is_deleted: true)
     respond_with(@wiki_page)
   end
 
   def revert
     @wiki_page, _found_by = WikiPage.find_by_id_or_title(params[:id])
+    authorize @wiki_page
+
     @version = @wiki_page.versions.find(params[:version_id])
     @wiki_page.revert_to!(@version)
     flash[:notice] = "Page was reverted"
@@ -67,7 +75,7 @@ class WikiPagesController < ApplicationController
 
   def show_or_new
     if params[:title].blank?
-      redirect_to new_wiki_page_path(wiki_page_params(:create))
+      redirect_to new_wiki_page_path(permitted_attributes(WikiPage))
     else
       redirect_to wiki_page_path(params[:title])
     end
@@ -89,11 +97,4 @@ class WikiPagesController < ApplicationController
       params[:search][:title] = params.delete(:title)
     end
   end
-
-  def wiki_page_params(context)
-    permitted_params = %i[title body other_names other_names_string is_deleted]
-    permitted_params += %i[is_locked] if CurrentUser.is_builder?
-
-    params.fetch(:wiki_page, {}).permit(permitted_params)
-  end
 end

app/models/wiki_page.rb

@@ -11,7 +11,6 @@ class WikiPage < ApplicationRecord
   validates_presence_of :title
   validates_presence_of :body, :unless => -> { is_deleted? || other_names.present? }
   validate :validate_rename
-  validate :validate_not_locked
 
   array_attribute :other_names
   has_one :tag, :foreign_key => "name", :primary_key => "title"
@@ -116,12 +115,6 @@ class WikiPage < ApplicationRecord
   extend SearchMethods
   include ApiMethods
 
-  def validate_not_locked
-    if is_locked? && !CurrentUser.is_builder?
-      errors.add(:is_locked, "and cannot be updated")
-    end
-  end
-
   def validate_rename
     return unless title_changed?
 

app/policies/wiki_page_policy.rb

@@ -0,0 +1,17 @@
+class WikiPagePolicy < ApplicationPolicy
+  def update?
+    unbanned? && (can_edit_locked? || !record.is_locked?)
+  end
+
+  def revert?
+    update?
+  end
+
+  def can_edit_locked?
+    user.is_builder?
+  end
+
+  def permitted_attributes
+    [:title, :body, :other_names, :other_names_string, :is_deleted, (:is_locked if can_edit_locked?)].compact
+  end
+end

app/views/wiki_pages/_form.html.erb

@@ -7,7 +7,7 @@
 
     <%= dtext_field "wiki_page", "body" %>
 
-    <% if CurrentUser.is_builder? %>
+    <% if policy(@wiki_page).can_edit_locked? %>
       <%= f.input :is_locked, label: "Locked", hint: "Locked wikis can only be edited by Builders." %>
     <% end %>
 

app/views/wiki_pages/_secondary_links.html.erb

@@ -2,7 +2,7 @@
   <%= quick_search_form_for(:title_normalize, wiki_pages_path, "wiki pages", autocomplete: "wiki-page", redirect: true) %>
   <%= subnav_link_to "Listing", wiki_pages_path %>
   <%= subnav_link_to "Search", search_wiki_pages_path %>
-  <% if CurrentUser.is_member? %>
+  <% if policy(WikiPage).new? %>
     <%= subnav_link_to "New", new_wiki_page_path %>
   <% end %>
   <%= subnav_link_to "Help", wiki_page_path("help:wiki") %>
@@ -13,7 +13,7 @@
     <li>|</li>
     <%= subnav_link_to "Posts (#{@wiki_page.tag.try(:post_count) || 0})", posts_path(:tags => @wiki_page.title) %>
     <%= subnav_link_to "History", wiki_page_versions_path(:search => {:wiki_page_id => @wiki_page.id}) %>
-    <% if CurrentUser.is_member? %>
+    <% if policy(@wiki_page).edit? %>
       <%= subnav_link_to "Edit", edit_wiki_page_path(@wiki_page.id), "data-shortcut": "e" %>
 
       <% if @wiki_page.is_deleted? %>
@@ -28,7 +28,7 @@
     <% if @wiki_page_version.previous %>
       <%= subnav_link_to "Diff", diff_wiki_page_versions_path(:otherpage => @wiki_page_version.id, :thispage => @wiki_page_version.previous.id) %>
     <% end %>
-    <% if CurrentUser.is_member? %>
+    <% if policy(@wiki_page_version.wiki_page).revert? %>
       <%= subnav_link_to "Revert to", revert_wiki_page_path(@wiki_page_version.wiki_page_id, :version_id => @wiki_page_version.id), :method => :put, :data => {:confirm => "Are you sure you want to revert to this version?"} %>
     <% end %>
   <% end %>

test/functional/wiki_pages_controller_test.rb

@@ -33,6 +33,13 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
       end
     end
 
+    context "search action" do
+      should "work" do
+        get search_wiki_pages_path
+        assert_response :success
+      end
+    end
+
     context "show action" do
       setup do
         as_user do
@@ -146,6 +153,7 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
       should "create a wiki_page" do
         assert_difference("WikiPage.count", 1) do
           post_auth wiki_pages_path, @user, params: {:wiki_page => {:title => "abc", :body => "abc"}}
+          assert_redirected_to(wiki_page_path(WikiPage.last))
         end
       end
     end
@@ -155,13 +163,45 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
         as_user do
           @tag = create(:tag, name: "foo", post_count: 42)
           @wiki_page = create(:wiki_page, title: "foo")
+          @builder = create(:builder_user)
         end
       end
 
       should "update a wiki_page" do
         put_auth wiki_page_path(@wiki_page), @user, params: {:wiki_page => {:body => "xyz"}}
-        @wiki_page.reload
-        assert_equal("xyz", @wiki_page.body)
+
+        assert_redirected_to wiki_page_path(@wiki_page)
+        assert_equal("xyz", @wiki_page.reload.body)
+      end
+
+      should "not allow members to edit locked wiki pages" do
+        as(@user) { @wiki_page.update!(is_locked: true) }
+        put_auth wiki_page_path(@wiki_page), @user, params: { wiki_page: { body: "xyz" }}
+
+        assert_response 403
+        assert_not_equal("xyz", @wiki_page.reload.body)
+      end
+
+      should "allow builders to edit locked wiki pages" do
+        as(@builder) { @wiki_page.update!(is_locked: true) }
+        put_auth wiki_page_path(@wiki_page), @builder, params: { wiki_page: { body: "xyz" }}
+
+        assert_redirected_to wiki_page_path(@wiki_page)
+        assert_equal("xyz", @wiki_page.reload.body)
+      end
+
+      should "not allow members to edit the is_locked flag" do
+        put_auth wiki_page_path(@wiki_page), @user, params: { wiki_page: { is_locked: true }}
+
+        assert_response 403
+        assert_equal(false, @wiki_page.reload.is_locked)
+      end
+
+      should "allow builders to edit the is_locked flag" do
+        put_auth wiki_page_path(@wiki_page), @builder, params: { wiki_page: { is_locked: true }}
+
+        assert_redirected_to wiki_page_path(@wiki_page)
+        assert_equal(true, @wiki_page.reload.is_locked)
       end
 
       should "warn about renaming a wiki page with a non-empty tag" do

test/unit/wiki_page_test.rb

@@ -11,38 +11,6 @@ class WikiPageTest < ActiveSupport::TestCase
   end
 
   context "A wiki page" do
-    context "that is locked" do
-      should "not be editable by a member" do
-        CurrentUser.user = FactoryBot.create(:moderator_user)
-        @wiki_page = FactoryBot.create(:wiki_page, :is_locked => true)
-        CurrentUser.user = FactoryBot.create(:user)
-        @wiki_page.update(body: "hello")
-        assert_equal(["Is locked and cannot be updated"], @wiki_page.errors.full_messages)
-      end
-
-      should "be editable by a moderator" do
-        CurrentUser.user = FactoryBot.create(:moderator_user)
-        @wiki_page = FactoryBot.create(:wiki_page, :is_locked => true)
-        CurrentUser.user = FactoryBot.create(:moderator_user)
-        @wiki_page.update(body: "hello")
-        assert_equal([], @wiki_page.errors.full_messages)
-      end
-    end
-
-    context "updated by a moderator" do
-      setup do
-        @user = FactoryBot.create(:moderator_user)
-        CurrentUser.user = @user
-        @wiki_page = FactoryBot.create(:wiki_page)
-      end
-
-      should "allow the is_locked attribute to be updated" do
-        @wiki_page.update(is_locked: true)
-        @wiki_page.reload
-        assert_equal(true, @wiki_page.is_locked?)
-      end
-    end
-
     context "updated by a regular user" do
       setup do
         @user = FactoryBot.create(:user)
@@ -50,13 +18,6 @@ class WikiPageTest < ActiveSupport::TestCase
         @wiki_page = FactoryBot.create(:wiki_page, :title => "HOT POTATO", :other_names => "foo*bar baz")
       end
 
-      should "not allow the is_locked attribute to be updated" do
-        @wiki_page.update(is_locked: true)
-        assert_equal(["Is locked and cannot be updated"], @wiki_page.errors.full_messages)
-        @wiki_page.reload
-        assert_equal(false, @wiki_page.is_locked?)
-      end
-
       should "normalize its title" do
         assert_equal("hot_potato", @wiki_page.title)
       end