FIX: Cap bookmark name at 100 chars and truncate existing names (#10189)

FIX: Cap bookmark name at 100 chars and truncate existing names (#10189)

We have a couple of examples of enormous amounts of text being entered in the name column of bookmarks. This is not desirable…it is just meant to be a short note / reminder of why you bookmarked this.

This PR caps the column at 100 characters and truncates existing names in the database to 100 characters.

diff --git a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs
index d3811ac..17f41b1 100644
--- a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs
+++ b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs
@@ -9,7 +9,7 @@
     {{/if}}
 
     <div class="control-group bookmark-name-wrap">
-      {{input id="bookmark-name" value=model.name name="bookmark-name" class="bookmark-name" enter=(action "saveAndClose") placeholder=(i18n "post.bookmarks.name_placeholder")}}
+      {{input id="bookmark-name" value=model.name name="bookmark-name" class="bookmark-name" enter=(action "saveAndClose") placeholder=(i18n "post.bookmarks.name_placeholder") maxlength="100"}}
       {{d-button icon="cog" action=(action "toggleOptionsPanel") class="bookmark-options-button"}}
     </div>
 
diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb
index c90af0f..32ef516 100644
--- a/app/models/bookmark.rb
+++ b/app/models/bookmark.rb
@@ -12,6 +12,7 @@ class Bookmark < ActiveRecord::Base
 
   validate :unique_per_post_for_user
   validate :ensure_sane_reminder_at_time
+  validates :name, length: { maximum: 100 }
 
   # we don't care whether the post or topic is deleted,
   # they hold important information about the bookmark
@@ -83,7 +84,7 @@ end
 #  user_id                   :bigint           not null
 #  topic_id                  :bigint           not null
 #  post_id                   :bigint           not null
-#  name                      :string
+#  name                      :string(100)
 #  reminder_type             :integer
 #  reminder_at               :datetime
 #  created_at                :datetime         not null
diff --git a/db/migrate/20200708051009_cap_bookmark_name_at_100_characters.rb b/db/migrate/20200708051009_cap_bookmark_name_at_100_characters.rb
new file mode 100644
index 0000000..1428778
--- /dev/null
+++ b/db/migrate/20200708051009_cap_bookmark_name_at_100_characters.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class CapBookmarkNameAt100Characters < ActiveRecord::Migration[6.0]
+  def up
+    DB.exec("UPDATE bookmarks SET name = LEFT(name, 100) WHERE name IS NOT NULL AND name <> LEFT(name, 100)")
+    change_column :bookmarks, :name, :string, limit: 100
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end
diff --git a/spec/lib/bookmark_manager_spec.rb b/spec/lib/bookmark_manager_spec.rb
index 6aabf79..9d2a05f 100644
--- a/spec/lib/bookmark_manager_spec.rb
+++ b/spec/lib/bookmark_manager_spec.rb
@@ -75,6 +75,13 @@ RSpec.describe BookmarkManager do
       end
     end
 
+    context "when the bookmark name is too long" do
+      it "adds an error to the manager" do
+        subject.create(post_id: post.id, name: "test" * 100)
+        expect(subject.errors.full_messages).to include("Name is too long (maximum is 100 characters)")
+      end
+    end
+
     context "when the reminder time is not provided when it needs to be" do
       let(:reminder_at) { nil }
       it "adds an error to the manager" do

GitHub sha: 6be7a66b

1 Like

This commit appears in #10189 which was merged by martin.