Skip to content

Commit

Permalink
Do not use redirects_to_external? method
Browse files Browse the repository at this point in the history
This method has been deprecated and since we do not want to log
deprecation warnings we must not use it.
  • Loading branch information
tvdeyen committed Feb 25, 2020
1 parent 45aa4ad commit 15e0880
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 37 deletions.
4 changes: 2 additions & 2 deletions app/controllers/alchemy/admin/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/alchemy/pages_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions app/models/alchemy/page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions app/models/alchemy/page/page_naming.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,29 @@ 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
validates :urlname,
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,
if: :should_update_descendants_urlnames?

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.
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions app/serializers/alchemy/page_tree_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion app/views/alchemy/admin/pages/_page.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
) -%>
<label class="center"><%= Alchemy.t(:edit_page_properties) %></label>
Expand Down
2 changes: 1 addition & 1 deletion app/views/alchemy/admin/pages/info.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<p><%= @page.layout_display_name %></p>
</div>
<div class="value">
<% if @page.redirects_to_external? %>
<% if @page.definition['redirects_to_external'] %>
<label><%= Alchemy::Page.human_attribute_name(:urlname) %></label>
<p><%= @page.urlname %></p>
<% else %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/alchemy/navigation/_link.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% if page.redirects_to_external? %>
<% if page.definition['redirects_to_external'] %>
<%= link_to(
h(page.name),
page.external_urlname,
Expand Down
8 changes: 4 additions & 4 deletions lib/tasks/alchemy/convert.rake
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ 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
end
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
Expand All @@ -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 "."
Expand Down
32 changes: 18 additions & 14 deletions spec/models/alchemy/page_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions spec/requests/alchemy/admin/pages_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 15e0880

Please sign in to comment.