Skip to content

Commit

Permalink
Clear unnecessary legacy URLs
Browse files Browse the repository at this point in the history
When renaming a page back to a URL it used to have, we get a legacy URL
that is the same as the current URL of the page, which can lead to
infinite redirects. This deletes any legacy URLs that are the same as
the current URL name in a callback.
  • Loading branch information
mamhoff committed Jan 8, 2024
1 parent 1f5553a commit a6b2397
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
7 changes: 7 additions & 0 deletions app/models/alchemy/page/page_naming.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ module PageNaming
after_update :update_descendants_urlnames,
if: :saved_change_to_urlname?

before_save :destroy_obsolete_legacy_urls, if: :renamed?

after_move :update_urlname!
end

Expand Down Expand Up @@ -55,6 +57,11 @@ def update_descendants_urlnames
descendants.each(&:update_urlname!)
end

def destroy_obsolete_legacy_urls
obsolete_legacy_urls = legacy_urls.select { |legacy_url| legacy_url.urlname == urlname }
legacy_urls.destroy(obsolete_legacy_urls)
end

# Sets the urlname to a url friendly slug.
# Either from name, or if present, from urlname.
# The urlname contains the whole path including parent urlnames.
Expand Down
12 changes: 12 additions & 0 deletions spec/models/alchemy/page_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1689,6 +1689,18 @@ def copy_children_to(new_parent)
expect(page.legacy_urls.pluck(:urlname)).to include("parentparent/parent/page")
end
end

context "if new urlname exists as a legacy url" do
it "will delete obsolete legacy_urls" do
expect(page.urlname).to eq("parentparent/parent/page")
page.update!(urlname: "other-name")
expect(page.legacy_urls.pluck(:urlname)).to include("parentparent/parent/page")
page.update!(urlname: "page")
expect(page.legacy_urls.pluck(:urlname)).to include("parentparent/parent/other-name")
expect(page.urlname).to eq("parentparent/parent/page")
expect(page.legacy_urls.pluck(:urlname)).not_to include("parentparent/parent/page")
end
end
end

describe "#update_node!" do
Expand Down

0 comments on commit a6b2397

Please sign in to comment.