From a835fd99bd6de6916395b50354be60667f0e3281 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nenciu.bianca@gmail.com>
Date: Wed, 11 Dec 2024 18:23:30 +0200
Subject: [PATCH] FIX: Truncate bookmarks.name when remapping

The new name may be too long for the bookmarks.name column and raise an
exception. This changes allows the remapper to truncate the new value to
fit (truncates to 100 characters).
---
 lib/db_helper.rb           |  2 +-
 spec/lib/db_helper_spec.rb | 64 ++++++++++++++++++++------------------
 2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/lib/db_helper.rb b/lib/db_helper.rb
index 561ac51896b..c8e5d0691cd 100644
--- a/lib/db_helper.rb
+++ b/lib/db_helper.rb
@@ -18,7 +18,7 @@ class DbHelper
      WHERE trigger_name LIKE '%_readonly'
   SQL
 
-  TRUNCATABLE_COLUMNS = ["topic_links.url"]
+  TRUNCATABLE_COLUMNS = %w[bookmarks.name topic_links.url]
 
   def self.remap(
     from,
diff --git a/spec/lib/db_helper_spec.rb b/spec/lib/db_helper_spec.rb
index 29d9784cfc8..9b4377cd4ac 100644
--- a/spec/lib/db_helper_spec.rb
+++ b/spec/lib/db_helper_spec.rb
@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
 RSpec.describe DbHelper do
-  fab!(:bookmark1) { Fabricate(:bookmark, name: "short-bookmark") }
-  fab!(:bookmark2) { Fabricate(:bookmark, name: "another-bookmark") }
-  let(:bookmark_name_limit) { Bookmark.columns_hash["name"].limit }
-  let(:long_bookmark_name) { "a" * (bookmark_name_limit + 1) }
+  fab!(:sidebar_url1) { Fabricate(:sidebar_url, name: "short-sidebar-url") }
+  fab!(:sidebar_url2) { Fabricate(:sidebar_url, name: "another-sidebar-url") }
+  let(:sidebar_url_name_limit) { SidebarUrl.columns_hash["name"].limit }
+  let(:long_sidebar_url_name) { "a" * (sidebar_url_name_limit + 1) }
 
   describe ".remap" do
     it "should remap columns properly" do
@@ -52,39 +52,39 @@ RSpec.describe DbHelper do
 
     context "when skip_max_length_violations is false" do
       it "raises an exception if remap exceeds column length constraint by default" do
-        expect { DbHelper.remap("bookmark", long_bookmark_name) }.to raise_error(
+        expect { DbHelper.remap("sidebar-url", long_sidebar_url_name) }.to raise_error(
           PG::StringDataRightTruncation,
-          /value too long.*table: bookmarks,.*name/,
+          /value too long.*table: sidebar_urls,.*name/,
         )
       end
     end
 
     context "when skip_max_length_violations is true" do
       it "skips a remap eligible row if new value exceeds column length constraint" do
-        DbHelper.remap("bookmark", long_bookmark_name, skip_max_length_violations: true)
+        DbHelper.remap("sidebar-url", long_sidebar_url_name, skip_max_length_violations: true)
 
-        bookmark1.reload
-        bookmark2.reload
+        sidebar_url1.reload
+        sidebar_url2.reload
 
-        expect(bookmark1.name).to eq("short-bookmark")
-        expect(bookmark2.name).to eq("another-bookmark")
+        expect(sidebar_url1.name).to eq("short-sidebar-url")
+        expect(sidebar_url2.name).to eq("another-sidebar-url")
       end
 
       it "logs skipped remaps due to max length constraints when verbose is true" do
         expect {
           DbHelper.remap(
-            "bookmark",
-            long_bookmark_name,
+            "sidebar-url",
+            long_sidebar_url_name,
             verbose: true,
             skip_max_length_violations: true,
           )
         }.to output(/SKIPPED:/).to_stdout
 
-        bookmark1.reload
-        bookmark2.reload
+        sidebar_url1.reload
+        sidebar_url2.reload
 
-        expect(bookmark1.name).to eq("short-bookmark")
-        expect(bookmark2.name).to eq("another-bookmark")
+        expect(sidebar_url1.name).to eq("short-sidebar-url")
+        expect(sidebar_url2.name).to eq("another-sidebar-url")
       end
     end
   end
@@ -100,39 +100,43 @@ RSpec.describe DbHelper do
 
     context "when skip_max_length_violations is false" do
       it "raises an exception if regexp_replace exceeds column length constraint by default" do
-        expect { DbHelper.regexp_replace("bookmark", long_bookmark_name) }.to raise_error(
+        expect { DbHelper.regexp_replace("sidebar-url", long_sidebar_url_name) }.to raise_error(
           PG::StringDataRightTruncation,
-          /value too long.*table: bookmarks,.*name/,
+          /value too long.*table: sidebar_urls,.*name/,
         )
       end
     end
 
     context "when skip_max_length_violations is true" do
       it "skips regexp_replace eligible rows if new value exceeds column length constraint" do
-        DbHelper.regexp_replace("bookmark", long_bookmark_name, skip_max_length_violations: true)
+        DbHelper.regexp_replace(
+          "sidebar-url",
+          long_sidebar_url_name,
+          skip_max_length_violations: true,
+        )
 
-        bookmark1.reload
-        bookmark2.reload
+        sidebar_url1.reload
+        sidebar_url2.reload
 
-        expect(bookmark1.name).to eq("short-bookmark")
-        expect(bookmark2.name).to eq("another-bookmark")
+        expect(sidebar_url1.name).to eq("short-sidebar-url")
+        expect(sidebar_url2.name).to eq("another-sidebar-url")
       end
 
       it "logs skipped regexp_replace due to max length constraints when verbose is true" do
         expect {
           DbHelper.regexp_replace(
-            "bookmark",
-            long_bookmark_name,
+            "sidebar-url",
+            long_sidebar_url_name,
             verbose: true,
             skip_max_length_violations: true,
           )
         }.to output(/SKIPPED:/).to_stdout
 
-        bookmark1.reload
-        bookmark2.reload
+        sidebar_url1.reload
+        sidebar_url2.reload
 
-        expect(bookmark1.name).to eq("short-bookmark")
-        expect(bookmark2.name).to eq("another-bookmark")
+        expect(sidebar_url1.name).to eq("short-sidebar-url")
+        expect(sidebar_url2.name).to eq("another-sidebar-url")
       end
     end
   end