[WikiPages] Add editing tag category (#920)

Co-authored-by: Donovan Daniels <hewwo@yiff.rocks>

app/controllers/wiki_pages_controller.rb

@@ -123,9 +123,10 @@ class WikiPagesController < ApplicationController
   end
 
   def wiki_page_params(context)
-    permitted_params = %i[body edit_reason]
+    permitted_params = %i[body category_id edit_reason]
     permitted_params += %i[parent] if CurrentUser.is_privileged?
     permitted_params += %i[is_locked is_deleted skip_secondary_validations] if CurrentUser.is_janitor?
+    permitted_params += %i[category_is_locked] if CurrentUser.is_admin?
     permitted_params += %i[title] if context == :create || CurrentUser.is_janitor?
 
     params.fetch(:wiki_page, {}).permit(permitted_params)

app/models/tag.rb

@@ -80,10 +80,14 @@ class Tag < ApplicationRecord
 
       def category_for(tag_name)
         Cache.fetch("tc:#{tag_name}") do
-          Tag.where(name: tag_name).pick(:category).to_i
+          category_for!(tag_name).to_i
         end
       end
 
+      def category_for!(tag_name)
+        Tag.where(name: tag_name).pick(:category)
+      end
+
       def categories_for(tag_names, disable_cache: false)
         if disable_cache
           tag_cats = {}

app/models/wiki_page.rb

@@ -1,14 +1,21 @@
 # frozen_string_literal: true
 
 class WikiPage < ApplicationRecord
-  class RevertError < Exception ; end
+  class RevertError < Exception; end
 
   before_validation :normalize_title
   before_validation :normalize_other_names
   before_validation :normalize_parent
+  before_save :log_changes
+  before_save :update_tag, if: :tag_changed?
+  before_destroy :validate_not_used_as_help_page
+  before_destroy :log_destroy
   after_save :create_version
+  after_save :update_help_page, if: :saved_change_to_title?
+
   normalizes :body, with: ->(body) { body.gsub("\r\n", "\n") }
-  validates :title, uniqueness: { :case_sensitive => false }
+
+  validates :title, uniqueness: { case_sensitive: false }
   validates :title, presence: true
   validates :title, tag_name: true, if: :title_changed?
   validates :body, presence: { unless: -> { is_deleted? || other_names.present? || parent.present? } }
@@ -19,12 +26,8 @@ class WikiPage < ApplicationRecord
   validate :validate_redirect
   validate :validate_not_locked
 
-  before_destroy :validate_not_used_as_help_page
-  before_destroy :log_destroy
-  before_save :log_changes
-  after_save :update_help_page, if: :saved_change_to_title?
-
   attr_accessor :skip_secondary_validations, :edit_reason
+
   array_attribute :other_names
   belongs_to_creator
   belongs_to_updater
@@ -33,19 +36,19 @@ class WikiPage < ApplicationRecord
   has_many :versions, -> { order("wiki_page_versions.id ASC") }, class_name: "WikiPageVersion", dependent: :destroy
   has_one :help_page, foreign_key: "wiki_page", primary_key: "title"
 
-  def log_destroy
-    ModAction.log(:wiki_page_delete, {wiki_page: title, wiki_page_id: id})
-  end
-
   def log_changes
     if title_changed? && !new_record?
-      ModAction.log(:wiki_page_rename, {new_title: title, old_title: title_was})
+      ModAction.log(:wiki_page_rename, { new_title: title, old_title: title_was })
     end
     if is_locked_changed?
-      ModAction.log(is_locked ? :wiki_page_lock : :wiki_page_unlock, {wiki_page: title})
+      ModAction.log(is_locked ? :wiki_page_lock : :wiki_page_unlock, { wiki_page: title })
     end
   end
 
+  def log_destroy
+    ModAction.log(:wiki_page_delete, { wiki_page: title, wiki_page_id: id })
+  end
+
   module SearchMethods
     def titled(title)
       find_by(title: WikiPage.normalize_name(title))
@@ -123,7 +126,7 @@ class WikiPage < ApplicationRecord
 
   module ApiMethods
     def method_attributes
-      super + [:creator_name, :category_id]
+      super + %i[creator_name category_id]
     end
   end
 
@@ -140,9 +143,92 @@ class WikiPage < ApplicationRecord
     end
   end
 
+  module TagMethods
+    def tag
+      @tag ||= super
+    end
+
+    def category_id
+      return @category_id if instance_variable_defined?(:@category_id)
+      @category_id = tag&.category
+    end
+
+    def category_id=(value)
+      return if value.blank? || value.to_i == category_id
+      category_id_will_change!
+      @category_id = value.to_i
+    end
+
+    def category_is_locked
+      return @category_is_locked if instance_variable_defined?(:@category_is_locked)
+      @category_is_locked = tag&.is_locked || false
+    end
+
+    def category_is_locked=(value)
+      return if value == category_is_locked
+      category_is_locked_will_change!
+      @category_is_locked = value
+    end
+
+    def category_id_changed?
+      attribute_changed?("category_id")
+    end
+
+    def category_id_will_change!
+      attribute_will_change!("category_id")
+    end
+
+    def category_is_locked_changed?
+      attribute_changed?("category_is_locked")
+    end
+
+    def category_is_locked_will_change!
+      attribute_will_change!("category_is_locked")
+    end
+
+    def tag_update_map
+      {}.tap do |updates|
+        updates[:category] = @category_id if category_id_changed?
+        updates[:is_locked] = @category_is_locked if category_is_locked_changed?
+      end
+    end
+
+    def tag_changed?
+      tag_update_map.present?
+    end
+
+    def update_tag
+      updates = tag_update_map
+      @tag = Tag.find_or_create_by_name(title)
+
+      return if updates.empty?
+      unless @tag.category_editable_by?(CurrentUser.user)
+        reload_tag_attributes
+        errors.add(:category_id, "Cannot be changed")
+        throw(:abort)
+      end
+
+      @tag.update(updates)
+      @tag.save
+
+      if @tag.invalid?
+        errors.add(:category_id, @tag.errors.full_messages.join(", "))
+        throw(:abort)
+      end
+
+      reload_tag_attributes
+    end
+
+    def reload_tag_attributes
+      remove_instance_variable(:@category_id) if instance_variable_defined?(:@category_id)
+      remove_instance_variable(:@category_is_locked) if instance_variable_defined?(:@category_is_locked)
+    end
+  end
+
   extend SearchMethods
   include ApiMethods
   include HelpPageMethods
+  include TagMethods
 
   def user_not_limited
     allowed = CurrentUser.can_wiki_edit_with_reason
@@ -156,7 +242,7 @@ class WikiPage < ApplicationRecord
   def validate_not_locked
     if is_locked? && !CurrentUser.is_janitor?
       errors.add(:is_locked, "and cannot be updated")
-      return false
+      false
     end
   end
 
@@ -203,7 +289,12 @@ class WikiPage < ApplicationRecord
   end
 
   def normalize_title
-    self.title = title.downcase.tr(" ", "_")
+    title = self.title.downcase.tr(" ", "_")
+    if title =~ /\A(#{Tag.categories.regexp}):(.+)\Z/
+      self.category_id = Tag.categories.value_for($1)
+      title = $2
+    end
+    self.title = title
   end
 
   def normalize_other_names
@@ -226,16 +317,12 @@ class WikiPage < ApplicationRecord
     @skip_secondary_validations = value.to_s.truthy?
   end
 
-  def category_id
-    Tag.category_for(title)
-  end
-
   def pretty_title
-    title&.tr("_", " ") || ''
+    title&.tr("_", " ") || ""
   end
 
   def pretty_title_with_category
-    return pretty_title if category_id == 0
+    return pretty_title if category_id.blank? || category_id == 0
     "#{Tag.category_for_value(category_id)}: #{pretty_title}"
   end
 
@@ -274,10 +361,6 @@ class WikiPage < ApplicationRecord
       else
         match
       end
-    end.map {|x| x.downcase.tr(" ", "_").to_s}.uniq
+    end.map { |x| x.downcase.tr(" ", "_").to_s }.uniq
-  end
-
-  def visible?
-    true
   end
 end

app/models/wiki_page_version.rb

@@ -6,7 +6,6 @@ class WikiPageVersion < ApplicationRecord
   belongs_to_updater
   user_status_counter :wiki_edit_count, foreign_key: :updater_id
   belongs_to :artist, optional: true
-  delegate :visible?, to: :wiki_page
 
   module SearchMethods
     def for_user(user_id)

app/views/wiki_page_versions/diff.html.erb

@@ -2,7 +2,6 @@
   <div id="a-diff">
     <h1>Wiki Page: <%= @thispage.title %></h1>
 
-    <% if @thispage.visible? %>
     <p>Showing differences between <%= compact_time @thispage.updated_at %> (<%= link_to_user @thispage.updater %>) and <%= compact_time @otherpage.updated_at %> (<%= link_to_user @otherpage.updater %>)</p>
 
     <% if @thispage.parent != @otherpage.parent %>
@@ -16,9 +15,6 @@
     <div>
       <%= text_diff(@thispage.body, @otherpage.body) %>
     </div>
-    <% else %>
-      <p>The artist requested removal of this page.</p>
-    <% end %>
   </div>
 </div>
 

app/views/wiki_page_versions/show.html.erb

@@ -10,11 +10,7 @@
       <% end %>
 
       <div id="wiki-page-body" class="dtext dtext-container">
-        <% if @wiki_page_version.visible? %>
         <%= format_text(@wiki_page_version.body) %>
-        <% else %>
-          <p>The artist has requested removal of this page.</p>
-        <% end %>
       </div>
     </section>
   </div>

app/views/wiki_pages/_form.html.erb

@@ -12,22 +12,28 @@
 
     <%= f.input :body, as: :dtext, limit: Danbooru.config.wiki_page_max_size, allow_color: true %>
 
+    <%= f.input :category_id, 
+      label: "Tag Category", 
+      collection: TagCategory::CANONICAL_MAPPING.to_a, 
+      include_blank: true, 
+      disabled: @wiki_page.category_is_locked && !CurrentUser.is_admin? %>
+
+    <% if CurrentUser.is_admin? %>
+      <%= f.input :category_is_locked, label: "Lock Category", as: :boolean %>
+    <% end %>
+
     <%= f.input :parent, label: "Redirects to", autocomplete: "wiki-page", input_html: { disabled: !CurrentUser.is_privileged? } %>
 
-    <% if CurrentUser.is_janitor? && @wiki_page.is_deleted? %>
-      <%= f.input :is_deleted, :label => "Deleted", :hint => "Uncheck to restore this wiki page" %>
-    <% end %>
-
     <% if CurrentUser.is_janitor? %>
-      <%= f.input :is_locked, :label => "Locked" %>
+      <%= f.input :is_locked, :label => "Lock Page" %>
     <% end %>
 
-    <%= f.input :edit_reason, label: "Edit Reason" %>
-
     <% if CurrentUser.is_janitor? && @wiki_page.errors[:title]&.any? { |error| error.include?("Move the posts and update any wikis linking to this page first.") }  %>
       <%= f.input :skip_secondary_validations, as: :boolean, label: "Force rename", hint: "Ignore the renaming requirements" %>
     <% end %>
 
+    <%= f.input :edit_reason, label: "Edit Reason" %>
+
     <%= f.button :submit, "Submit" %>
   <% end %>
 </div>

app/views/wiki_pages/_secondary_links.html.erb

@@ -17,13 +17,10 @@
     <% if @wiki_page.tag.present? %>
       <%= subnav_link_to "Posts (#{@wiki_page.tag.post_count})", posts_path(tags: @wiki_page.title) %>
 
-      <% if CurrentUser.is_member? %>
+      <% if CurrentUser.is_janitor?%>
-        <%= subnav_link_to "Edit Tag Type", edit_tag_path(@wiki_page.tag) %>
-        <% if CurrentUser.is_janitor? %>
         <%= subnav_link_to "Fix Tag Count", new_tag_correction_path(tag_id: @wiki_page.tag.id) %>
       <% end %>
     <% end %>
-    <% end %>
 
     <% if @wiki_page.persisted? %>
       <%= subnav_link_to "History", wiki_page_versions_path(search: { wiki_page_id: @wiki_page.id }) %>
@@ -40,6 +37,8 @@
         <%= subnav_link_to "Report", new_ticket_path(disp_id: @wiki_page.id, qtype: "wiki") %>
       <% end %>
     <% end %>
+
+
   <% elsif @wiki_page_version %>
     <li class="divider"></li>
 

app/views/wiki_pages/edit.html.erb

@@ -5,11 +5,7 @@
     <section id="content">
       <h1>Edit Wiki</h1>
 
-      <% if @wiki_page.visible? %>
       <%= render "form" %>
-      <% else %>
-        <p>The artist requested removal of this page.</p>
-      <% end %>
 
       <%= wiki_page_alias_and_implication_list(@wiki_page)%>
       <%= wiki_page_post_previews(@wiki_page) %>

app/views/wiki_pages/show.html.erb

@@ -21,7 +21,6 @@
       <% end %>
 
       <div id="wiki-page-body" class="dtext-container">
-        <% if wiki_content.visible? %>
         <%= format_text(wiki_content.body, allow_color: true, max_thumbs: 75) %>
 
         <% if wiki_content.artist %>
@@ -29,9 +28,6 @@
         <% end %>
 
         <%= wiki_page_alias_and_implication_list(wiki_content) %>
-        <% else %>
-          <p>This artist has requested removal of their information.</p>
-        <% end %>
       </div>
 
         <%= wiki_page_post_previews(wiki_content) %>

test/functional/wiki_pages_controller_test.rb

@@ -12,8 +12,8 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
     context "index action" do
       setup do
         as(@user) do
-          @wiki_page_abc = create(:wiki_page, :title => "abc")
+          @wiki_page_abc = create(:wiki_page, title: "abc")
-          @wiki_page_def = create(:wiki_page, :title => "def")
+          @wiki_page_def = create(:wiki_page, title: "def")
         end
       end
 
@@ -23,12 +23,12 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
       end
 
       should "list all wiki_pages (with search)" do
-        get wiki_pages_path, params: {:search => {:title => "abc"}}
+        get wiki_pages_path, params: { search: { title: "abc" } }
         assert_redirected_to(wiki_page_path(@wiki_page_abc))
       end
 
       should "list wiki_pages without tags with order=post_count" do
-        get wiki_pages_path, params: {:search => {:title => "abc", :order => "post_count"}}
+        get wiki_pages_path, params: { search: { title: "abc", order: "post_count" } }
         assert_redirected_to(wiki_page_path(@wiki_page_abc))
       end
     end
@@ -46,7 +46,7 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
       end
 
       should "render for a title" do
-        get wiki_page_path(:id => @wiki_page.title)
+        get wiki_page_path(id: @wiki_page.title)
         assert_response :success
       end
 
@@ -64,7 +64,7 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
         as(@user) do
           @wiki_page.update(title: "-aaa")
         end
-        get wiki_page_path(:id => @wiki_page.id)
+        get wiki_page_path(id: @wiki_page.id)
         assert_response :success
       end
     end
@@ -89,7 +89,7 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
 
     context "new action" do
       should "render" do
-        get_auth new_wiki_page_path, @mod, params: { wiki_page: { title: "test" }}
+        get_auth new_wiki_page_path, @mod, params: { wiki_page: { title: "test" } }
         assert_response :success
       end
     end
@@ -108,7 +108,55 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
     context "create action" do
       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"}}
+          post_auth wiki_pages_path, @user, params: { wiki_page: { title: "abc", body: "abc" } }
+        end
+      end
+
+      context "with prefix" do
+        should "work" do
+          assert_difference(%w[WikiPage.count Tag.count], 1) do
+            post_auth wiki_pages_path, @user, params: { wiki_page: { title: "character:abc", body: "abc" } }
+          end
+          @wiki = WikiPage.last
+          assert_equal("abc", @wiki.title)
+          assert_equal(Tag.categories.character, @wiki.category_id)
+        end
+
+        should "not work for disallowed prefixes" do
+          assert_no_difference("WikiPage.count") do
+            post_auth wiki_pages_path, @user, params: { wiki_page: { title: "lore:abc", body: "abc" } }
+          end
+        end
+
+        should "not work for tags over the threshold" do
+          @tag = create(:tag, post_count: 500)
+          assert_no_difference("WikiPage.count") do
+            post_auth wiki_pages_path, @user, params: { wiki_page: { title: "character:#{@tag.name}", body: "abc" } }
+          end
+        end
+      end
+
+      context "with category_id" do
+        should "work" do
+          assert_difference(%w[WikiPage.count Tag.count], 1) do
+            post_auth wiki_pages_path, @user, params: { wiki_page: { title: "abc", body: "abc", category_id: Tag.categories.character } }
+          end
+          @wiki = WikiPage.last
+          assert_equal("abc", @wiki.title)
+          assert_equal(Tag.categories.character, @wiki.category_id)
+        end
+
+        should "not work for disallowed categories" do
+          assert_no_difference("WikiPage.count") do
+            post_auth wiki_pages_path, @user, params: { wiki_page: { title: "abc", body: "abc", category_id: Tag.categories.lore } }
+          end
+        end
+
+        should "not work for tags over the threshold" do
+          @tag = create(:tag, post_count: 500)
+          assert_no_difference("WikiPage.count") do
+            post_auth wiki_pages_path, @user, params: { wiki_page: { title: @tag.name, body: "abc", category_id: Tag.categories.character } }
+          end
         end
       end
     end
@@ -122,18 +170,18 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
       end
 
       should "update a wiki_page" do
-        put_auth wiki_page_path(@wiki_page), @user, params: {:wiki_page => {:body => "xyz"}}
+        put_auth wiki_page_path(@wiki_page), @user, params: { wiki_page: { body: "xyz" } }
         @wiki_page.reload
         assert_equal("xyz", @wiki_page.body)
       end
 
       should "not rename a wiki page with a non-empty tag" do
-        put_auth wiki_page_path(@wiki_page), @user, params: {:wiki_page => {:title => "bar"}}
+        put_auth wiki_page_path(@wiki_page), @user, params: { wiki_page: { title: "bar"}}
         assert_equal("foo", @wiki_page.reload.title)
       end
 
       should "rename a wiki page with a non-empty tag if secondary validations are skipped" do
-        put_auth wiki_page_path(@wiki_page), @mod, params: {:wiki_page => {:title => "bar", :skip_secondary_validations => "1"}}
+        put_auth wiki_page_path(@wiki_page), @mod, params: { wiki_page: { title: "bar", skip_secondary_validations: "1" } }
         assert_equal("bar", @wiki_page.reload.title)
       end
 
@@ -141,6 +189,27 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
         put_auth wiki_page_path(@wiki_page), @user, params: {wiki_page: { is_deleted: true }}
         assert_equal(false, @wiki_page.reload.is_deleted?)
       end
+
+      context "with category_id" do
+        should "work" do
+          put_auth wiki_page_path(@wiki_page), @user, params: { wiki_page: { category_id: Tag.categories.character } }
+          @wiki_page.reload
+          assert_equal(Tag.categories.character, @wiki_page.category_id)
+        end
+
+        should "not work for disallowed categories" do
+          put_auth wiki_page_path(@wiki_page), @user, params: { wiki_page: { category_id: Tag.categories.lore } }
+          @wiki_page.reload
+          assert_equal(Tag.categories.general, @wiki_page.category_id)
+        end
+
+        should "not work for tags over the threshold" do
+          @tag.update_column(:post_count, 500)
+          put_auth wiki_page_path(@wiki_page), @user, params: { wiki_page: { category_id: Tag.categories.character } }
+          @wiki_page.reload
+          assert_equal(Tag.categories.general, @wiki_page.category_id)
+        end
+      end
     end
 
     context "destroy action" do
@@ -172,7 +241,7 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
       should "revert to a previous version" do
         version = @wiki_page.versions.first
         assert_equal("1", version.body)
-        put_auth revert_wiki_page_path(@wiki_page), @user, params: {:version_id => version.id}
+        put_auth revert_wiki_page_path(@wiki_page), @user, params: { version_id: version.id }
         @wiki_page.reload
         assert_equal("1", @wiki_page.body)
       end
@@ -182,7 +251,7 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest
           @wiki_page_2 = create(:wiki_page)
         end
 
-        put_auth revert_wiki_page_path(@wiki_page), @user, params: { :version_id => @wiki_page_2.versions.first.id }
+        put_auth revert_wiki_page_path(@wiki_page), @user, params: { version_id: @wiki_page_2.versions.first.id }
         @wiki_page.reload
 
         assert_not_equal(@wiki_page.body, @wiki_page_2.body)