From 5e4667a93cab64b4b1eece7358356eb8524f8ccf Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Mon, 24 Feb 2020 18:04:13 +0100 Subject: [PATCH 1/2] Do not use redirects_to_external? method This method has been deprecated and since we do not want to log deprecation warnings we must not use it. --- .../alchemy/admin/pages_controller.rb | 4 +-- app/helpers/alchemy/pages_helper.rb | 2 +- app/models/alchemy/page.rb | 4 +-- app/models/alchemy/page/page_naming.rb | 12 +++---- .../alchemy/page_tree_serializer.rb | 4 +-- app/views/alchemy/admin/pages/_page.html.erb | 2 +- app/views/alchemy/admin/pages/info.html.erb | 2 +- app/views/alchemy/navigation/_link.html.erb | 2 +- lib/tasks/alchemy/convert.rake | 8 ++--- spec/models/alchemy/page_spec.rb | 32 +++++++++++-------- .../alchemy/admin/pages_controller_spec.rb | 6 ++-- 11 files changed, 41 insertions(+), 37 deletions(-) diff --git a/app/controllers/alchemy/admin/pages_controller.rb b/app/controllers/alchemy/admin/pages_controller.rb index 4e89eb8040..97475c4dae 100644 --- a/app/controllers/alchemy/admin/pages_controller.rb +++ b/app/controllers/alchemy/admin/pages_controller.rb @@ -93,7 +93,7 @@ def edit # Set page configuration like page names, meta tags and states. def configure @page_layouts = PageLayout.layouts_with_own_for_select(@page.page_layout, Language.current.id, @page.layoutpage?) - render @page.redirects_to_external? ? 'configure_external' : 'configure' + render @page.definition['redirects_to_external'] ? 'configure_external' : 'configure' end # Updates page @@ -330,7 +330,7 @@ def pages_from_raw_request end def redirect_path_after_create_page - if @page.redirects_to_external? || !@page.editable_by?(current_alchemy_user) + if @page.definition['redirects_to_external'] || !@page.editable_by?(current_alchemy_user) admin_pages_path else params[:redirect_to] || edit_admin_page_path(@page) diff --git a/app/helpers/alchemy/pages_helper.rb b/app/helpers/alchemy/pages_helper.rb index b1b8947ae5..8c2f02e7fa 100644 --- a/app/helpers/alchemy/pages_helper.rb +++ b/app/helpers/alchemy/pages_helper.rb @@ -244,7 +244,7 @@ def page_active?(page) # Returns +'active'+ if the given external page is in the current url path or +nil+. def external_page_css_class(page) - return nil if !page.redirects_to_external? + return nil if !page.definition['redirects_to_external'] request.path.split('/').delete_if(&:blank?).first == page.urlname.gsub(/^\//, '') ? 'active' : nil end diff --git a/app/models/alchemy/page.rb b/app/models/alchemy/page.rb index 84d637c142..2bdab0c540 100644 --- a/app/models/alchemy/page.rb +++ b/app/models/alchemy/page.rb @@ -143,7 +143,7 @@ class Page < BaseRecord after_update :create_legacy_url, if: :should_create_legacy_url?, - unless: :redirects_to_external? + unless: -> { definition['redirects_to_external'] } after_update :attach_to_menu!, if: :should_attach_to_menu? @@ -470,7 +470,7 @@ def publish! def update_node!(node) hash = {lft: node.left, rgt: node.right, parent_id: node.parent, depth: node.depth, restricted: node.restricted} - if Config.get(:url_nesting) && !redirects_to_external? && urlname != node.url + if Config.get(:url_nesting) && !definition['redirects_to_external'] && urlname != node.url LegacyPageUrl.create(page_id: id, urlname: urlname) hash[:urlname] = node.url end diff --git a/app/models/alchemy/page/page_naming.rb b/app/models/alchemy/page/page_naming.rb index d5d9c652ea..489259252a 100644 --- a/app/models/alchemy/page/page_naming.rb +++ b/app/models/alchemy/page/page_naming.rb @@ -9,7 +9,7 @@ module Page::PageNaming included do before_validation :set_urlname, if: :renamed?, - unless: -> { systempage? || redirects_to_external? || name.blank? } + unless: -> { systempage? || definition['redirects_to_external'] || name.blank? } validates :name, presence: true @@ -17,13 +17,13 @@ module Page::PageNaming uniqueness: {scope: [:language_id, :layoutpage], if: -> { urlname.present? }}, exclusion: {in: RESERVED_URLNAMES}, length: {minimum: 3, if: -> { urlname.present? }}, - format: {with: /\A[:\.\w\-+_\/\?&%;=]*\z/, if: :redirects_to_external?} + format: {with: /\A[:\.\w\-+_\/\?&%;=]*\z/, if: -> { definition['redirects_to_external'] }} validates :urlname, on: :update, - presence: {if: :redirects_to_external?} + presence: {if: -> { definition['redirects_to_external'] }} before_save :set_title, - unless: -> { systempage? || redirects_to_external? }, + unless: -> { systempage? || definition['redirects_to_external'] }, if: -> { title.blank? } after_update :update_descendants_urlnames, @@ -31,7 +31,7 @@ module Page::PageNaming after_move :update_urlname!, if: -> { Config.get(:url_nesting) }, - unless: :redirects_to_external? + unless: -> { definition['redirects_to_external'] } end # Returns true if name or urlname has changed. @@ -86,7 +86,7 @@ def should_update_descendants_urlnames? def update_descendants_urlnames reload descendants.each do |descendant| - next if descendant.redirects_to_external? + next if descendant.definition['redirects_to_external'] descendant.update_urlname! end end diff --git a/app/serializers/alchemy/page_tree_serializer.rb b/app/serializers/alchemy/page_tree_serializer.rb index a1edc2daec..d831a7fbca 100644 --- a/app/serializers/alchemy/page_tree_serializer.rb +++ b/app/serializers/alchemy/page_tree_serializer.rb @@ -57,9 +57,9 @@ def page_hash(page, has_children, level, folded) restricted: page.restricted?, page_layout: page.page_layout, slug: page.slug, - redirects_to_external: page.redirects_to_external?, + redirects_to_external: page.definition['redirects_to_external'], urlname: page.urlname, - external_urlname: page.redirects_to_external? ? page.external_urlname : nil, + external_urlname: page.definition['redirects_to_external'] ? page.external_urlname : nil, level: level, root: level == 1, root_or_leaf: level == 1 || !has_children, diff --git a/app/views/alchemy/admin/pages/_page.html.erb b/app/views/alchemy/admin/pages/_page.html.erb index 1e93b487ba..d063dcb876 100644 --- a/app/views/alchemy/admin/pages/_page.html.erb +++ b/app/views/alchemy/admin/pages/_page.html.erb @@ -71,7 +71,7 @@ alchemy.configure_admin_page_path(page), { title: Alchemy.t(:edit_page_properties), - size: page.redirects_to_external? ? '450x330' : '450x680' + size: page.definition['redirects_to_external'] ? '450x330' : '450x680' } ) -%> diff --git a/app/views/alchemy/admin/pages/info.html.erb b/app/views/alchemy/admin/pages/info.html.erb index 0aaa7449d6..377b431a10 100644 --- a/app/views/alchemy/admin/pages/info.html.erb +++ b/app/views/alchemy/admin/pages/info.html.erb @@ -14,7 +14,7 @@

