From be9bdc0ab3df1ab82976643ad703dd2f6e69055f Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 16 Dec 2019 18:32:48 -0600 Subject: [PATCH] wiki pages: warn when renaming wikis still linked from other wikis. * Warn when renaming a wiki that still has links from other wikis. * When renaming a wiki that still has posts, just show a warning instead of returning an error and making the user confirm the rename. --- app/controllers/wiki_pages_controller.rb | 4 +++- app/models/wiki_page.rb | 21 ++++++++++++------- app/views/wiki_pages/_form.html.erb | 4 ---- test/functional/wiki_pages_controller_test.rb | 11 +++------- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/app/controllers/wiki_pages_controller.rb b/app/controllers/wiki_pages_controller.rb index 4dbaf1b29..607e35a30 100644 --- a/app/controllers/wiki_pages_controller.rb +++ b/app/controllers/wiki_pages_controller.rb @@ -52,6 +52,8 @@ class WikiPagesController < ApplicationController def update @wiki_page, _ = WikiPage.find_by_id_or_title(params[:id]) @wiki_page.update(wiki_page_params(:update)) + flash[:notice] = @wiki_page.warnings.full_messages.join(".\n \n") if @wiki_page.warnings.any? + respond_with(@wiki_page) end @@ -87,7 +89,7 @@ class WikiPagesController < ApplicationController end def wiki_page_params(context) - permitted_params = %i[body other_names other_names_string skip_secondary_validations] + permitted_params = %i[body other_names other_names_string] permitted_params += %i[is_locked is_deleted] if CurrentUser.is_builder? permitted_params += %i[title] if context == :create || CurrentUser.is_builder? diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 974a7b4dc..ec70f2049 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -13,7 +13,6 @@ class WikiPage < ApplicationRecord validate :validate_rename validate :validate_not_locked - attr_accessor :skip_secondary_validations array_attribute :other_names has_one :tag, :foreign_key => "name", :primary_key => "title" has_one :artist, -> {where(:is_active => true)}, :foreign_key => "name", :primary_key => "title" @@ -62,6 +61,10 @@ class WikiPage < ApplicationRecord where(title: Tag.search(params).select(:name).reorder(nil)) end + def linked_to(title) + where(id: DtextLink.wiki_page.wiki_link.where(link_target: title).select(:model_id)) + end + def default_order order(updated_at: :desc) end @@ -85,7 +88,7 @@ class WikiPage < ApplicationRecord end if params[:linked_to].present? - q = q.where(id: DtextLink.wiki_page.wiki_link.where(link_target: params[:linked_to]).select(:model_id)) + q = q.linked_to(params[:linked_to]) end if params[:hide_deleted].to_s.truthy? @@ -121,11 +124,17 @@ class WikiPage < ApplicationRecord end def validate_rename - return if !will_save_change_to_title? || skip_secondary_validations + return unless title_changed? tag_was = Tag.find_by_name(Tag.normalize_name(title_was)) if tag_was.present? && tag_was.post_count > 0 - errors.add(:title, "cannot be changed: '#{tag_was.name}' still has #{tag_was.post_count} posts. Move the posts and update any wikis linking to this page first.") + warnings[:base] << %Q!Warning: {{#{title_was}}} still has #{tag_was.post_count} #{"post".pluralize(tag_was.post_count)}. Be sure to move the posts! + end + + broken_wikis = WikiPage.linked_to(title_was) + if broken_wikis.count > 0 + broken_wiki_search = Rails.application.routes.url_helpers.wiki_pages_path(search: { linked_to: title_was }) + warnings[:base] << %Q!Warning: [[#{title_was}]] is still linked from "#{broken_wikis.count} #{"other wiki page".pluralize(broken_wikis.count)}":[#{broken_wiki_search}]. Update #{broken_wikis.count > 1 ? "these wikis" : "this wiki"} to link to [[#{title}]] instead! end end @@ -161,10 +170,6 @@ class WikiPage < ApplicationRecord name.unicode_normalize(:nfkc).gsub(/[[:space:]]+/, " ").strip.tr(" ", "_") end - def skip_secondary_validations=(value) - @skip_secondary_validations = value.to_s.truthy? - end - def category_name Tag.category_for(title) end diff --git a/app/views/wiki_pages/_form.html.erb b/app/views/wiki_pages/_form.html.erb index 9d02f1458..ad1c285c8 100644 --- a/app/views/wiki_pages/_form.html.erb +++ b/app/views/wiki_pages/_form.html.erb @@ -20,10 +20,6 @@ <%= f.input :is_locked, :label => "Locked" %> <% end %> - <% if @wiki_page.errors[:title].present? %> - <%= f.input :skip_secondary_validations, as: :boolean, label: "Force rename", hint: "Ignore the renaming requirements" %> - <% end %> - <%= f.submit "Submit" %> <%= dtext_preview_button "wiki_page", "body" %> <% end %> diff --git a/test/functional/wiki_pages_controller_test.rb b/test/functional/wiki_pages_controller_test.rb index 9de94df07..4a28c5a4f 100644 --- a/test/functional/wiki_pages_controller_test.rb +++ b/test/functional/wiki_pages_controller_test.rb @@ -164,14 +164,9 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest 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"}} - 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"}} - assert_equal("bar", @wiki_page.reload.title) + should "warn about renaming a wiki page with a non-empty tag" do + put_auth wiki_page_path(@wiki_page), @mod, params: { wiki_page: { title: "bar" }} + assert_match(/still has 42 posts/, flash[:notice]) end should "not allow non-Builders to delete wiki pages" do