Skip to content

Commit

Permalink
Remove the url_nesting config option
Browse files Browse the repository at this point in the history
And always created nested urls.
  • Loading branch information
tvdeyen committed May 27, 2020
1 parent 2f15aba commit 21a1028
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 264 deletions.
2 changes: 1 addition & 1 deletion app/models/alchemy/page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,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) && urlname != node.url
if urlname != node.url
LegacyPageUrl.create(page_id: id, urlname: urlname)
hash[:urlname] = node.url
end
Expand Down
18 changes: 5 additions & 13 deletions app/models/alchemy/page/page_naming.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ module Page::PageNaming
after_update :update_descendants_urlnames,
if: :should_update_descendants_urlnames?

after_move :update_urlname!,
if: -> { Config.get(:url_nesting) }
after_move :update_urlname!
end

# Returns true if name or urlname has changed.
Expand Down Expand Up @@ -64,8 +63,6 @@ def visible_ancestors
private

def should_update_descendants_urlnames?
return false if !Config.get(:url_nesting)

saved_change_to_urlname? || saved_change_to_visible?
end

Expand All @@ -76,14 +73,9 @@ def update_descendants_urlnames

# Sets the urlname to a url friendly slug.
# Either from name, or if present, from urlname.
# If url_nesting is enabled the urlname contains the whole path.
# The urlname contains the whole path including parent urlnames.
def set_urlname
if Config.get(:url_nesting)
value = slug
else
value = urlname
end
self[:urlname] = nested_url_name(value)
self[:urlname] = nested_url_name(slug)
end

def set_title
Expand All @@ -110,9 +102,9 @@ def nested_url_name(value)

# Slugs of all visible/non-language_root ancestors.
# Returns [], if there is no parent, the parent is
# the root page itself, or url_nesting is off.
# the root page itself.
def ancestor_slugs
return [] if !Config.get(:url_nesting) || parent.nil?
return [] if parent.nil?

visible_ancestors.map(&:slug).compact
end
Expand Down
4 changes: 1 addition & 3 deletions lib/alchemy/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ def show
# a value of nil means there is no new default
# any not nil value is the new default
def deprecated_configs
{
url_nesting: true,
}
{}
end

private
Expand Down
40 changes: 0 additions & 40 deletions lib/tasks/alchemy/convert.rake

This file was deleted.

39 changes: 19 additions & 20 deletions spec/controllers/alchemy/pages_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module Alchemy
it "returns a 404" do
expect { get(:index) }.to raise_exception(
ActionController::RoutingError,
'Alchemy::Page not found "/"',
'Alchemy::Page not found "/"',
)
end
end
Expand Down Expand Up @@ -153,12 +153,12 @@ module Alchemy
end

it "loads the root page of that language" do
get :index, params: {locale: "en"}
get :index, params: { locale: "en" }
expect(assigns(:page)).to eq(start_page)
end

it "sets @root_page to root page of that language" do
get :index, params: {locale: "en"}
get :index, params: { locale: "en" }
expect(assigns(:root_page)).to eq(start_page)
end
end
Expand All @@ -173,7 +173,7 @@ module Alchemy

it "renders 404" do
expect {
get :show, params: {urlname: not_yet_public.urlname}
get :show, params: { urlname: not_yet_public.urlname }
}.to raise_error(ActionController::RoutingError)
end
end
Expand All @@ -188,7 +188,7 @@ module Alchemy

it "renders 404" do
expect {
get :show, params: {urlname: no_longer_public.urlname}
get :show, params: { urlname: no_longer_public.urlname }
}.to raise_error(ActionController::RoutingError)
end
end
Expand All @@ -202,7 +202,7 @@ module Alchemy
end

it "renders page" do
get :show, params: {urlname: still_public_page.urlname}
get :show, params: { urlname: still_public_page.urlname }
expect(response).to be_successful
end
end
Expand All @@ -216,7 +216,7 @@ module Alchemy
end

it "renders page" do
get :show, params: {urlname: still_public_page.urlname}
get :show, params: { urlname: still_public_page.urlname }
expect(response).to be_successful
end
end
Expand All @@ -225,28 +225,28 @@ module Alchemy
render_views

it "should render a rss feed" do
get :show, params: {urlname: page.urlname, format: :rss}
get :show, params: { urlname: page.urlname, format: :rss }
expect(response.media_type).to eq("application/rss+xml")
end

it "should include content" do
page.elements.first.content_by_name("news_headline").essence.update_columns(body: "Peters Petshop")
get :show, params: {urlname: "news", format: :rss}
get :show, params: { urlname: "news", format: :rss }
expect(response.body).to match /Peters Petshop/
end
end

context "requested for a page that does not contain a feed" do
it "should render xml 404 error" do
get :show, params: {urlname: default_language_root.urlname, format: :rss}
get :show, params: { urlname: default_language_root.urlname, format: :rss }
expect(response.status).to eq(404)
end
end

describe "Layout rendering" do
context "with ajax request" do
it "should not render a layout" do
get :show, params: {urlname: page.urlname}, xhr: true
get :show, params: { urlname: page.urlname }, xhr: true
expect(response).to render_template(:show)
expect(response).not_to render_template(layout: "application")
end
Expand All @@ -256,19 +256,18 @@ module Alchemy
describe "url nesting" do
render_views

