Merge pull request #3554 from AlchemyCMS/move-page-meta-data-to-page-version

feat: Move page metadata to PageVersion

Author: tvdeyen

app/models/alchemy/page.rb

@@ -7,12 +7,12 @@
 #  id               :integer          not null, primary key
 #  name             :string
 #  urlname          :string
-#  title            :string
+#  title            :string           (deprecated - use draft_version.title)
 #  language_code    :string
 #  language_root    :boolean
 #  page_layout      :string
-#  meta_keywords    :text
-#  meta_description :text
+#  meta_keywords    :text             (deprecated - use draft_version.meta_keywords)
+#  meta_description :text             (deprecated - use draft_version.meta_description)
 #  lft              :integer
 #  rgt              :integer
 #  parent_id        :integer
@@ -66,11 +66,12 @@ class Page < BaseRecord
       depth
       urlname
       cached_tag_list
+      title
+      meta_description
+      meta_keywords
     ]
 
     PERMITTED_ATTRIBUTES = [
-      :meta_description,
-      :meta_keywords,
       :name,
       :page_layout,
       :public_on,
@@ -81,10 +82,12 @@ class Page < BaseRecord
       :searchable,
       :sitemap,
       :tag_list,
-      :title,
       :urlname,
       :layoutpage,
-      :menu_id
+      :menu_id,
+      {
+        draft_version_attributes: [:id] + PageVersion::METADATA_ATTRIBUTES.map(&:to_sym)
+      }
     ]
 
     acts_as_nested_set(dependent: :destroy, scope: [:layoutpage, :language_id])
@@ -120,6 +123,8 @@ class Page < BaseRecord
     has_one :draft_version, -> { drafts }, class_name: "Alchemy::PageVersion"
     has_one :public_version, -> { published }, class_name: "Alchemy::PageVersion", autosave: -> { persisted? }
 
+    accepts_nested_attributes_for :draft_version
+
     has_many :page_ingredients, class_name: "Alchemy::Ingredients::Page", foreign_key: :related_object_id, dependent: :nullify
 
     before_validation :set_language,
@@ -129,8 +134,7 @@ class Page < BaseRecord
     validates_format_of :page_layout, with: /\A[a-z0-9_-]+\z/, unless: -> { page_layout.blank? }
     validates_presence_of :parent, unless: -> { layoutpage? || language_root? }
 
-    before_create -> { versions.build },
-      if: -> { versions.none? }
+    after_initialize :ensure_draft_version
 
     before_save :set_language_code,
       if: -> { language.present? }
@@ -179,7 +183,7 @@ def url_path_class=(klass)
       end
 
       def searchable_alchemy_resource_attributes
-        %w[name urlname title]
+        %w[name urlname]
       end
 
       # @return the language root page for given language id.
@@ -213,8 +217,7 @@ def copy_and_paste(source, new_parent, new_name)
           .call(changed_attributes: {
             parent: new_parent,
             language: new_parent&.language,
-            name: new_name,
-            title: new_name
+            name: new_name
           })
         if source.children.any?
           source.copy_children_to(page)
@@ -458,6 +461,54 @@ def public_until
       attribute_fixed?(:public_until) ? fixed_attributes[:public_until] : public_version&.public_until
     end
 
+    # Returns the title from the public version, falling back to draft version
+    #
+    # If it's a fixed attribute then the fixed value is returned instead
+    #
+    def title
+      return fixed_attributes[:title] if attribute_fixed?(:title)
+
+      public_version&.title || draft_version&.title
+    end
+
+    # Returns the meta_description from the public version, falling back to draft version
+    #
+    # If it's a fixed attribute then the fixed value is returned instead
+    #
+    def meta_description
+      return fixed_attributes[:meta_description] if attribute_fixed?(:meta_description)
+
+      public_version&.meta_description || draft_version&.meta_description
+    end
+
+    # Returns the meta_keywords from the public version, falling back to draft version
+    #
+    # If it's a fixed attribute then the fixed value is returned instead
+    #
+    def meta_keywords
+      return fixed_attributes[:meta_keywords] if attribute_fixed?(:meta_keywords)
+
+      public_version&.meta_keywords || draft_version&.meta_keywords
+    end
+
+    # @deprecated Use draft_version.title= instead
+    def title=(value)
+      draft_version&.title = value
+    end
+    deprecate "title=": :"page.draft_version.title=", deprecator: Alchemy::Deprecation
+
+    # @deprecated Use draft_version.meta_description= instead
+    def meta_description=(value)
+      draft_version&.meta_description = value
+    end
+    deprecate "meta_description=": :"page.draft_version.meta_description=", deprecator: Alchemy::Deprecation
+
+    # @deprecated Use draft_version.meta_keywords= instead
+    def meta_keywords=(value)
+      draft_version&.meta_keywords = value
+    end
+    deprecate "meta_keywords=": :"page.draft_version.meta_keywords=", deprecator: Alchemy::Deprecation
+
     # Returns the name of the creator of this page.
     #
     # If no creator could be found or associated user model
