From 6be7a66ba7aba67e528ae0f530694b6876128a0f Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 8 Jul 2020 17:19:01 +1000 Subject: [PATCH] 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. --- .../discourse/app/templates/modal/bookmark.hbs | 2 +- app/models/bookmark.rb | 3 ++- ...0708051009_cap_bookmark_name_at_100_characters.rb | 12 ++++++++++++ spec/lib/bookmark_manager_spec.rb | 7 +++++++ 4 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20200708051009_cap_bookmark_name_at_100_characters.rb diff --git a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs index d3811ac9abd..17f41b174ef 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}}
- {{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"}}
diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb index c90af0f147b..32ef5169ff5 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 00000000000..1428778531e --- /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 6aabf798240..9d2a05f6db2 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