diff --git a/app/assets/javascripts/discourse/app/controllers/bookmark.js b/app/assets/javascripts/discourse/app/controllers/bookmark.js index c2a6b5585a..6d4b5f9512 100644 --- a/app/assets/javascripts/discourse/app/controllers/bookmark.js +++ b/app/assets/javascripts/discourse/app/controllers/bookmark.js @@ -27,6 +27,20 @@ const LATER_TODAY_CUTOFF_HOUR = 17; const LATER_TODAY_MAX_HOUR = 18; const MOMENT_MONDAY = 1; const MOMENT_THURSDAY = 4; +const AUTO_DELETE_PREFERENCES = [ + { + id: 0, + name: I18n.t("bookmarks.auto_delete_preference.never") + }, + { + id: 1, + name: I18n.t("bookmarks.auto_delete_preference.when_reminder_sent") + }, + { + id: 2, + name: I18n.t("bookmarks.auto_delete_preference.on_owner_reply") + } +]; const BOOKMARK_BINDINGS = { enter: { handler: "saveAndClose" }, @@ -65,7 +79,6 @@ export default Controller.extend(ModalFunctionality, { mouseTrap: null, userTimezone: null, showOptions: false, - options: null, onShow() { this.setProperties({ @@ -79,7 +92,6 @@ export default Controller.extend(ModalFunctionality, { lastCustomReminderTime: null, userTimezone: this.currentUser.resolvedTimezone(this.currentUser), showOptions: false, - options: {}, model: this.model || {} }); @@ -143,19 +155,26 @@ export default Controller.extend(ModalFunctionality, { _loadBookmarkOptions() { this.set( - "options.deleteWhenReminderSent", - this.model.deleteWhenReminderSent || - localStorage.bookmarkOptionsDeleteWhenReminderSent === "true" + "autoDeletePreference", + this.model.autoDeletePreference || this._preferredDeleteOption() || 0 ); // we want to make sure the options panel opens so the user // knows they have set these options previously. run next otherwise // the modal is not visible when it tries to slide down the options - if (this.options.deleteWhenReminderSent) { + if (this.autoDeletePreference) { next(() => this.toggleOptionsPanel()); } }, + _preferredDeleteOption() { + let preferred = localStorage.bookmarkDeleteOption; + if (preferred && preferred !== "") { + preferred = parseInt(preferred, 10); + } + return preferred; + }, + _loadLastUsedCustomReminderDatetime() { let lastTime = localStorage.lastCustomBookmarkReminderTime; let lastDate = localStorage.lastCustomBookmarkReminderDate; @@ -217,6 +236,11 @@ export default Controller.extend(ModalFunctionality, { return REMINDER_TYPES; }, + @discourseComputed() + autoDeletePreferences: () => { + return AUTO_DELETE_PREFERENCES; + }, + showLastCustom: and("lastCustomReminderTime", "lastCustomReminderDate"), get showLaterToday() { @@ -294,7 +318,7 @@ export default Controller.extend(ModalFunctionality, { localStorage.lastCustomBookmarkReminderDate = this.customReminderDate; } - localStorage.bookmarkOptionsDeleteWhenReminderSent = this.options.deleteWhenReminderSent; + localStorage.bookmarkDeleteOption = this.autoDeletePreference; let reminderType; if (this.selectedReminderType === REMINDER_TYPES.NONE) { @@ -311,7 +335,7 @@ export default Controller.extend(ModalFunctionality, { name: this.model.name, post_id: this.model.postId, id: this.model.id, - delete_when_reminder_sent: this.options.deleteWhenReminderSent + auto_delete_preference: this.autoDeletePreference }; if (this._editingExistingBookmark()) { @@ -323,7 +347,7 @@ export default Controller.extend(ModalFunctionality, { this.afterSave({ reminderAt: reminderAtISO, reminderType: this.selectedReminderType, - deleteWhenReminderSent: this.options.deleteWhenReminderSent, + autoDeletePreference: this.autoDeletePreference, id: this.model.id, name: this.model.name }); @@ -335,7 +359,7 @@ export default Controller.extend(ModalFunctionality, { this.afterSave({ reminderAt: reminderAtISO, reminderType: this.selectedReminderType, - deleteWhenReminderSent: this.options.deleteWhenReminderSent, + autoDeletePreference: this.autoDeletePreference, id: response.id, name: this.model.name }); diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index 54e69b2770..4845a00e64 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -110,6 +110,10 @@ export default Controller.extend(bufferedProperty("model"), { this._super(...arguments); this.appEvents.on("post:show-revision", this, "_showRevision"); + this.appEvents.on("post:created", this, () => { + this._removeDeleteOnOwnerReplyBookmarks(); + this.appEvents.trigger("post-stream:refresh", { force: true }); + }); this.setProperties({ selectedPostIds: [], @@ -183,6 +187,15 @@ export default Controller.extend(bufferedProperty("model"), { : null; }, + _removeDeleteOnOwnerReplyBookmarks() { + let posts = this.model.get("postStream").posts; + posts + .filter(p => p.bookmarked && p.bookmark_auto_delete_preference === 2) // 2 is on_owner_reply + .forEach(p => { + p.clearBookmark(); + }); + }, + _forceRefreshPostStream() { this.appEvents.trigger("post-stream:refresh", { force: true }); }, diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js index 18bc421fea..f181eaefe2 100644 --- a/app/assets/javascripts/discourse/app/models/post.js +++ b/app/assets/javascripts/discourse/app/models/post.js @@ -305,7 +305,7 @@ const Post = RestModel.extend({ postId: this.id, id: this.bookmark_id, reminderAt: this.bookmark_reminder_at, - deleteWhenReminderSent: this.bookmark_delete_when_reminder_sent, + autoDeletePreference: this.bookmark_auto_delete_preference, name: this.bookmark_name }, title: this.bookmark_id @@ -324,8 +324,7 @@ const Post = RestModel.extend({ bookmarked: true, bookmark_reminder_at: savedData.reminderAt, bookmark_reminder_type: savedData.reminderType, - bookmark_delete_when_reminder_sent: - savedData.deleteWhenReminderSent, + bookmark_auto_delete_preference: savedData.autoDeletePreference, bookmark_name: savedData.name, bookmark_id: savedData.id }); @@ -334,19 +333,24 @@ const Post = RestModel.extend({ }, afterDelete: topicBookmarked => { this.set("topic.bookmarked", topicBookmarked); - this.setProperties({ - bookmark_reminder_at: null, - bookmark_reminder_type: null, - bookmark_name: null, - bookmark_id: null, - bookmarked: false - }); + this.clearBookmark(); this.appEvents.trigger("page:bookmark-post-toggled", this); } }); }); }, + clearBookmark() { + this.setProperties({ + bookmark_reminder_at: null, + bookmark_reminder_type: null, + bookmark_name: null, + bookmark_id: null, + bookmarked: false, + bookmark_auto_delete_preference: null + }); + }, + updateActionsSummary(json) { if (json && json.id === this.id) { json = Post.munge(json); diff --git a/app/assets/javascripts/discourse/app/routes/topic-from-params.js b/app/assets/javascripts/discourse/app/routes/topic-from-params.js index 1d70023660..c592b85edc 100644 --- a/app/assets/javascripts/discourse/app/routes/topic-from-params.js +++ b/app/assets/javascripts/discourse/app/routes/topic-from-params.js @@ -73,12 +73,7 @@ export default DiscourseRoute.extend({ // completely clear out all the bookmark related attributes // because they are not in the response if bookmarked == false if (closestPost && !closestPost.bookmarked) { - closestPost.setProperties({ - bookmark_reminder_at: null, - bookmark_reminder_type: null, - bookmark_name: null, - bookmark_id: null - }); + closestPost.clearBookmark(); } if (!isEmpty(topic.draft)) { diff --git a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs index 87d75375c1..5335a6e9b5 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs @@ -14,10 +14,13 @@
- + + {{combo-box + content=autoDeletePreferences + value=autoDeletePreference + class="bookmark-option-selector" + onChange=(action (mut autoDeletePreference)) + }}
{{#if showExistingReminderAt }} diff --git a/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs b/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs index c4a0425e8b..07effe370c 100644 --- a/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs @@ -35,9 +35,9 @@ {{#if site.mobileView}} {{#if bookmark.post_user_avatar_template}} - - {{avatar bookmark.postUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}} - + + {{avatar bookmark.postUser avatarTemplatePath="avatar_template" usernamePath="username" namePath="name" imageSize="small"}} + {{/if}} {{/if}} diff --git a/app/assets/stylesheets/common/components/bookmark-modal.scss b/app/assets/stylesheets/common/components/bookmark-modal.scss index 8d65fb4866..5a5d62f96b 100644 --- a/app/assets/stylesheets/common/components/bookmark-modal.scss +++ b/app/assets/stylesheets/common/components/bookmark-modal.scss @@ -75,14 +75,24 @@ margin-left: 0.5em; margin-bottom: 0.5em; background: transparent; + padding-right: 6px; } .bookmark-options-panel { + .select-kit { + width: 100%; + } + margin-bottom: 18px; display: none; - input[type="checkbox"] { - margin-right: 0.85em; + label { + display: flex; + + span { + display: block; + flex: 1; + } } } } diff --git a/app/controllers/bookmarks_controller.rb b/app/controllers/bookmarks_controller.rb index 001e75c119..7d103ea507 100644 --- a/app/controllers/bookmarks_controller.rb +++ b/app/controllers/bookmarks_controller.rb @@ -13,7 +13,7 @@ class BookmarksController < ApplicationController reminder_type: params[:reminder_type], reminder_at: params[:reminder_at], options: { - delete_when_reminder_sent: params[:delete_when_reminder_sent] + auto_delete_preference: params[:auto_delete_preference] || 0 } ) @@ -40,7 +40,7 @@ class BookmarksController < ApplicationController reminder_type: params[:reminder_type], reminder_at: params[:reminder_at], options: { - delete_when_reminder_sent: params[:delete_when_reminder_sent] + auto_delete_preference: params[:auto_delete_preference] || 0 } ) diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb index 32ef5169ff..9918e0f06d 100644 --- a/app/models/bookmark.rb +++ b/app/models/bookmark.rb @@ -44,6 +44,14 @@ class Bookmark < ActiveRecord::Base self.reminder_at.blank? && self.reminder_type.blank? end + def delete_when_reminder_sent? + self.auto_delete_preference == Bookmark.auto_delete_preferences[:when_reminder_sent] + end + + def delete_on_owner_reply? + self.auto_delete_preference == Bookmark.auto_delete_preferences[:on_owner_reply] + end + scope :pending_reminders, ->(before_time = Time.now.utc) do where("reminder_at IS NOT NULL AND reminder_at <= :before_time", before_time: before_time) end @@ -53,7 +61,7 @@ class Bookmark < ActiveRecord::Base end def self.reminder_types - @reminder_type = Enum.new( + @reminder_types ||= Enum.new( later_today: 1, next_business_day: 2, tomorrow: 3, @@ -65,6 +73,14 @@ class Bookmark < ActiveRecord::Base ) end + def self.auto_delete_preferences + @auto_delete_preferences ||= Enum.new( + never: 0, + when_reminder_sent: 1, + on_owner_reply: 2 + ) + end + def self.count_per_day(opts = nil) opts ||= {} result = where('bookmarks.created_at >= ?', opts[:start_date] || (opts[:since_days_ago] || 30).days.ago) @@ -91,7 +107,7 @@ end # updated_at :datetime not null # reminder_last_sent_at :datetime # reminder_set_at :datetime -# delete_when_reminder_sent :boolean default(FALSE), not null +# auto_delete_preference :integer # # Indexes # diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 2372a16a98..56ac4fe766 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -54,7 +54,7 @@ class PostSerializer < BasicPostSerializer :bookmark_id, :bookmark_reminder_type, :bookmark_name, - :bookmark_delete_when_reminder_sent, + :bookmark_auto_delete_preference, :raw, :actions_summary, :moderator?, @@ -335,7 +335,11 @@ class PostSerializer < BasicPostSerializer bookmarked end - def include_bookmark_delete_when_reminder_sent? + def include_bookmark_auto_delete_preference? + bookmarked + end + + def include_bookmark_delete_on_owner_reply? bookmarked end @@ -361,8 +365,8 @@ class PostSerializer < BasicPostSerializer post_bookmark&.name end - def bookmark_delete_when_reminder_sent - post_bookmark&.delete_when_reminder_sent + def bookmark_auto_delete_preference + post_bookmark&.auto_delete_preference end def bookmark_id diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 1aaed41651..9ddc636610 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -319,7 +319,11 @@ en: no_timezone: 'You have not set a timezone yet. You will not be able to set reminders. Set one up in your profile.' invalid_custom_datetime: "The date and time you provided is invalid, please try again." list_permission_denied: "You do not have permission to view this user's bookmarks." - delete_when_reminder_sent: "Delete this bookmark when the reminder notification is sent." + auto_delete_preference: + label: "Automatically delete" + never: "Never" + when_reminder_sent: "Once the reminder is sent" + on_owner_reply: "After I reply to this topic" search_placeholder: "Search bookmarks by name, topic title, or post content" search: "Search" reminders: diff --git a/db/migrate/20200715044833_add_delete_option_to_bookmarks.rb b/db/migrate/20200715044833_add_delete_option_to_bookmarks.rb new file mode 100644 index 0000000000..7b2fb0be12 --- /dev/null +++ b/db/migrate/20200715044833_add_delete_option_to_bookmarks.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class AddDeleteOptionToBookmarks < ActiveRecord::Migration[6.0] + def up + add_column :bookmarks, :auto_delete_preference, :integer, index: true, null: false, default: 0 + DB.exec("UPDATE bookmarks SET auto_delete_preference = 1 WHERE delete_when_reminder_sent") + end + + def down + remove_column :bookmarks, :auto_delete_preference + end +end diff --git a/db/post_migrate/20200715045152_remove_bookmarks_delete_when_reminder_sent.rb b/db/post_migrate/20200715045152_remove_bookmarks_delete_when_reminder_sent.rb new file mode 100644 index 0000000000..7f4db8ae8b --- /dev/null +++ b/db/post_migrate/20200715045152_remove_bookmarks_delete_when_reminder_sent.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class RemoveBookmarksDeleteWhenReminderSent < ActiveRecord::Migration[6.0] + def up + remove_column :bookmarks, :delete_when_reminder_sent + end + + def down + add_column :bookmarks, :delete_when_reminder_sent, :boolean, default: false + end +end diff --git a/lib/bookmark_manager.rb b/lib/bookmark_manager.rb index 44e6361f22..90088181f6 100644 --- a/lib/bookmark_manager.rb +++ b/lib/bookmark_manager.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class BookmarkManager - DEFAULT_OPTIONS = { delete_when_reminder_sent: false } - include HasErrors def initialize(user) @@ -24,7 +22,7 @@ class BookmarkManager reminder_type: reminder_type, reminder_at: reminder_at, reminder_set_at: Time.zone.now - }.merge(default_options(options)) + }.merge(options) ) if bookmark.errors.any? @@ -84,7 +82,7 @@ class BookmarkManager reminder_at: reminder_at, reminder_type: reminder_type, reminder_set_at: Time.zone.now - }.merge(default_options(options)) + }.merge(options) ) if bookmark.errors.any? @@ -104,8 +102,4 @@ class BookmarkManager return if reminder_type.blank? reminder_type.is_a?(Integer) ? reminder_type : Bookmark.reminder_types[reminder_type.to_sym] end - - def default_options(options) - DEFAULT_OPTIONS.merge(options) { |key, old, new| new.nil? ? old : new } - end end diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 9e9441982c..8f890d83c8 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -202,6 +202,7 @@ class PostCreator create_embedded_topic @post.link_post_uploads update_uploads_secure_status + delete_owned_bookmarks ensure_in_allowed_users if guardian.is_staff? unarchive_message if !@opts[:import_mode] DraftSequence.next!(@user, draft_key) if !@opts[:import_mode] @@ -402,6 +403,14 @@ class PostCreator @post.update_uploads_secure_status end + def delete_owned_bookmarks + return if !@post.topic_id + @user.bookmarks.where( + topic_id: @post.topic_id, + auto_delete_preference: Bookmark.auto_delete_preferences[:on_owner_reply] + ).destroy_all + end + def handle_spam if @spam GroupMessage.create(Group[:moderators].name, diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index e05fa1397a..196bcfa8c0 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -654,6 +654,19 @@ describe PostCreator do end end + context "when the user has bookmarks with auto_delete_preference on_owner_reply" do + before do + Fabricate(:bookmark, topic: topic, user: user, auto_delete_preference: Bookmark.auto_delete_preferences[:on_owner_reply]) + Fabricate(:bookmark, topic: topic, user: user, auto_delete_preference: Bookmark.auto_delete_preferences[:on_owner_reply]) + Fabricate(:bookmark, topic: topic, user: user) + Fabricate(:bookmark, user: user) + end + it "deletes the bookmarks" do + creator.create + expect(Bookmark.where(user: user).count).to eq(2) + end + end + context "topic stats" do before do PostCreator.new( diff --git a/spec/lib/bookmark_manager_spec.rb b/spec/lib/bookmark_manager_spec.rb index 9d2a05f6db..0ccd35679d 100644 --- a/spec/lib/bookmark_manager_spec.rb +++ b/spec/lib/bookmark_manager_spec.rb @@ -43,24 +43,13 @@ RSpec.describe BookmarkManager do end context "when options are provided" do - let(:options) { { delete_when_reminder_sent: true } } + let(:options) { { auto_delete_preference: Bookmark.auto_delete_preferences[:when_reminder_sent] } } it "saves any additional options successfully" do subject.create(post_id: post.id, name: name, options: options) bookmark = Bookmark.find_by(user: user) - expect(bookmark.delete_when_reminder_sent).to eq(true) - end - end - - context "when options are provided with null values" do - let(:options) { { delete_when_reminder_sent: nil } } - - it "saves defaults successfully" do - subject.create(post_id: post.id, name: name, options: options) - bookmark = Bookmark.find_by(user: user) - - expect(bookmark.delete_when_reminder_sent).to eq(false) + expect(bookmark.auto_delete_preference).to eq(1) end end @@ -192,12 +181,12 @@ RSpec.describe BookmarkManager do end context "when options are provided" do - let(:options) { { delete_when_reminder_sent: true } } + let(:options) { { auto_delete_preference: Bookmark.auto_delete_preferences[:when_reminder_sent] } } it "saves any additional options successfully" do update_bookmark bookmark.reload - expect(bookmark.delete_when_reminder_sent).to eq(true) + expect(bookmark.auto_delete_preference).to eq(1) end end diff --git a/spec/lib/bookmark_reminder_notification_handler_spec.rb b/spec/lib/bookmark_reminder_notification_handler_spec.rb index 3d9a6d77f6..2147089881 100644 --- a/spec/lib/bookmark_reminder_notification_handler_spec.rb +++ b/spec/lib/bookmark_reminder_notification_handler_spec.rb @@ -34,9 +34,9 @@ RSpec.describe BookmarkReminderNotificationHandler do expect(bookmark.reload.no_reminder?).to eq(true) end - context "when the delete_when_reminder_sent boolean is true " do + context "when the auto_delete_preference is when_reminder_sent" do it "deletes the bookmark after the reminder gets sent" do - bookmark.update(delete_when_reminder_sent: true) + bookmark.update(auto_delete_preference: Bookmark.auto_delete_preferences[:when_reminder_sent]) subject.send_notification(bookmark) expect(Bookmark.find_by(id: bookmark.id)).to eq(nil) end diff --git a/test/javascripts/acceptance/bookmarks-test.js b/test/javascripts/acceptance/bookmarks-test.js index 79b30f6ebc..b100266222 100644 --- a/test/javascripts/acceptance/bookmarks-test.js +++ b/test/javascripts/acceptance/bookmarks-test.js @@ -1,4 +1,5 @@ import I18n from "I18n"; +import selectKit from "helpers/select-kit-helper"; import { acceptance, loggedInUser, @@ -115,7 +116,8 @@ test("Opening the options panel and remembering the option", async assert => { exists(".bookmark-options-panel"), "it should open the options panel" ); - await click("#delete_when_reminder_sent"); + await selectKit(".bookmark-option-selector").expand(); + await selectKit(".bookmark-option-selector").selectRowByValue(1); await click("#save-bookmark"); await openEditBookmarkModal(); @@ -123,9 +125,11 @@ test("Opening the options panel and remembering the option", async assert => { exists(".bookmark-options-panel"), "it should reopen the options panel" ); - assert.ok( - exists(".bookmark-options-panel #delete_when_reminder_sent:checked"), - "it should pre-check delete when reminder sent option" + assert.equal( + selectKit(".bookmark-option-selector") + .header() + .value(), + 1 ); assert.verifySteps(["none"]); });