<%= @page.layout_display_name %>

- <% if @page.redirects_to_external? %> + <% if @page.definition['redirects_to_external'] %>

<%= @page.urlname %>

<% else %> diff --git a/app/views/alchemy/navigation/_link.html.erb b/app/views/alchemy/navigation/_link.html.erb index 18e4e6aa64..bcb63d2d18 100644 --- a/app/views/alchemy/navigation/_link.html.erb +++ b/app/views/alchemy/navigation/_link.html.erb @@ -1,4 +1,4 @@ -<% if page.redirects_to_external? %> +<% if page.definition['redirects_to_external'] %> <%= link_to( h(page.name), page.external_urlname, diff --git a/lib/tasks/alchemy/convert.rake b/lib/tasks/alchemy/convert.rake index 1039ed00a6..690600118e 100644 --- a/lib/tasks/alchemy/convert.rake +++ b/lib/tasks/alchemy/convert.rake @@ -40,7 +40,7 @@ namespace :alchemy do end def name_for_node(page) - if page.visible? && page.public? && !page.redirects_to_external? + if page.visible? && page.public? && !page.definition['redirects_to_external'] nil else page.name @@ -48,7 +48,7 @@ namespace :alchemy do end def page_for_node(page) - if page.visible? && page.public? && !page.redirects_to_external? + if page.visible? && page.public? && !page.definition['redirects_to_external'] page elsif Alchemy::Config.get(:redirect_to_public_child) && page.visible? && !page.public? && page.children.published.any? page.children.published.first @@ -64,8 +64,8 @@ namespace :alchemy do new_node = node.children.create!( name: name_for_node(page), page: page_for_node(page), - url: page.redirects_to_external? ? page.urlname : nil, - external: page.redirects_to_external? && Alchemy::Config.get(:open_external_links_in_new_tab), + url: page.definition['redirects_to_external'] ? page.urlname : nil, + external: page.definition['redirects_to_external'] && Alchemy::Config.get(:open_external_links_in_new_tab), language_id: page.language_id ) print "." diff --git a/spec/models/alchemy/page_spec.rb b/spec/models/alchemy/page_spec.rb index 0ace3ba976..16ba87e02f 100644 --- a/spec/models/alchemy/page_spec.rb +++ b/spec/models/alchemy/page_spec.rb @@ -1835,10 +1835,6 @@ module Alchemy end context "when page is not external" do - before do - expect(page).to receive(:redirects_to_external?).and_return(false) - end - it "should update all attributes" do page.update_node!(node) page.reload @@ -1870,10 +1866,15 @@ module Alchemy end context "when page is external" do - before do - expect(page) - .to receive(:redirects_to_external?) - .and_return(true) + let(:page) do + create( + :alchemy_page, + language: language, + parent_id: language_root.id, + urlname: original_url, + restricted: false, + page_layout: 'external' + ) end it "should update all attributes except url" do @@ -1901,10 +1902,6 @@ module Alchemy end context "when page is not external" do - before do - allow(page).to receive(:redirects_to_external?).and_return(false) - end - it "should update all attributes except url" do page.update_node!(node) page.reload @@ -1924,8 +1921,15 @@ module Alchemy end context "when page is external" do - before do - allow(page).to receive(:redirects_to_external?).and_return(true) + let(:page) do + create( + :alchemy_page, + language: language, + parent_id: language_root.id, + urlname: original_url, + restricted: false, + page_layout: 'external' + ) end it "should update all attributes except url" do diff --git a/spec/requests/alchemy/admin/pages_controller_spec.rb b/spec/requests/alchemy/admin/pages_controller_spec.rb index 16a5f99da6..b118d4a29e 100644 --- a/spec/requests/alchemy/admin/pages_controller_spec.rb +++ b/spec/requests/alchemy/admin/pages_controller_spec.rb @@ -292,9 +292,9 @@ module Alchemy let(:page_1) { create(:alchemy_page, visible: true) } let(:page_2) { create(:alchemy_page, visible: true) } let(:page_3) { create(:alchemy_page, visible: true) } - let(:page_item_1) { {id: page_1.id, slug: page_1.slug, restricted: false, external: page_1.redirects_to_external?, visible: page_1.visible?, children: [page_item_2]} } - let(:page_item_2) { {id: page_2.id, slug: page_2.slug, restricted: false, external: page_2.redirects_to_external?, visible: page_2.visible?, children: [page_item_3]} } - let(:page_item_3) { {id: page_3.id, slug: page_3.slug, restricted: false, external: page_3.redirects_to_external?, visible: page_3.visible? } } + let(:page_item_1) { {id: page_1.id, slug: page_1.slug, restricted: false, external: page_1.definition['redirects_to_external'], visible: page_1.visible?, children: [page_item_2]} } + let(:page_item_2) { {id: page_2.id, slug: page_2.slug, restricted: false, external: page_2.definition['redirects_to_external'], visible: page_2.visible?, children: [page_item_3]} } + let(:page_item_3) { {id: page_3.id, slug: page_3.slug, restricted: false, external: page_3.definition['redirects_to_external'], visible: page_3.visible? } } let(:set_of_pages) { [page_item_1] } it "stores the new order" do From ad31bc3a3568f38a70cf1b1d922c4dc357aad8f8 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 25 Feb 2020 12:24:28 +0100 Subject: [PATCH 2/2] Do not use has_controller? method This method has been deprecated and we do not want to use it internally to avoid loggin deprecation warnings. --- app/controllers/concerns/alchemy/page_redirects.rb | 2 +- app/models/alchemy/page/page_natures.rb | 2 +- spec/models/alchemy/page_spec.rb | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/controllers/concerns/alchemy/page_redirects.rb b/app/controllers/concerns/alchemy/page_redirects.rb index d9b4f6add5..32a3b1eeaa 100644 --- a/app/controllers/concerns/alchemy/page_redirects.rb +++ b/app/controllers/concerns/alchemy/page_redirects.rb @@ -49,7 +49,7 @@ def public_child_redirect_url end def controller_and_action_url - return unless @page.has_controller? + return unless @page.definition['controller'] main_app.url_for(@page.controller_and_action) end diff --git a/app/models/alchemy/page/page_natures.rb b/app/models/alchemy/page/page_natures.rb index 8f796104d7..9e361028e8 100644 --- a/app/models/alchemy/page/page_natures.rb +++ b/app/models/alchemy/page/page_natures.rb @@ -74,7 +74,7 @@ def locked? # @deprecated Please use a menu node with an url pointing to your controller path instead. def controller_and_action - if has_controller? + if definition['controller'] { controller: definition["controller"].gsub(/(^\b)/, "/#{$1}"), action: definition["action"] diff --git a/spec/models/alchemy/page_spec.rb b/spec/models/alchemy/page_spec.rb index 16ba87e02f..528fedf85d 100644 --- a/spec/models/alchemy/page_spec.rb +++ b/spec/models/alchemy/page_spec.rb @@ -2211,7 +2211,6 @@ module Alchemy context 'if the page has a custom controller defined in its definition' do before do - allow(page).to receive(:has_controller?).and_return(true) allow(page).to receive(:definition).and_return({'controller' => 'comments', 'action' => 'index'}) end it "should return a Hash with controller and action key-value pairs" do