diff --git a/app/controllers/wiki_pages_controller.rb b/app/controllers/wiki_pages_controller.rb index d586e4d9b..74b533b2c 100644 --- a/app/controllers/wiki_pages_controller.rb +++ b/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) diff --git a/app/models/tag.rb b/app/models/tag.rb index 62db4a89c..fc304d381 100644 --- a/app/models/tag.rb +++ b/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 = {} diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index a0ae4c1cd..a5c6d551e 100644 --- a/app/models/wiki_page.rb +++ b/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 - - def visible? - true + end.map { |x| x.downcase.tr(" ", "_").to_s }.uniq end end diff --git a/app/models/wiki_page_version.rb b/app/models/wiki_page_version.rb index e0e42536e..39830e612 100644 --- a/app/models/wiki_page_version.rb +++ b/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) diff --git a/app/views/wiki_page_versions/diff.html.erb b/app/views/wiki_page_versions/diff.html.erb index 0d739e392..141136533 100644 --- a/app/views/wiki_page_versions/diff.html.erb +++ b/app/views/wiki_page_versions/diff.html.erb @@ -2,23 +2,19 @@

Wiki Page: <%= @thispage.title %>

- <% if @thispage.visible? %> -

Showing differences between <%= compact_time @thispage.updated_at %> (<%= link_to_user @thispage.updater %>) and <%= compact_time @otherpage.updated_at %> (<%= link_to_user @otherpage.updater %>)

+

Showing differences between <%= compact_time @thispage.updated_at %> (<%= link_to_user @thispage.updater %>) and <%= compact_time @otherpage.updated_at %> (<%= link_to_user @otherpage.updater %>)

- <% if @thispage.parent != @otherpage.parent %> -
- Page redirect changed - from <%= @thispage.parent.blank? ? "none" : link_to(@thispage.parent, show_or_new_wiki_pages_path(title: @thispage.parent)) %> - to <%= @otherpage.parent.blank? ? "none" : link_to(@otherpage.parent, show_or_new_wiki_pages_path(title: @otherpage.parent)) %>. -
- <% end %> - -
- <%= text_diff(@thispage.body, @otherpage.body) %> + <% if @thispage.parent != @otherpage.parent %> +
+ Page redirect changed + from <%= @thispage.parent.blank? ? "none" : link_to(@thispage.parent, show_or_new_wiki_pages_path(title: @thispage.parent)) %> + to <%= @otherpage.parent.blank? ? "none" : link_to(@otherpage.parent, show_or_new_wiki_pages_path(title: @otherpage.parent)) %>.
- <% else %> -

The artist requested removal of this page.

<% end %> + +
+ <%= text_diff(@thispage.body, @otherpage.body) %> +
diff --git a/app/views/wiki_page_versions/show.html.erb b/app/views/wiki_page_versions/show.html.erb index 3e5553358..a8d575b16 100644 --- a/app/views/wiki_page_versions/show.html.erb +++ b/app/views/wiki_page_versions/show.html.erb @@ -10,11 +10,7 @@ <% end %>
- <% if @wiki_page_version.visible? %> - <%= format_text(@wiki_page_version.body) %> - <% else %> -

The artist has requested removal of this page.

- <% end %> + <%= format_text(@wiki_page_version.body) %>
diff --git a/app/views/wiki_pages/_form.html.erb b/app/views/wiki_pages/_form.html.erb index f7d8ebf5c..280c64258 100644 --- a/app/views/wiki_pages/_form.html.erb +++ b/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 %> diff --git a/app/views/wiki_pages/_secondary_links.html.erb b/app/views/wiki_pages/_secondary_links.html.erb index 47dc049bd..4eaa3112d 100644 --- a/app/views/wiki_pages/_secondary_links.html.erb +++ b/app/views/wiki_pages/_secondary_links.html.erb @@ -17,11 +17,8 @@ <% if @wiki_page.tag.present? %> <%= subnav_link_to "Posts (#{@wiki_page.tag.post_count})", posts_path(tags: @wiki_page.title) %> - <% if CurrentUser.is_member? %> - <%= 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 %> + <% if CurrentUser.is_janitor?%> + <%= subnav_link_to "Fix Tag Count", new_tag_correction_path(tag_id: @wiki_page.tag.id) %> <% end %> <% end %> @@ -40,6 +37,8 @@ <%= subnav_link_to "Report", new_ticket_path(disp_id: @wiki_page.id, qtype: "wiki") %> <% end %> <% end %> + + <% elsif @wiki_page_version %>
  • diff --git a/app/views/wiki_pages/edit.html.erb b/app/views/wiki_pages/edit.html.erb index 63b9f3549..131e2c181 100644 --- a/app/views/wiki_pages/edit.html.erb +++ b/app/views/wiki_pages/edit.html.erb @@ -5,11 +5,7 @@

    Edit Wiki

    - <% if @wiki_page.visible? %> - <%= render "form" %> - <% else %> -

    The artist requested removal of this page.

    - <% end %> + <%= render "form" %> <%= wiki_page_alias_and_implication_list(@wiki_page)%> <%= wiki_page_post_previews(@wiki_page) %> diff --git a/app/views/wiki_pages/show.html.erb b/app/views/wiki_pages/show.html.erb index 389451b0f..8e7e954b8 100644 --- a/app/views/wiki_pages/show.html.erb +++ b/app/views/wiki_pages/show.html.erb @@ -21,17 +21,13 @@ <% end %>
    - <% if wiki_content.visible? %> - <%= format_text(wiki_content.body, allow_color: true, max_thumbs: 75) %> + <%= format_text(wiki_content.body, allow_color: true, max_thumbs: 75) %> - <% if wiki_content.artist %> -

    <%= link_to "View artist", wiki_content.artist %>

    - <% end %> - - <%= wiki_page_alias_and_implication_list(wiki_content) %> - <% else %> -

    This artist has requested removal of their information.

    + <% if wiki_content.artist %> +

    <%= link_to "View artist", wiki_content.artist %>

    <% end %> + + <%= wiki_page_alias_and_implication_list(wiki_content) %>
    <%= wiki_page_post_previews(wiki_content) %> diff --git a/test/functional/wiki_pages_controller_test.rb b/test/functional/wiki_pages_controller_test.rb index ca29c2462..15ddf5dfe 100644 --- a/test/functional/wiki_pages_controller_test.rb +++ b/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_def = create(:wiki_page, :title => "def") + @wiki_page_abc = create(:wiki_page, title: "abc") + @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)