Introduce page.url_path and use it for alchemyPageSelect (#1859)

* 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

* 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.

* Remove @url_prefix

This variable is not used anymore. since we generate the url in page links via the API response.

* 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.

* 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.

* Appease Rubocop

* Use page.url_path in page link dialog

And with that fix bugs for wrongly generated urls in multi language environments.

app/assets/javascripts/alchemy/alchemy.link_dialog.js.coffee

@@ -76,26 +76,26 @@ 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) =>
             page = data.pages[0]
             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

app/assets/javascripts/alchemy/templates/page.hbs

@@ -4,6 +4,6 @@
     {{ page.name }}
   </span>
   <span class="page-select--page-urlname">
-    /{{ page.urlname }}
+    {{ page.url_path }}
   </span>
 </div>

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

app/controllers/alchemy/api/pages_controller.rb

@@ -105,6 +105,7 @@ def page_includes
       [
         :tags,
         {
+          language: :site,
           elements: [
             {
               nested_elements: [

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
 
@@ -57,7 +58,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

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

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

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

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)
     }
   })
 </script>

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

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