@@ -499,9 +550,18 @@ def menus
 
     private
 
+    def ensure_draft_version
+      self.draft_version ||= versions.build
+    end
+
     def set_fixed_attributes
       fixed_attributes.all.each do |attribute, value|
-        send(:"#{attribute}=", value)
+        attribute_name = attribute.to_s
+        if PageVersion::METADATA_ATTRIBUTES.include?(attribute_name)
+          draft_version&.send(:"#{attribute}=", value)
+        else
+          send(:"#{attribute}=", value)
+        end
       end
     end
 

app/models/alchemy/page/page_naming.rb

@@ -19,9 +19,6 @@ module PageNaming
           uniqueness: {scope: [:language_id, :layoutpage], if: -> { urlname.present? }, case_sensitive: false},
           exclusion: {in: RESERVED_URLNAMES}
 
-        before_save :set_title,
-          if: -> { title.blank? }
-
         after_update :update_descendants_urlnames,
           if: :saved_change_to_urlname?
 
@@ -69,10 +66,6 @@ def set_urlname
         self[:urlname] = nested_url_name
       end
 
-      def set_title
-        self[:title] = name
-      end
-
       # Returns the full nested urlname.
       #
       def nested_url_name

app/models/alchemy/page/publisher.rb

@@ -24,6 +24,8 @@ def publish!(public_on:)
             version = public_version(public_on)
             DeleteElements.new(version.elements).call
 
+            copy_metadata(public_version: version)
+
             repository = page.draft_version.element_repository
             ActiveRecord::Base.no_touching do
               Element.acts_as_list_no_update do
@@ -51,6 +53,16 @@ def publish!(public_on:)
       def public_version(public_on)
         page.public_version || page.versions.create!(public_on: public_on)
       end
+
+      # Copy metadata from draft_version to public_version.
+      def copy_metadata(public_version:)
+        draft = page.draft_version
+        return unless draft
+
+        PageVersion::METADATA_ATTRIBUTES.each do |attr|
+          public_version.send(:"#{attr}=", draft.send(attr))
+        end
+      end
     end
   end
 end

app/models/alchemy/page_version.rb

@@ -2,6 +2,13 @@
 
 module Alchemy
   class PageVersion < BaseRecord
+    # Metadata attributes that are versioned (moved from Page)
+    METADATA_ATTRIBUTES = %w[
+      title
+      meta_description
+      meta_keywords
+    ].freeze
+
     belongs_to :page, class_name: "Alchemy::Page", inverse_of: :versions, touch: true
 
     has_many :elements, -> { order(:position) },
@@ -11,6 +18,8 @@ class PageVersion < BaseRecord
     scope :drafts, -> { where(public_on: nil).order(updated_at: :desc) }
     scope :published, -> { where.not(public_on: nil).order(public_on: :desc) }
 
+    before_create :set_title_from_page
+
     def self.public_on(time = Time.current)
       where("#{table_name}.public_on <= :time AND " \
             "(#{table_name}.public_until IS NULL " \
@@ -54,5 +63,11 @@ def element_repository
     def delete_elements
       DeleteElements.new(elements).call
     end
+
+    def set_title_from_page
+      return if title.present?
+
+      self.title = page&.name
+    end
   end
 end

app/services/alchemy/copy_page.rb

@@ -28,8 +28,14 @@ class CopyPage
       depth
       urlname
       cached_tag_list
+      title
+      meta_description
+      meta_keywords
     ]
 
+    # Metadata to copy via nested attributes (title is derived from page.name)
+    METADATA_ATTRIBUTES_TO_COPY = (Alchemy::PageVersion::METADATA_ATTRIBUTES - %w[title]).freeze
+
     attr_reader :page
 
     # @param page [Alchemy::Page]
@@ -70,6 +76,7 @@ def attributes_from_source_for_copy(differences = {})
         .merge(DEFAULT_ATTRIBUTES_FOR_COPY)
         .merge(differences)
       desired_attributes["name"] = best_name_for_copy(source_attributes, desired_attributes)
+      desired_attributes["draft_version_attributes"] = draft_version_attributes_for_copy
       desired_attributes.except(*SKIPPED_ATTRIBUTES_ON_COPY)
     end
 
@@ -94,5 +101,15 @@ def best_name_for_copy(source_attributes, desired_attributes)
         desired_name
       end
     end
