From 2bf782fc34ac98a6d74f68f1c46784c65dddf394 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Fri, 29 May 2020 15:48:05 +0200 Subject: [PATCH 1/7] Introduce page url_path service class Use this class to generate the url_path to a page. It takes several circumstances into account 1. It returns just a slash for language root pages of the default langauge 2. It returns a url path with a leading slash for regular pages 3. It returns a url path with a leading slash and language code prefix for pages not having the default language 4. It returns a url path with a leading slash and the language code for language root pages of a non-default language --- app/models/alchemy/page.rb | 7 +++ app/models/alchemy/page/url_path.rb | 66 ++++++++++++++++++++ spec/models/alchemy/page/url_path_spec.rb | 76 +++++++++++++++++++++++ 3 files changed, 149 insertions(+) create mode 100644 app/models/alchemy/page/url_path.rb create mode 100644 spec/models/alchemy/page/url_path_spec.rb diff --git a/app/models/alchemy/page.rb b/app/models/alchemy/page.rb index ac41578fb3..2b1c654619 100644 --- a/app/models/alchemy/page.rb +++ b/app/models/alchemy/page.rb @@ -351,6 +351,13 @@ def find_elements(options = {}, show_non_public = false) finder.elements(page: self) end + # = The url_path for this page + # + # @see Alchemy::Page::UrlPath#call + def url_path + Alchemy::Page::UrlPath.new(self).call + end + # The page's view partial is dependent from its page layout # # == Define page layouts diff --git a/app/models/alchemy/page/url_path.rb b/app/models/alchemy/page/url_path.rb new file mode 100644 index 0000000000..3a7119b083 --- /dev/null +++ b/app/models/alchemy/page/url_path.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Alchemy + class Page + # = The url_path for this page + # + # Use this to build relative links to this page + # + # It takes several circumstances into account: + # + # 1. It returns just a slash for language root pages of the default langauge + # 2. It returns a url path with a leading slash for regular pages + # 3. It returns a url path with a leading slash and language code prefix for pages not having the default language + # 4. It returns a url path with a leading slash and the language code for language root pages of a non-default language + # + # == Examples + # + # Using Rails' link_to helper + # + # link_to page.url + # + class UrlPath + ROOT_PATH = "/" + + def initialize(page) + @page = page + @language = @page.language + @site = @language.site + end + + def call + return @page.urlname if @page.definition["redirects_to_external"] + + if @page.language_root? + language_root_path + elsif @site.languages.select(&:public?).length > 1 + page_path_with_language_prefix + else + page_path_with_leading_slash + end + end + + private + + def language_root_path + @language.default? ? ROOT_PATH : language_path + end + + def page_path_with_language_prefix + @language.default? ? page_path : language_path + page_path + end + + def page_path_with_leading_slash + @page.language_root? ? ROOT_PATH : page_path + end + + def language_path + "/#{@page.language_code}" + end + + def page_path + "/#{@page.urlname}" + end + end + end +end diff --git a/spec/models/alchemy/page/url_path_spec.rb b/spec/models/alchemy/page/url_path_spec.rb new file mode 100644 index 0000000000..58a124ef0e --- /dev/null +++ b/spec/models/alchemy/page/url_path_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Alchemy::Page::UrlPath do + subject(:url) { described_class.new(page).call } + + let(:site) { create(:alchemy_site) } + + context "if the page redirects to external" do + let(:page) do + create(:alchemy_page, page_layout: "external", site: site, urlname: "https://example.com") + end + + it { is_expected.to eq("https://example.com") } + end + + context "on a single language site" do + context "for the language root page" do + let(:page) { create(:alchemy_page, :language_root, site: site) } + + it { is_expected.to eq("/") } + end + + context "for a regular page" do + let(:page) { create(:alchemy_page, site: site) } + + it { is_expected.to eq("/#{page.urlname}") } + end + end + + context "on a multi language site" do + let!(:default_language) { site.default_language } + let!(:language) { create(:alchemy_language, :klingon, site: site) } + + context "for the language root page" do + context "and page having the default language" do + let(:page) do + create(:alchemy_page, :language_root, site: site, language: default_language) + end + + it { is_expected.to eq("/") } + end + + context "and page having a non-default language" do + let(:page) do + create(:alchemy_page, :language_root, site: site, language: language) + end + + it do + is_expected.to eq("/#{page.language_code}") + end + end + end + + context "for a regular page" do + context "and page having the default language" do + let(:page) do + create(:alchemy_page, site: site, language: default_language) + end + + it { is_expected.to eq("/#{page.urlname}") } + end + + context "and page having a non-default language" do + let(:page) do + create(:alchemy_page, site: site, language: language) + end + + it do + is_expected.to eq("/#{page.language_code}/#{page.urlname}") + end + end + end + end +end From 4818165974747ad3409fbd3f55c7e5c92f649a59 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Thu, 28 May 2020 17:01:57 +0200 Subject: [PATCH 2/7] Return the page url_path with API responses We want to use the newly introduced url_path attribute on the page selector in the admin. It in general makes sense to have the correct url to a page from the API anyway. --- app/controllers/alchemy/api/pages_controller.rb | 1 + app/serializers/alchemy/page_serializer.rb | 3 ++- spec/controllers/alchemy/api/pages_controller_spec.rb | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/controllers/alchemy/api/pages_controller.rb b/app/controllers/alchemy/api/pages_controller.rb index 27dc8df7e1..c3c024ffe0 100644 --- a/app/controllers/alchemy/api/pages_controller.rb +++ b/app/controllers/alchemy/api/pages_controller.rb @@ -105,6 +105,7 @@ def page_includes [ :tags, { + language: :site, elements: [ { nested_elements: [ diff --git a/app/serializers/alchemy/page_serializer.rb b/app/serializers/alchemy/page_serializer.rb index 776cda862e..1fb53f3b41 100644 --- a/app/serializers/alchemy/page_serializer.rb +++ b/app/serializers/alchemy/page_serializer.rb @@ -13,7 +13,8 @@ class PageSerializer < ActiveModel::Serializer :tag_list, :created_at, :updated_at, - :status + :status, + :url_path has_many :elements end diff --git a/spec/controllers/alchemy/api/pages_controller_spec.rb b/spec/controllers/alchemy/api/pages_controller_spec.rb index 0675fb3487..2fb424bc69 100644 --- a/spec/controllers/alchemy/api/pages_controller_spec.rb +++ b/spec/controllers/alchemy/api/pages_controller_spec.rb @@ -204,6 +204,7 @@ module Alchemy result = JSON.parse(response.body) expect(result['id']).to eq(page.id) + expect(result["url_path"]).to eq("/a-page") end context 'requesting an restricted page' do From d46ac228c34958f041e493100128d53f2a9b451f Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Fri, 29 May 2020 15:54:34 +0200 Subject: [PATCH 3/7] Use Page#url_path in alchemyPageSelect We do not need to care about adding leading slashes in the template anymore and it fixes a lot of missing features for multi language pages. --- app/assets/javascripts/alchemy/templates/page.hbs | 2 +- app/views/alchemy/admin/nodes/_form.html.erb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/alchemy/templates/page.hbs b/app/assets/javascripts/alchemy/templates/page.hbs index 1686cacbe1..81748efe20 100644 --- a/app/assets/javascripts/alchemy/templates/page.hbs +++ b/app/assets/javascripts/alchemy/templates/page.hbs @@ -4,6 +4,6 @@ {{ page.name }} - /{{ page.urlname }} + {{ page.url_path }} diff --git a/app/views/alchemy/admin/nodes/_form.html.erb b/app/views/alchemy/admin/nodes/_form.html.erb index dca2a49c03..f582439471 100644 --- a/app/views/alchemy/admin/nodes/_form.html.erb +++ b/app/views/alchemy/admin/nodes/_form.html.erb @@ -30,7 +30,7 @@ initialSelection: { id: <%= node.page_id %>, text: "<%= node.page.name %>", - url: "/<%= node.page.urlname %>" + url_path: "<%= node.page.url_path %>" } <% end %> }).on('change', function(e) { @@ -39,7 +39,7 @@ $('#node_url').val('').prop('disabled', false) } else { $('#node_name').attr('placeholder', e.added.name) - $('#node_url').val('/' + e.added.urlname).prop('disabled', true) + $('#node_url').val(e.added.url_path).prop('disabled', true) } }) From d28f5645043b6ecb9b6025c962b8839419f6acc8 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Fri, 29 May 2020 15:55:26 +0200 Subject: [PATCH 4/7] Use page.url_path in node.url Instead of having (buggy) page url generating code in the node model, we use the newly introduced page.url_path attribute that takes all necessities into account. --- app/models/alchemy/node.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/alchemy/node.rb b/app/models/alchemy/node.rb index 0368e271bd..293e00495d 100644 --- a/app/models/alchemy/node.rb +++ b/app/models/alchemy/node.rb @@ -57,7 +57,7 @@ def definitions_file_path # Either the value is stored in the database, aka. an external url. # Or, if attached, the values comes from a page. def url - page && "/#{page.urlname}" || read_attribute(:url).presence + page&.url_path || read_attribute(:url).presence end def to_partial_path From 3801a8e6cb4ae59e9cf00bd8b4c59f10c7e3f5cf Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Fri, 29 May 2020 15:57:29 +0200 Subject: [PATCH 5/7] Use page.url_path in page link dialog And with that fix bugs for wrongly generated urls in multi language environments. --- .../javascripts/alchemy/alchemy.link_dialog.js.coffee | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/alchemy/alchemy.link_dialog.js.coffee b/app/assets/javascripts/alchemy/alchemy.link_dialog.js.coffee index 46bf1bad57..cbc7e463b3 100644 --- a/app/assets/javascripts/alchemy/alchemy.link_dialog.js.coffee +++ b/app/assets/javascripts/alchemy/alchemy.link_dialog.js.coffee @@ -76,16 +76,16 @@ class window.Alchemy.LinkDialog extends Alchemy.Dialog meta = data.meta results: data.pages.map (page) -> - id: "/#{page.urlname}" + id: page.url_path name: page.name - urlname: page.urlname + url_path: page.url_path page_id: page.id more: meta.page * meta.per_page < meta.total_count initSelection: ($element, callback) => urlname = $element.val() $.get Alchemy.routes.api_pages_path, q: - urlname_eq: urlname.replace(/^\//, '') + urlname_eq: urlname.replace(/^\/([a-z]{2}(-[A-Z]{2})?\/)?/, '') page: 1 per_page: 1, (data) => @@ -93,9 +93,9 @@ class window.Alchemy.LinkDialog extends Alchemy.Dialog if page @initElementSelect(page.id) callback - id: "/#{page.urlname}" + id: page.url_path name: page.name - urlname: page.name + url_path: page.url_path page_id: page.id formatSelection: (page) -> page.name From ce24df0cafde13dadaac4349a325d621dfbedaa2 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Thu, 28 May 2020 19:39:03 +0200 Subject: [PATCH 6/7] Remove @url_prefix This variable is not used anymore. since we generate the url in page links via the API response. --- app/controllers/alchemy/admin/pages_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/alchemy/admin/pages_controller.rb b/app/controllers/alchemy/admin/pages_controller.rb index 97475c4dae..6a206d6064 100644 --- a/app/controllers/alchemy/admin/pages_controller.rb +++ b/app/controllers/alchemy/admin/pages_controller.rb @@ -142,7 +142,6 @@ def link @attachments = Attachment.all.collect { |f| [f.name, download_attachment_path(id: f.id, name: f.urlname)] } - @url_prefix = prefix_locale? ? "#{Language.current.code}/" : "" end def fold From 50b60f10e1aa0e3ce0a6d23abf30e9d25dc0fe32 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Fri, 29 May 2020 15:56:20 +0200 Subject: [PATCH 7/7] Appease Rubocop --- app/models/alchemy/node.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/models/alchemy/node.rb b/app/models/alchemy/node.rb index 293e00495d..c3804a57a3 100644 --- a/app/models/alchemy/node.rb +++ b/app/models/alchemy/node.rb @@ -4,12 +4,12 @@ module Alchemy class Node < BaseRecord VALID_URL_REGEX = /\A(\/|\D[a-z\+\d\.\-]+:)/ - acts_as_nested_set scope: 'language_id', touch: true + acts_as_nested_set scope: "language_id", touch: true stampable stamper_class_name: Alchemy.user_class_name - belongs_to :site, class_name: 'Alchemy::Site' - belongs_to :language, class_name: 'Alchemy::Language' - belongs_to :page, class_name: 'Alchemy::Page', optional: true, inverse_of: :nodes + belongs_to :site, class_name: "Alchemy::Site" + belongs_to :language, class_name: "Alchemy::Language" + belongs_to :page, class_name: "Alchemy::Page", optional: true, inverse_of: :nodes validates :name, presence: true, if: -> { page.nil? } validates :url, format: { with: VALID_URL_REGEX }, unless: -> { url.nil? } @@ -25,7 +25,8 @@ def name class << self # Returns all root nodes for current language def language_root_nodes - raise 'No language found' if Language.current.nil? + raise "No language found" if Language.current.nil? + roots.where(language_id: Language.current.id) end @@ -48,7 +49,7 @@ def read_definitions_file # Returns the +menus.yml+ file path # def definitions_file_path - Rails.root.join 'config/alchemy/menus.yml' + Rails.root.join "config/alchemy/menus.yml" end end