diff --git a/app/controllers/pageflow/entries_controller.rb b/app/controllers/pageflow/entries_controller.rb index c018c1975..c0819aa68 100644 --- a/app/controllers/pageflow/entries_controller.rb +++ b/app/controllers/pageflow/entries_controller.rb @@ -14,7 +14,11 @@ def index def show respond_to do |format| format.html do - entry = find_by_permalink || find_by_slug + entry = find_by_permalink + + return if !entry && redirect_according_to_permalink_redirect + + entry ||= find_by_slug! return if redirect_according_to_entry_redirect(entry) return if redirect_according_to_public_https_mode @@ -62,7 +66,7 @@ def find_by_permalink ) end - def find_by_slug + def find_by_slug! PublishedEntry.find(params[:id], entry_request_scope) end @@ -70,10 +74,26 @@ def entry_request_scope Pageflow.config.public_entry_request_scope.call(Entry, request) end + def redirect_according_to_permalink_redirect + entry = PublishedEntry.find_by_permalink_redirect( + directory: params[:directory], + slug: params[:id], + site: Site.for_request(request) + ) + + return false unless entry + + redirect_to(EntriesHelper::PrettyUrl.build(pageflow, entry), + status: :moved_permanently, + allow_other_host: true) + true + end + def redirect_according_to_entry_redirect(entry) - return unless (redirect_location = entry_redirect(entry)) + return false unless (redirect_location = entry_redirect(entry)) redirect_to(redirect_location, status: :moved_permanently, allow_other_host: true) + true end def entry_redirect(entry) diff --git a/app/helpers/pageflow/entries_helper.rb b/app/helpers/pageflow/entries_helper.rb index 25030def0..24fc9f6d7 100644 --- a/app/helpers/pageflow/entries_helper.rb +++ b/app/helpers/pageflow/entries_helper.rb @@ -15,7 +15,7 @@ def pretty_entry_url(entry, options = {}) module PrettyUrl extend self - def build(routes, entry, options) + def build(routes, entry, options = {}) with_custom_canonical_url_prefix(entry) || default(routes, entry, options) end @@ -24,15 +24,24 @@ def build(routes, entry, options) def with_custom_canonical_url_prefix(entry) return if entry.site.canonical_entry_url_prefix.blank? + entry = ensure_entry_with_revision(entry) [ entry.site.canonical_entry_url_prefix.gsub(':locale', entry.locale), - entry.to_param, + entry_suffix(entry), entry.site.trailing_slash_in_canonical_urls ? '/' : '' ].join end + def entry_suffix(entry) + if entry.permalink.present? + entry_permalink_parts(entry).join + else + entry.to_param + end + end + def default(routes, entry, options) params = options @@ -40,16 +49,19 @@ def default(routes, entry, options) .reverse_merge(Pageflow.config.site_url_options(entry.site) || {}) if entry.permalink.present? - routes.permalink_url( - entry.permalink.directory&.path || '', - entry.permalink.slug, - params - ) + routes.permalink_url(*entry_permalink_parts(entry), params) else routes.short_entry_url(entry.to_model, params) end end + def entry_permalink_parts(entry) + [ + entry.permalink.directory&.path || '', + entry.permalink.slug + ] + end + def ensure_entry_with_revision(entry) if entry.is_a?(EntryAtRevision) entry diff --git a/app/models/concerns/pageflow/permalinkable.rb b/app/models/concerns/pageflow/permalinkable.rb index 4c77ea30d..7a9dea93c 100644 --- a/app/models/concerns/pageflow/permalinkable.rb +++ b/app/models/concerns/pageflow/permalinkable.rb @@ -5,6 +5,7 @@ module Permalinkable included do belongs_to :permalink, optional: true + has_many :permalink_redirects accepts_nested_attributes_for :permalink, update_only: true end diff --git a/app/models/pageflow/permalink.rb b/app/models/pageflow/permalink.rb index 2c817be39..037f94d3e 100644 --- a/app/models/pageflow/permalink.rb +++ b/app/models/pageflow/permalink.rb @@ -16,6 +16,9 @@ class Permalink < ApplicationRecord validate :belongs_to_same_site_as_entry + before_update(:create_redirect, + if: -> { entry.first_published_at.present? && (slug_changed? || directory_changed?) }) + private def set_default_slug @@ -35,5 +38,11 @@ def belongs_to_same_site_as_entry errors.add(:directory, :invalid) end + + def create_redirect + directory.redirects.where(slug:).delete_all + entry.permalink_redirects.create!(slug: slug_was, + directory_id: directory_id_was) + end end end diff --git a/app/models/pageflow/permalink_directory.rb b/app/models/pageflow/permalink_directory.rb index 5ae4d7889..e37756e73 100644 --- a/app/models/pageflow/permalink_directory.rb +++ b/app/models/pageflow/permalink_directory.rb @@ -6,5 +6,9 @@ class PermalinkDirectory < ApplicationRecord validates(:path, format: %r{\A([0-9a-zA-Z-]+/)*\z}, uniqueness: {scope: :site_id}) + + has_many(:redirects, + class_name: 'PermalinkRedirect', + foreign_key: :directory_id) end end diff --git a/app/models/pageflow/permalink_redirect.rb b/app/models/pageflow/permalink_redirect.rb new file mode 100644 index 000000000..91f22a9ef --- /dev/null +++ b/app/models/pageflow/permalink_redirect.rb @@ -0,0 +1,7 @@ +module Pageflow + # @api private + class PermalinkRedirect < ApplicationRecord + belongs_to :entry + belongs_to :directory, class_name: 'PermalinkDirectory' + end +end diff --git a/app/models/pageflow/published_entry.rb b/app/models/pageflow/published_entry.rb index 077f09454..eaafb0c0c 100644 --- a/app/models/pageflow/published_entry.rb +++ b/app/models/pageflow/published_entry.rb @@ -66,7 +66,7 @@ def self.find(id, scope = Entry) PublishedEntry.new(scope.published.find(id)) end - def self.find_by_permalink(directory: nil, slug:, scope:) + def self.find_by_permalink(slug:, scope:, directory: nil) wrap( scope.published.includes(permalink: :directory).where( pageflow_permalink_directories: {path: directory || ''}, @@ -75,6 +75,18 @@ def self.find_by_permalink(directory: nil, slug:, scope:) ) end + def self.find_by_permalink_redirect(site:, slug:, directory: nil) + wrap( + PermalinkRedirect.joins(:entry, :directory).merge(Entry.published).where( + slug:, + pageflow_permalink_directories: { + path: directory || '', + site: + } + ).first&.entry + ) + end + def self.wrap_all(entries) entries = entries.includes(:published_revision) unless entries.loaded? entries.map { |entry| new(entry) } diff --git a/app/views/admin/entries/_permalink_inputs.html.erb b/app/views/admin/entries/_permalink_inputs.html.erb index 70c033b09..8c33130b6 100644 --- a/app/views/admin/entries/_permalink_inputs.html.erb +++ b/app/views/admin/entries/_permalink_inputs.html.erb @@ -1,5 +1,7 @@ <%= form.input(:permalink, as: :pageflow_permalink, - base_url: pretty_site_url(entry.site), + base_url: entry.site.canonical_entry_url_prefix.presence || + pretty_site_url(entry.site), site: entry.site, - slug_placeholder: entry.default_permalink_slug) %> + slug_placeholder: entry.default_permalink_slug, + hint: entry.new_record? ? nil : t('pageflow.admin.entries.permalink_hint'))%> diff --git a/config/locales/new/permalink_redirects.de.yml b/config/locales/new/permalink_redirects.de.yml new file mode 100644 index 000000000..a055dad7f --- /dev/null +++ b/config/locales/new/permalink_redirects.de.yml @@ -0,0 +1,8 @@ +de: + pageflow: + admin: + entries: + permalink_hint: |- + Wenn der Permalink geändert wird, nachdem der Beitrag + bereits veröffentlicht wurde, wird automatisch eine + Weiterleitung von der alten URL zur neuen URL eingerichtet. diff --git a/config/locales/new/permalink_redirects.en.yml b/config/locales/new/permalink_redirects.en.yml new file mode 100644 index 000000000..a3cda2811 --- /dev/null +++ b/config/locales/new/permalink_redirects.en.yml @@ -0,0 +1,8 @@ +en: + pageflow: + admin: + entries: + permalink_hint: |- + Changing the permalink after the story has already been + published automatically creates a redirect from the old URL + to the new URL diff --git a/db/migrate/20240918084059_create_pageflow_permalink_redirects.rb b/db/migrate/20240918084059_create_pageflow_permalink_redirects.rb new file mode 100644 index 000000000..3f8edc0ab --- /dev/null +++ b/db/migrate/20240918084059_create_pageflow_permalink_redirects.rb @@ -0,0 +1,13 @@ +class CreatePageflowPermalinkRedirects < ActiveRecord::Migration[5.2] + def change + create_table :pageflow_permalink_redirects do |t| + t.belongs_to :entry, index: true + t.string :slug + t.belongs_to :directory, index: true + + t.timestamps + + t.index ['slug', 'directory_id'], name: 'index_pageflow_permalink_redirects_on_slug_and_dir', unique: true + end + end +end diff --git a/spec/controllers/admin/entries_controller_spec.rb b/spec/controllers/admin/entries_controller_spec.rb index c3bb30e0b..a9013f164 100644 --- a/spec/controllers/admin/entries_controller_spec.rb +++ b/spec/controllers/admin/entries_controller_spec.rb @@ -579,6 +579,40 @@ def self.name expect(response.body) .not_to have_selector('input[name="entry[permalink_attributes][slug]"]') end + + it 'displays host of site in permalink input' do + user = create(:user) + account = create(:account, with_publisher: user) + account.default_site.update(cname: 'some.example.com') + create( + :permalink_directory, + path: '', + site: account.default_site + ) + + sign_in(user, scope: :user) + get :new + + expect(response.body) + .to have_selector('.permalink_base_url', text: 'some.example.com/') + end + + it 'prefers cononical entry url prefix in permalink input' do + user = create(:user) + account = create(:account, with_publisher: user) + account.default_site.update(canonical_entry_url_prefix: 'https://some.example.com/foo/') + create( + :permalink_directory, + path: '', + site: account.default_site + ) + + sign_in(user, scope: :user) + get :new + + expect(response.body) + .to have_selector('.permalink_base_url', text: 'some.example.com/foo/') + end end describe '#create' do diff --git a/spec/helpers/pageflow/entries_helper_spec.rb b/spec/helpers/pageflow/entries_helper_spec.rb index f665db402..767a0535e 100644 --- a/spec/helpers/pageflow/entries_helper_spec.rb +++ b/spec/helpers/pageflow/entries_helper_spec.rb @@ -40,7 +40,7 @@ module Pageflow describe '#pretty_entry_url' do it 'uses default host' do site = create(:site, cname: '') - entry = PublishedEntry.new(create(:entry, title: 'test', site: site), + entry = PublishedEntry.new(create(:entry, title: 'test', site:), create(:revision)) expect(helper.pretty_entry_url(entry)).to eq('http://test.host/test') @@ -48,7 +48,7 @@ module Pageflow it 'supports custom params' do site = create(:site, cname: '') - entry = PublishedEntry.new(create(:entry, title: 'test', site: site), + entry = PublishedEntry.new(create(:entry, title: 'test', site:), create(:revision)) expect(helper.pretty_entry_url(entry, page: 4)).to eq('http://test.host/test?page=4') @@ -56,7 +56,7 @@ module Pageflow it 'uses site cname if present' do site = create(:site, cname: 'my.example.com') - entry = PublishedEntry.new(create(:entry, title: 'test', site: site), + entry = PublishedEntry.new(create(:entry, title: 'test', site:), create(:revision)) expect(helper.pretty_entry_url(entry)).to eq('http://my.example.com/test') @@ -64,9 +64,9 @@ module Pageflow it 'uses permalink if present' do site = create(:site, - cname: 'my.example.com') + cname: 'my.example.com') entry = create(:published_entry, - site: site, + site:, permalink_attributes: { slug: 'custom', directory_path: 'de/' @@ -77,27 +77,27 @@ module Pageflow it 'uses site canonical entry url prefix if present' do site = create(:site, - canonical_entry_url_prefix: 'https://example.com/blog/') - entry = create(:published_entry, title: 'test', site: site) + canonical_entry_url_prefix: 'https://example.com/blog/') + entry = create(:published_entry, title: 'test', site:) expect(helper.pretty_entry_url(entry)).to eq('https://example.com/blog/test') end it 'prefers canonical entry url prefix over cname' do site = create(:site, - cname: 'my.example.com', - canonical_entry_url_prefix: 'https://example.com/blog/') - entry = create(:published_entry, title: 'test', site: site) + cname: 'my.example.com', + canonical_entry_url_prefix: 'https://example.com/blog/') + entry = create(:published_entry, title: 'test', site:) expect(helper.pretty_entry_url(entry)).to eq('https://example.com/blog/test') end it 'supports interpolating entry locale in canonical entry url prefix' do site = create(:site, - canonical_entry_url_prefix: 'https://example.com/:locale/blog/') + canonical_entry_url_prefix: 'https://example.com/:locale/blog/') entry = create(:published_entry, title: 'test', - site: site, + site:, revision_attributes: {locale: 'fr'}) expect(helper.pretty_entry_url(entry)).to eq('https://example.com/fr/blog/test') @@ -105,10 +105,10 @@ module Pageflow it 'supports reading locale from draft entry' do site = create(:site, - canonical_entry_url_prefix: 'https://example.com/:locale/blog/') + canonical_entry_url_prefix: 'https://example.com/:locale/blog/') entry = create(:draft_entry, title: 'test', - site: site, + site:, revision_attributes: {locale: 'fr'}) expect(helper.pretty_entry_url(entry)).to eq('https://example.com/fr/blog/test') @@ -116,11 +116,11 @@ module Pageflow it 'uses locale of published revision if entry model is passed' do site = create(:site, - canonical_entry_url_prefix: 'https://example.com/:locale/blog/') + canonical_entry_url_prefix: 'https://example.com/:locale/blog/') entry = create(:entry, :published, title: 'test', - site: site, + site:, published_revision_attributes: {locale: 'fr'}) expect(helper.pretty_entry_url(entry)).to eq('https://example.com/fr/blog/test') @@ -128,7 +128,7 @@ module Pageflow it 'falls back to draft locale if unpublished entry is passed' do site = create(:site, - canonical_entry_url_prefix: 'https://example.com/:locale/blog/') + canonical_entry_url_prefix: 'https://example.com/:locale/blog/') entry = create(:entry, title: 'test', site: site) @@ -137,11 +137,24 @@ module Pageflow expect(helper.pretty_entry_url(entry)).to eq('https://example.com/fr/blog/test') end + it 'combines canonical entry url preifx with permalink if present' do + site = create(:site, + canonical_entry_url_prefix: 'https://example.com/blog/') + entry = create(:published_entry, + site:, + permalink_attributes: { + slug: 'custom', + directory_path: 'de/' + }) + + expect(helper.pretty_entry_url(entry)).to eq('https://example.com/blog/de/custom') + end + it 'supports adding trailing slash' do site = create(:site, - cname: 'my.example.com', - trailing_slash_in_canonical_urls: true) - entry = PublishedEntry.new(create(:entry, title: 'test', site: site), + cname: 'my.example.com', + trailing_slash_in_canonical_urls: true) + entry = PublishedEntry.new(create(:entry, title: 'test', site:), create(:revision)) expect(helper.pretty_entry_url(entry)).to eq('http://my.example.com/test/') @@ -149,9 +162,9 @@ module Pageflow it 'supports adding trailing slash to url with custom prefix' do site = create(:site, - canonical_entry_url_prefix: 'https://example.com/blog/', - trailing_slash_in_canonical_urls: true) - entry = PublishedEntry.new(create(:entry, title: 'test', site: site), + canonical_entry_url_prefix: 'https://example.com/blog/', + trailing_slash_in_canonical_urls: true) + entry = PublishedEntry.new(create(:entry, title: 'test', site:), create(:revision)) expect(helper.pretty_entry_url(entry)).to eq('https://example.com/blog/test/') @@ -166,7 +179,9 @@ module Pageflow end it 'can be configured via lambda in public_entry_url_options' do - Pageflow.config.public_entry_url_options = lambda { |site| {host: "#{site.account.name}.example.com" } } + Pageflow.config.public_entry_url_options = lambda do |site| + {host: "#{site.account.name}.example.com"} + end account = create(:account, name: 'myaccount') entry = PublishedEntry.new(create(:entry, title: 'test', account: account), create(:revision)) diff --git a/spec/models/concerns/pageflow/permalinkable_spec.rb b/spec/models/concerns/pageflow/permalinkable_spec.rb index 38cd0fb48..2c9844de5 100644 --- a/spec/models/concerns/pageflow/permalinkable_spec.rb +++ b/spec/models/concerns/pageflow/permalinkable_spec.rb @@ -158,5 +158,131 @@ module Pageflow expect(entry).to have(1).error_on('permalink.slug') end + + it 'creates permalink redirect when slug changes' do + entry = create( + :entry, + :published, + permalink_attributes: { + slug: 'old-slug' + } + ) + + entry.update( + permalink_attributes: { + slug: 'new-slug' + } + ) + + expect(entry.permalink_redirects) + .to include(an_object_having_attributes(slug: 'old-slug', + directory: entry.permalink.directory)) + end + + it 'creates permalink redirect when directory changes' do + site = create(:site) + old_directory = create(:permalink_directory, site:, path: 'de/') + new_directory = create(:permalink_directory, site:, path: 'en/') + entry = create( + :entry, + :published, + site:, + permalink_attributes: { + slug: 'some-slug', + directory: old_directory + } + ) + + entry.update( + permalink_attributes: { + directory: new_directory + } + ) + + expect(entry.permalink_redirects) + .to include(an_object_having_attributes(slug: 'some-slug', + directory: old_directory)) + end + + it 'does not create permalink redirect when directory and slug are unchanged' do + entry = create( + :entry, + :published, + permalink_attributes: { + slug: 'some-slug' + } + ) + + entry.update( + permalink_attributes: { + updated_at: 1.day.ago + } + ) + + expect(entry.permalink_redirects).to be_empty + end + + it 'does not create permalink redirect for unpublished entries' do + site = create(:site) + old_directory = create(:permalink_directory, site:, path: 'de/') + new_directory = create(:permalink_directory, site:, path: 'en/') + entry = create( + :entry, + site:, + permalink_attributes: { + slug: 'some-slug', + directory: old_directory + } + ) + + entry.update( + permalink_attributes: { + slug: 'new-slug', + directory: new_directory + } + ) + + expect(entry.permalink_redirects).to be_empty + end + + it 'removes old redirect when new is created' do + directory = create(:permalink_directory) + entry = create( + :entry, + :published, + site: directory.site, + permalink_attributes: { + directory:, + slug: 'old-slug' + } + ) + other_entry = create( + :entry, + :published, + site: directory.site, + permalink_attributes: { + directory:, + slug: 'other-slug' + } + ) + + entry.update( + permalink_attributes: { + slug: 'new-slug' + } + ) + other_entry.update( + permalink_attributes: { + slug: 'old-slug' + } + ) + other_entry.update( + permalink_attributes: { + slug: 'other-new-slug' + } + ) + + expect(entry.permalink_redirects.reload).to be_empty + end end end diff --git a/spec/models/pageflow/published_entry_spec.rb b/spec/models/pageflow/published_entry_spec.rb index b5c17f3bc..be47ac1d0 100644 --- a/spec/models/pageflow/published_entry_spec.rb +++ b/spec/models/pageflow/published_entry_spec.rb @@ -564,6 +564,78 @@ module Pageflow end end + describe '.find_by_permalink_redirect' do + it 'finds entry based on permalink slug in root directory' do + entry = create( + :entry, + :published, + permalink_attributes: { + slug: 'old-slug' + } + ) + entry.update( + permalink_attributes: { + slug: 'new-slug' + } + ) + + result = PublishedEntry.find_by_permalink_redirect( + site: entry.site, + slug: 'old-slug' + ) + + expect(result).to be_kind_of(PublishedEntry) + expect(result.id).to eq(entry.id) + end + + it 'finds entry based on permalink slug and directory' do + entry = create( + :entry, + :published, + permalink_attributes: { + slug: 'old-slug', + directory_path: 'en/' + } + ) + entry.update( + permalink_attributes: { + slug: 'new-slug' + } + ) + + result = PublishedEntry.find_by_permalink_redirect( + site: entry.site, + directory: 'en/', + slug: 'old-slug' + ) + + expect(result.id).to eq(entry.id) + end + + it 'does not find not published entries' do + entry = create( + :entry, + permalink_attributes: { + slug: 'old-slug', + directory_path: 'en/' + } + ) + entry.update( + permalink_attributes: { + slug: 'new-slug' + } + ) + + result = PublishedEntry.find_by_permalink_redirect( + site: entry.site, + directory: 'en/', + slug: 'old-slug' + ) + + expect(result).to be_nil + end + end + describe '#wrap_all' do it 'returns array of published entries for all entries in scope' do create(:entry, diff --git a/spec/requests/pageflow/entries_show_request_spec.rb b/spec/requests/pageflow/entries_show_request_spec.rb index 64aacff5a..14a585ded 100644 --- a/spec/requests/pageflow/entries_show_request_spec.rb +++ b/spec/requests/pageflow/entries_show_request_spec.rb @@ -103,6 +103,110 @@ module Pageflow .to include('some-entry published rendered by entry type frontend app') end + it 'redirects to renamed permalink' do + site = create(:site, cname: 'my.example.com') + entry = create( + :entry, + :published, + title: 'some-entry', + type_name: 'test', + site:, + permalink_attributes: { + slug: 'old-slug', + directory_path: 'en/' + } + ) + + entry.permalink.update!(slug: 'new-slug') + get('http://my.example.com/en/old-slug') + + expect(response).to redirect_to('http://my.example.com/en/new-slug') + end + + it 'redirects even when new entry with same permalink is created but not yet published' do + site = create(:site, cname: 'my.example.com') + directory = create(:permalink_directory, site:) + entry = create( + :entry, + :published, + title: 'some-entry', + type_name: 'test', + site:, + permalink_attributes: { + slug: 'old-slug', + directory: + } + ) + + entry.permalink.update!(slug: 'new-slug') + create( + :entry, + title: 'new-entry', + type_name: 'test', + site:, + permalink_attributes: { + slug: 'old-slug', + directory: + } + ) + get('http://my.example.com/old-slug') + + expect(response).to redirect_to('http://my.example.com/new-slug') + end + + it 'no longer redirects when new entry with same permalink is published' do + site = create(:site, cname: 'my.example.com') + directory = create(:permalink_directory, site:) + entry = create( + :entry, + :published, + title: 'some-entry', + type_name: 'test', + site:, + permalink_attributes: { + slug: 'old-slug', + directory: + } + ) + + entry.permalink.update!(slug: 'new-slug') + create( + :entry, + :published, + title: 'new-entry', + type_name: 'test', + site:, + permalink_attributes: { + slug: 'old-slug', + directory: + } + ) + get('http://my.example.com/old-slug') + + expect(response.body).to include('new-entry') + end + + it 'redirects to permalink that moved to different site' do + site = create(:site, cname: 'my.example.com') + entry = create( + :entry, + :published, + type_name: 'test', + site:, + permalink_attributes: { + slug: 'some-slug', + directory_path: 'en/' + } + ) + other_site = create(:site, cname: 'other.example.com') + directory_of_other_site = create(:permalink_directory, site: other_site) + + entry.update!(site: other_site, permalink_attributes: {directory: directory_of_other_site}) + get('http://my.example.com/en/some-slug') + + expect(response).to redirect_to('http://other.example.com/some-slug') + end + it 'responds with not found for non-published entry' do entry = create(:entry, type_name: 'test')