+
+    # Builds nested attributes for draft_version metadata (except title).
+    # Title is handled by PageVersion#set_title_from_page callback based on page.name.
+    def draft_version_attributes_for_copy
+      return {} unless page.draft_version
+
+      METADATA_ATTRIBUTES_TO_COPY.each_with_object({}) do |attr, hash|
+        hash[attr] = page.draft_version.send(attr)
+      end
+    end
   end
 end

app/views/alchemy/admin/pages/_form.html.erb

@@ -19,9 +19,14 @@
 
     <%= f.input :name, autofocus: true %>
     <%= f.input :urlname, as: 'string', input_html: {value: @page.slug}, label: Alchemy::Page.human_attribute_name(:slug) %>
-    <alchemy-char-counter max-chars="60">
-      <%= f.input :title %>
-    </alchemy-char-counter>
+
+    <%= f.fields_for :draft_version, @page.draft_version do |v| %>
+      <alchemy-char-counter max-chars="60">
+        <%= v.input :title, input_html: {
+          disabled: @page.attribute_fixed?(:title)
+        } %>
+      </alchemy-char-counter>
+    <% end %>
 
     <% if Alchemy.config.show_page_searchable_checkbox %>
       <div class="input check_boxes">
@@ -40,13 +45,20 @@
       </div>
     </div>
 
-    <alchemy-char-counter max-chars="160">
-      <%= f.input :meta_description, as: 'text' %>
-    </alchemy-char-counter>
+    <%= f.fields_for :draft_version, @page.draft_version do |v| %>
+      <alchemy-char-counter max-chars="160">
+        <%= v.input :meta_description, as: 'text', input_html: {
+          disabled: @page.attribute_fixed?(:meta_description)
+        } %>
+      </alchemy-char-counter>
 
-    <%= f.input :meta_keywords,
-      as: 'text',
-      hint: Alchemy.t('pages.update.comma_seperated') %>
+      <%= v.input :meta_keywords,
+        as: 'text',
+        hint: Alchemy.t('pages.update.comma_seperated'),
+        input_html: {
+          disabled: @page.attribute_fixed?(:meta_keywords)
+        } %>
+    <% end %>
 
     <%= render Alchemy::Admin::TagsAutocomplete.new do %>
       <%= f.input :tag_list, input_html: { value: f.object.tag_list.join(",") } %>

db/migrate/20260102121232_add_metadata_to_page_versions.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddMetadataToPageVersions < ActiveRecord::Migration[7.1]
+  def change
+    add_column :alchemy_page_versions, :title, :string
+    add_column :alchemy_page_versions, :meta_description, :text
+    add_column :alchemy_page_versions, :meta_keywords, :text
+  end
+end

lib/alchemy/tasks/usage.rb

@@ -18,11 +18,11 @@ def elements_count_by_name
       end
 
       def pages_count_by_type
-        res = Alchemy::Page.all
-          .select("page_layout, COUNT(*) AS count")
+        res = Alchemy::Page
           .group(:page_layout)
-          .order("count DESC, page_layout ASC")
-          .map { |p| {"page_layout" => p.page_layout, "count" => p.count} }
+          .order("count_all DESC, page_layout ASC")
+          .count
+          .map { |layout, count| {"page_layout" => layout, "count" => count} }
         Alchemy::PageDefinition.all.reject { |page_layout| res.map { |p| p["page_layout"] }.include?(page_layout.name) }.sort_by(&:name).each do |page_layout|
           res << {"page_layout" => page_layout.name, "count" => 0}
         end

lib/alchemy/test_support/factories/page_version_factory.rb

@@ -3,6 +3,9 @@
 FactoryBot.define do
   factory :alchemy_page_version, class: "Alchemy::PageVersion" do
     association :page, factory: :alchemy_page
+    title { nil }
+    meta_description { nil }
+    meta_keywords { nil }
 
     trait :published do
       public_on { Time.current }

lib/alchemy/upgrader.rb

@@ -12,11 +12,19 @@ class Upgrader
     Dir["#{File.dirname(__FILE__)}/upgrader/*.rb"].sort.each { require(_1) }
 
     VERSION_MODULE_MAP = {
-      "8.0" => "Alchemy::Upgrader::EightZero"
+      "8.0" => "Alchemy::Upgrader::EightZero",
+      "8.1" => "Alchemy::Upgrader::EightOne"
     }
 
     source_root Alchemy::Engine.root.join("lib/generators/alchemy/install")
 
+    # Returns a memoized upgrader instance for the given version.
+    # This ensures todos are accumulated across rake tasks.
+    def self.[](version)
+      @instances ||= {}
+      @instances[version.to_s] ||= new(version)
+    end
+
     def initialize(version)
       super()
       self.class.include VERSION_MODULE_MAP[version.to_s].constantize