Introduce page.url_path and use it for alchemyPageSelect (AlchemyCMS#…

…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

@@ -140,7 +140,6 @@ def link
         @attachments = Attachment.all.collect { |f|
           [f.name, download_attachment_path(id: f.id, name: f.urlname)]
         }
-        @url_prefix = prefix_locale? ? "#{@current_language.code}/" : ""
       end
 
       def fold

app/controllers/alchemy/api/pages_controller.rb

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

app/models/alchemy/node.rb

@@ -67,7 +67,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

@@ -302,6 +302,13 @@ def find_elements(options = {})
       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,64 @@
+# 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
+        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

@@ -33,7 +33,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) {
@@ -42,7 +42,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

@@ -11,7 +11,7 @@ module Alchemy
         let(:result) { JSON.parse(response.body) }
 
         it "returns JSON" do
-          get :index, params: {format: :json}
+          get :index, params: { format: :json }
           expect(result["pages"]).to eq([])
         end
       end
@@ -21,15 +21,15 @@ module Alchemy
         let(:result) { JSON.parse(response.body) }
 
         it "returns JSON" do
-          get :index, params: {format: :json}
+          get :index, params: { format: :json }
 
           expect(response.status).to eq(200)
           expect(response.media_type).to eq("application/json")
           expect(result).to have_key("pages")
         end
 
         it "returns all public pages" do
-          get :index, params: {format: :json}
+          get :index, params: { format: :json }
 
           expect(result["pages"].size).to eq(2)
         end
@@ -40,7 +40,7 @@ module Alchemy
           end
 
           it "returns all pages" do
-            get :index, params: {format: :json}
+            get :index, params: { format: :json }
 
             expect(result["pages"].size).to eq(Alchemy::Page.count)
           end
@@ -86,7 +86,7 @@ module Alchemy
         let!(:page) { create(:alchemy_page, :public, page_layout: "contact") }
 
         it "returns all pages as nested json tree without admin related infos", :aggregate_failures do
-          get :nested, params: {format: :json}
+          get :nested, params: { format: :json }
 
           expect(response.status).to eq(200)
           expect(response.media_type).to eq("application/json")
@@ -114,7 +114,7 @@ module Alchemy
           end
 
           it "returns all pages as nested json tree with admin related infos", :aggregate_failures do
-            get :nested, params: {format: :json}
+            get :nested, params: { format: :json }
 
             expect(response.status).to eq(200)
             expect(response.media_type).to eq("application/json")
@@ -139,7 +139,7 @@ module Alchemy
 
         context "when a page_id is passed" do
           it "returns all pages as nested json from this page only" do
-            get :nested, params: {page_id: page.id, format: :json}
+            get :nested, params: { page_id: page.id, format: :json }
 
             expect(response.status).to eq(200)
             expect(response.media_type).to eq("application/json")
@@ -153,7 +153,7 @@ module Alchemy
 
         context "when `elements=true` is passed" do
           it "returns all pages as nested json tree with elements included" do
-            get :nested, params: {elements: "true", format: :json}
+            get :nested, params: { elements: "true", format: :json }
 
             expect(response.status).to eq(200)
             expect(response.media_type).to eq("application/json")
@@ -170,7 +170,7 @@ module Alchemy
             end
 
             it "returns all pages as nested json tree with only these elements included" do
-              get :nested, params: {elements: "headline,text", format: :json}
+              get :nested, params: { elements: "headline,text", format: :json }
 
               result = JSON.parse(response.body)
 
@@ -189,21 +189,22 @@ module Alchemy
         let(:page) { create(:alchemy_page, :public, urlname: "a-page") }
 
         it "returns page as json" do
-          get :show, params: {urlname: page.urlname, format: :json}
+          get :show, params: { urlname: page.urlname, format: :json }
 
           expect(response.status).to eq(200)
           expect(response.media_type).to eq("application/json")
 
           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
           let(:page) { create(:alchemy_page, restricted: true, urlname: "a-page") }
 
           it "responds with 403" do
-            get :show, params: {urlname: page.urlname, format: :json}
+            get :show, params: { urlname: page.urlname, format: :json }
 
             expect(response.status).to eq(403)
             expect(response.media_type).to eq("application/json")
@@ -219,7 +220,7 @@ module Alchemy
           let(:page) { create(:alchemy_page, urlname: "a-page") }
 
           it "responds with 403" do
-            get :show, params: {urlname: page.urlname, format: :json}
+            get :show, params: { urlname: page.urlname, format: :json }
 
             expect(response.status).to eq(403)
             expect(response.media_type).to eq("application/json")
@@ -234,7 +235,7 @@ module Alchemy
 
       context "requesting an unknown page" do
         it "responds with 404" do
-          get :show, params: {urlname: "not-existing", format: :json}
+          get :show, params: { urlname: "not-existing", format: :json }
 
           expect(response.status).to eq(404)
           expect(response.media_type).to eq("application/json")
@@ -249,7 +250,7 @@ module Alchemy
           let(:page) { create(:alchemy_page, :public) }
 
           it "responds with 404" do
-            get :show, params: {urlname: page.urlname, locale: "na", format: :json}
+            get :show, params: { urlname: page.urlname, locale: "na", format: :json }
             expect(response.status).to eq(404)
           end
         end
@@ -259,7 +260,7 @@ module Alchemy
         let(:page) { create(:alchemy_page, :public) }
 
         it "responds with json" do
-          get :show, params: {urlname: page.id, format: :json}
+          get :show, params: { urlname: page.id, format: :json }
 
           expect(response.status).to eq(200)
           expect(response.media_type).to eq("application/json")
@@ -280,15 +281,15 @@ module Alchemy
 
           context "when a locale is given" do
             it "renders the page related to its language" do
-              get :show, params: {urlname: "same-name", locale: klingon_page.language_code, format: :json}
+              get :show, params: { urlname: "same-name", locale: klingon_page.language_code, format: :json }
               result = JSON.parse(response.body)
               expect(result["id"]).to eq(klingon_page.id)
             end
           end
 
           context "when no locale is given" do
             it "renders the page of the default language" do
-              get :show, params: {urlname: "same-name", format: :json}
+              get :show, params: { urlname: "same-name", format: :json }
               result = JSON.parse(response.body)
               expect(result["id"]).to eq(english_page.id)
             end