let(:catalog) { create(:alchemy_page, :public, name: "Catalog", urlname: "catalog", parent: default_language_root, language: default_language, visible: true) }
let(:catalog) { create(:alchemy_page, :public, name: "Catalog", urlname: "catalog", parent: default_language_root, language: default_language, visible: true) }
let(:products) { create(:alchemy_page, :public, name: "Products", urlname: "products", parent: catalog, language: default_language, visible: true) }
let(:product) { create(:alchemy_page, :public, name: "Screwdriver", urlname: "screwdriver", parent: products, language: default_language, autogenerate_elements: true, visible: true) }
let(:product) { create(:alchemy_page, :public, name: "Screwdriver", urlname: "screwdriver", parent: products, language: default_language, autogenerate_elements: true, visible: true) }

before do
allow(Alchemy.user_class).to receive(:admins).and_return(OpenStruct.new(count: 1))
stub_alchemy_config(:url_nesting, true)
product.elements.find_by_name("article").contents.essence_texts.first.essence.update_column(:body, "screwdriver")
end

context "with correct levelnames in params" do
it "should show the requested page" do
get :show, params: {urlname: "catalog/products/screwdriver"}
get :show, params: { urlname: "catalog/products/screwdriver" }
expect(response.status).to eq(200)
expect(response.body).to have_content("screwdriver")
end
Expand All @@ -277,7 +276,7 @@ module Alchemy
context "with incorrect levelnames in params" do
it "should render a 404 page" do
expect {
get :show, params: {urlname: "catalog/faqs/screwdriver"}
get :show, params: { urlname: "catalog/faqs/screwdriver" }
}.to raise_error(ActionController::RoutingError)
end
end
Expand All @@ -286,7 +285,7 @@ module Alchemy
context "when a non-existent page is requested" do
it "should rescue a RoutingError with rendering a 404 page." do
expect {
get :show, params: {urlname: "doesntexist"}
get :show, params: { urlname: "doesntexist" }
}.to raise_error(ActionController::RoutingError)
end
end
Expand All @@ -299,12 +298,12 @@ module Alchemy

context "with no lang parameter present" do
it "should store defaults language id in the session." do
get :show, params: {urlname: page.urlname}
get :show, params: { urlname: page.urlname }
expect(controller.session[:alchemy_language_id]).to eq(Language.default.id)
end

it "should store default language as class var." do
get :show, params: {urlname: page.urlname}
get :show, params: { urlname: page.urlname }
expect(Language.current).to eq(Language.default)
end
end
Expand All @@ -326,7 +325,7 @@ module Alchemy
end

it "renders the page related to its language" do
get :show, params: {urlname: "same-name", locale: klingon_page.language_code}
get :show, params: { urlname: "same-name", locale: klingon_page.language_code }
expect(response.body).to have_content("klingon page")
end
end
Expand Down
13 changes: 6 additions & 7 deletions spec/helpers/alchemy/pages_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@

module Alchemy
describe PagesHelper do
let(:language_root) { create(:alchemy_page, :language_root) }
let(:public_page) { create(:alchemy_page, :public) }
let(:klingon) { create(:alchemy_language, :klingon) }
let(:klingon_language_root) { create(:alchemy_page, :language_root, language: klingon) }
let(:language_root) { create(:alchemy_page, :language_root) }
let(:public_page) { create(:alchemy_page, :public) }
let(:klingon) { create(:alchemy_language, :klingon) }
let(:klingon_language_root) { create(:alchemy_page, :language_root, language: klingon) }

before do
helper.controller.class_eval { include Alchemy::ConfigurationMethods }
allow(Config).to receive(:get) { |arg| arg == :url_nesting ? true : Config.parameter(arg) }
@root_page = language_root # We need this instance variable in the helpers
end

Expand Down Expand Up @@ -92,8 +91,8 @@ module Alchemy

describe "#render_breadcrumb" do
let(:parent) { create(:alchemy_page, :public, visible: true) }
let(:page) { create(:alchemy_page, :public, parent_id: parent.id, visible: true) }
let(:user) { nil }
let(:page) { create(:alchemy_page, :public, parent_id: parent.id, visible: true) }
let(:user) { nil }

before do
allow(helper).to receive(:multi_language?).and_return(false)
Expand Down
16 changes: 11 additions & 5 deletions spec/libraries/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,34 @@ module Alchemy
context "without a new default" do
before do
expect(described_class).to receive(:deprecated_configs).at_least(:once) do
{ url_nesting: nil }
{ foo: nil }
end
end

it "warns" do
expect(Alchemy::Deprecation).to receive(:warn)
Config.get(:url_nesting)
Config.get(:foo)
end
end

context "with a new default" do
before do
expect(described_class).to receive(:deprecated_configs).at_least(:once) do
{ foo: true }
end
end

context "and current value is not the default" do
before do
expect(described_class).to receive(:show).at_least(:once) do
{ "url_nesting" => false }
{ "foo" => false }
end
end

it "warns about new default" do
expect(Alchemy::Deprecation).to \
receive(:warn).with("Setting url_nesting configuration to false is deprecated and will be always true in Alchemy 5.0")
Config.get(:url_nesting)
receive(:warn).with("Setting foo configuration to false is deprecated and will be always true in Alchemy 5.0")
Config.get(:foo)
end
end
end
Expand Down
Loading

0 comments on commit 21a1028

Please sign in to comment.