From 4eadce8ffe5a3167ae4625d954a10d9871f9c97f Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Tue, 24 Feb 2026 08:11:47 +0100 Subject: [PATCH] Automatically update account slugs in links --- .../action_text/rich_text_record_set.rb | 29 +++++++++-- .../action_text/rich_text_record_set_test.rb | 48 ++++++++++++++++--- test/models/account/import_test.rb | 30 ++++++++++++ 3 files changed, 96 insertions(+), 11 deletions(-) diff --git a/app/models/account/data_transfer/action_text/rich_text_record_set.rb b/app/models/account/data_transfer/action_text/rich_text_record_set.rb index d56a12dc1..e29abd011 100644 --- a/app/models/account/data_transfer/action_text/rich_text_record_set.rb +++ b/app/models/account/data_transfer/action_text/rich_text_record_set.rb @@ -31,7 +31,7 @@ class Account::DataTransfer::ActionText::RichTextRecordSet < Account::DataTransf def import_batch(files) batch_data = files.map do |file| data = load(file) - data["body"] = convert_gids_to_sgids(data["body"]) + data["body"] = transform_body_for_import(data["body"]) data.slice(*ATTRIBUTES).merge("account_id" => account.id) end @@ -75,11 +75,16 @@ class Account::DataTransfer::ActionText::RichTextRecordSet < Account::DataTransf content.fragment.source.to_html end - def convert_gids_to_sgids(html) - return html if html.blank? + def transform_body_for_import(body) + return body if body.blank? - fragment = Nokogiri::HTML.fragment(html) + Nokogiri::HTML.fragment(body) + .then { convert_gids_to_sgids(it) } + .then { replace_account_slugs(it) } + .to_html + end + def convert_gids_to_sgids(fragment) fragment.css("action-text-attachment[gid]").each do |node| gid = GlobalID.parse(node["gid"]) @@ -97,6 +102,20 @@ class Account::DataTransfer::ActionText::RichTextRecordSet < Account::DataTransf end end - fragment.to_html + fragment + end + + def replace_account_slugs(fragment) + fragment.css("a[href]").each do |link| + match = link["href"].match(AccountSlug::PATH_INFO_MATCH) + + if match + path = match.post_match.presence || "/" + valid_path = Rails.application.routes.recognize_path(path) rescue nil + link["href"] = "#{account.slug}#{remaining_path}" if valid_path + end + end + + fragment end end diff --git a/test/models/account/data_transfer/action_text/rich_text_record_set_test.rb b/test/models/account/data_transfer/action_text/rich_text_record_set_test.rb index 08e0b5610..bee51869c 100644 --- a/test/models/account/data_transfer/action_text/rich_text_record_set_test.rb +++ b/test/models/account/data_transfer/action_text/rich_text_record_set_test.rb @@ -42,7 +42,7 @@ class Account::DataTransfer::ActionText::RichTextRecordSetTest < ActiveSupport:: importing_account&.destroy end - test "convert_gids_to_sgids skips GIDs belonging to another account" do + test "transform_body_for_import skips GIDs belonging to another account" do victim_tag = tags(:web) attacker_account = accounts(:initech) assert_not_equal attacker_account.id, victim_tag.account_id @@ -51,13 +51,13 @@ class Account::DataTransfer::ActionText::RichTextRecordSetTest < ActiveSupport:: html = %() record_set = Account::DataTransfer::ActionText::RichTextRecordSet.new(attacker_account) - result = record_set.send(:convert_gids_to_sgids, html) + result = record_set.send(:transform_body_for_import, html) assert_no_match(/sgid=/, result, "Cross-tenant GID must not be converted to SGID") assert_match(/gid=/, result, "Original GID should remain unconverted") end - test "convert_gids_to_sgids converts GIDs belonging to the same account" do + test "transform_body_for_import converts GIDs belonging to the same account" do own_tag = tags(:web) own_account = accounts(:"37s") assert_equal own_account.id, own_tag.account_id @@ -66,19 +66,55 @@ class Account::DataTransfer::ActionText::RichTextRecordSetTest < ActiveSupport:: html = %() record_set = Account::DataTransfer::ActionText::RichTextRecordSet.new(own_account) - result = record_set.send(:convert_gids_to_sgids, html) + result = record_set.send(:transform_body_for_import, html) assert_match(/sgid=/, result, "Same-account GID should be converted to SGID") assert_no_match(/ gid=/, result, "GID should be removed after SGID conversion") end - test "convert_gids_to_sgids handles non-existent record GIDs gracefully" do + test "transform_body_for_import handles non-existent record GIDs gracefully" do nonexistent_gid = "gid://fizzy/Tag/00000000000000000000000000" html = %() record_set = Account::DataTransfer::ActionText::RichTextRecordSet.new(accounts(:"37s")) - result = record_set.send(:convert_gids_to_sgids, html) + result = record_set.send(:transform_body_for_import, html) assert_no_match(/sgid=/, result, "Non-existent record should not produce SGID") end + + test "replace_account_slugs rewrites relative URLs with account slug" do + target_account = accounts(:"37s") + source_slug = "9999999" + target_slug = AccountSlug.encode(target_account.external_account_id) + + html = %(

See card 42

) + + record_set = Account::DataTransfer::ActionText::RichTextRecordSet.new(target_account) + result = record_set.send(:transform_body_for_import, html) + + assert_includes result, "/#{target_slug}/cards/42" + assert_not_includes result, source_slug + end + + test "replace_account_slugs leaves absolute URLs alone" do + target_account = accounts(:"37s") + + html = %(

See board

) + + record_set = Account::DataTransfer::ActionText::RichTextRecordSet.new(target_account) + result = record_set.send(:transform_body_for_import, html) + + assert_includes result, "https://fizzy.app/9999999/boards/7" + end + + test "replace_account_slugs leaves plain text alone" do + target_account = accounts(:"37s") + + html = "

Nothing to rewrite here

" + + record_set = Account::DataTransfer::ActionText::RichTextRecordSet.new(target_account) + result = record_set.send(:transform_body_for_import, html) + + assert_equal html, result + end end diff --git a/test/models/account/import_test.rb b/test/models/account/import_test.rb index a33cc32c2..7bf407041 100644 --- a/test/models/account/import_test.rb +++ b/test/models/account/import_test.rb @@ -62,6 +62,36 @@ class Account::ImportTest < ActiveSupport::TestCase export_tempfile&.unlink end + test "import reconciles cards count so new cards get correct numbers" do + source_account = accounts("37s") + exporter = users(:david) + identity = exporter.identity + max_card_number = source_account.cards.maximum(:number) + + export = Account::Export.create!(account: source_account, user: exporter) + export.build + + export_tempfile = Tempfile.new([ "export", ".zip" ]) + export.file.open { |f| FileUtils.cp(f.path, export_tempfile.path) } + + source_account.destroy! + + target_account = Account.create_with_owner(account: { name: "Import Test" }, owner: { identity: identity, name: exporter.name }) + import = Account::Import.create!(identity: identity, account: target_account) + Current.set(account: target_account) do + import.file.attach(io: File.open(export_tempfile.path), filename: "export.zip", content_type: "application/zip") + end + + import.check + import.process + + target_account.reload + assert_operator target_account.cards_count, :>=, max_card_number + ensure + export_tempfile&.close + export_tempfile&.unlink + end + test "check sets no failure_reason for unexpected errors" do import = Account::Import.create!(identity: identities(:david), account: Account.create!(name: "Import Test"))