diff --git a/app/assets/javascripts/discourse/app/controllers/bookmark.js b/app/assets/javascripts/discourse/app/controllers/bookmark.js index 992c3228a02..7d86ed2b74a 100644 --- a/app/assets/javascripts/discourse/app/controllers/bookmark.js +++ b/app/assets/javascripts/discourse/app/controllers/bookmark.js @@ -1,4 +1,5 @@ import { and } from "@ember/object/computed"; +import { next } from "@ember/runloop"; import { action } from "@ember/object"; import { isPresent } from "@ember/utils"; import Controller from "@ember/controller"; @@ -59,6 +60,8 @@ export default Controller.extend(ModalFunctionality, { lastCustomReminderTime: null, mouseTrap: null, userTimezone: null, + showOptions: false, + options: null, onShow() { this.setProperties({ @@ -70,9 +73,13 @@ export default Controller.extend(ModalFunctionality, { customReminderTime: this._defaultCustomReminderTime(), lastCustomReminderDate: null, lastCustomReminderTime: null, - userTimezone: this.currentUser.resolvedTimezone() + userTimezone: this.currentUser.resolvedTimezone(), + showOptions: false, + options: {}, + model: this.model || {} }); + this._loadBookmarkOptions(); this._bindKeyboardShortcuts(); this._loadLastUsedCustomReminderDatetime(); @@ -124,6 +131,21 @@ export default Controller.extend(ModalFunctionality, { return isPresent(this.model) && isPresent(this.model.reminderAt); }, + _loadBookmarkOptions() { + this.set( + "options.deleteWhenReminderSent", + this.model.deleteWhenReminderSent || + localStorage.bookmarkOptionsDeleteWhenReminderSent === "true" + ); + + // 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) { + next(() => this.toggleOptionsPanel()); + } + }, + _loadLastUsedCustomReminderDatetime() { let lastTime = localStorage.lastCustomBookmarkReminderTime; let lastDate = localStorage.lastCustomBookmarkReminderDate; @@ -268,6 +290,8 @@ export default Controller.extend(ModalFunctionality, { localStorage.lastCustomBookmarkReminderDate = this.customReminderDate; } + localStorage.bookmarkOptionsDeleteWhenReminderSent = this.options.deleteWhenReminderSent; + let reminderType; if (this.selectedReminderType === REMINDER_TYPES.NONE) { reminderType = null; @@ -282,7 +306,8 @@ export default Controller.extend(ModalFunctionality, { reminder_at: reminderAtISO, name: this.model.name, post_id: this.model.postId, - id: this.model.id + id: this.model.id, + delete_when_reminder_sent: this.options.deleteWhenReminderSent }; if (this._editingExistingBookmark()) { @@ -294,6 +319,7 @@ export default Controller.extend(ModalFunctionality, { this.afterSave({ reminderAt: reminderAtISO, reminderType: this.selectedReminderType, + deleteWhenReminderSent: this.options.deleteWhenReminderSent, id: this.model.id, name: this.model.name }); @@ -305,6 +331,7 @@ export default Controller.extend(ModalFunctionality, { this.afterSave({ reminderAt: reminderAtISO, reminderType: this.selectedReminderType, + deleteWhenReminderSent: this.options.deleteWhenReminderSent, id: response.id, name: this.model.name }); @@ -420,6 +447,16 @@ export default Controller.extend(ModalFunctionality, { } }, + @action + toggleOptionsPanel() { + if (this.showOptions) { + $(".bookmark-options-panel").slideUp("fast"); + } else { + $(".bookmark-options-panel").slideDown("fast"); + } + this.toggleProperty("showOptions"); + }, + @action saveAndClose() { if (this._saving || this._deleting) { diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js index e7cf6028e57..e0565b420c0 100644 --- a/app/assets/javascripts/discourse/app/models/post.js +++ b/app/assets/javascripts/discourse/app/models/post.js @@ -316,6 +316,7 @@ const Post = RestModel.extend({ postId: this.id, id: this.bookmark_id, reminderAt: this.bookmark_reminder_at, + deleteWhenReminderSent: this.bookmark_delete_when_reminder_sent, name: this.bookmark_name }, title: this.bookmark_id @@ -334,6 +335,8 @@ const Post = RestModel.extend({ bookmarked: true, bookmark_reminder_at: savedData.reminderAt, bookmark_reminder_type: savedData.reminderType, + bookmark_delete_when_reminder_sent: + savedData.deleteWhenReminderSent, bookmark_name: savedData.name, bookmark_id: savedData.id }); diff --git a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs index 2cac81e9b65..b69713e7570 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs @@ -8,8 +8,16 @@ {{/if}} -
+
{{input id="bookmark-name" value=model.name name="name" class="bookmark-name" placeholder=(i18n "post.bookmarks.name_placeholder")}} + {{d-button icon="gear" action=(action "toggleOptionsPanel") class="bookmark-options-button"}} +
+ +
+
{{#if showExistingReminderAt }} diff --git a/app/assets/stylesheets/common/components/bookmark-modal.scss b/app/assets/stylesheets/common/components/bookmark-modal.scss index 45ad12fc189..b870ec00d1e 100644 --- a/app/assets/stylesheets/common/components/bookmark-modal.scss +++ b/app/assets/stylesheets/common/components/bookmark-modal.scss @@ -62,4 +62,24 @@ flex: 1 1 auto; } } + + .bookmark-name-wrap { + display: inline-flex; + width: 100%; + align-items: end; + } + + .bookmark-options-button { + margin-left: 0.5em; + background: transparent; + } + + .bookmark-options-panel { + margin-bottom: 18px; + display: none; + + input[type="checkbox"] { + margin-right: 0.85em; + } + } } diff --git a/app/controllers/bookmarks_controller.rb b/app/controllers/bookmarks_controller.rb index f7fed51f59b..001e75c119b 100644 --- a/app/controllers/bookmarks_controller.rb +++ b/app/controllers/bookmarks_controller.rb @@ -11,7 +11,10 @@ class BookmarksController < ApplicationController post_id: params[:post_id], name: params[:name], reminder_type: params[:reminder_type], - reminder_at: params[:reminder_at] + reminder_at: params[:reminder_at], + options: { + delete_when_reminder_sent: params[:delete_when_reminder_sent] + } ) if bookmark_manager.errors.empty? @@ -35,7 +38,10 @@ class BookmarksController < ApplicationController bookmark_id: params[:id], name: params[:name], reminder_type: params[:reminder_type], - reminder_at: params[:reminder_at] + reminder_at: params[:reminder_at], + options: { + delete_when_reminder_sent: params[:delete_when_reminder_sent] + } ) if bookmark_manager.errors.empty? diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb index 3cfae4fe174..3125f8f9fed 100644 --- a/app/models/bookmark.rb +++ b/app/models/bookmark.rb @@ -79,17 +79,18 @@ end # # Table name: bookmarks # -# id :bigint not null, primary key -# user_id :bigint not null -# topic_id :bigint not null -# post_id :bigint not null -# name :string -# reminder_type :integer -# reminder_at :datetime -# created_at :datetime not null -# updated_at :datetime not null -# reminder_last_sent_at :datetime -# reminder_set_at :datetime +# id :bigint not null, primary key +# user_id :bigint not null +# topic_id :bigint not null +# post_id :bigint not null +# name :string +# reminder_type :integer +# reminder_at :datetime +# created_at :datetime not null +# updated_at :datetime not null +# reminder_last_sent_at :datetime +# reminder_set_at :datetime +# delete_when_reminder_sent :boolean default(FALSE) # # Indexes # diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index ef39e37caab..35a50a5f96f 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -53,6 +53,7 @@ class PostSerializer < BasicPostSerializer :bookmark_id, :bookmark_reminder_type, :bookmark_name, + :bookmark_delete_when_reminder_sent, :raw, :actions_summary, :moderator?, @@ -331,6 +332,10 @@ class PostSerializer < BasicPostSerializer include_bookmarked? end + def include_bookmark_delete_when_reminder_sent? + include_bookmarked? + end + def include_bookmark_id? include_bookmarked? end @@ -353,6 +358,10 @@ class PostSerializer < BasicPostSerializer post_bookmark&.name end + def bookmark_delete_when_reminder_sent + post_bookmark&.delete_when_reminder_sent + end + def bookmark_id post_bookmark&.id end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b4b6ad31f2a..e4a65a3b1e3 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -319,6 +319,7 @@ 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." reminders: at_desktop: "Next time I'm at my desktop" later_today: "Later today" diff --git a/db/migrate/20200505060712_add_delete_when_reminder_sent_boolean_to_bookmarks.rb b/db/migrate/20200505060712_add_delete_when_reminder_sent_boolean_to_bookmarks.rb new file mode 100644 index 00000000000..7e4ca807fa3 --- /dev/null +++ b/db/migrate/20200505060712_add_delete_when_reminder_sent_boolean_to_bookmarks.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddDeleteWhenReminderSentBooleanToBookmarks < ActiveRecord::Migration[6.0] + def change + add_column :bookmarks, :delete_when_reminder_sent, :boolean, nullabe: false, default: false + end +end diff --git a/lib/bookmark_manager.rb b/lib/bookmark_manager.rb index 23328021f6a..90088181f63 100644 --- a/lib/bookmark_manager.rb +++ b/lib/bookmark_manager.rb @@ -7,20 +7,22 @@ class BookmarkManager @user = user end - def create(post_id:, name: nil, reminder_type: nil, reminder_at: nil) + def create(post_id:, name: nil, reminder_type: nil, reminder_at: nil, options: {}) post = Post.unscoped.includes(:topic).find(post_id) reminder_type = parse_reminder_type(reminder_type) raise Discourse::InvalidAccess.new if !Guardian.new(@user).can_see_post?(post) bookmark = Bookmark.create( - user_id: @user.id, - topic: post.topic, - post: post, - name: name, - reminder_type: reminder_type, - reminder_at: reminder_at, - reminder_set_at: Time.zone.now + { + user_id: @user.id, + topic: post.topic, + post: post, + name: name, + reminder_type: reminder_type, + reminder_at: reminder_at, + reminder_set_at: Time.zone.now + }.merge(options) ) if bookmark.errors.any? @@ -66,7 +68,7 @@ class BookmarkManager BookmarkReminderNotificationHandler.send_notification(bookmark) end - def update(bookmark_id:, name:, reminder_type:, reminder_at:) + def update(bookmark_id:, name:, reminder_type:, reminder_at:, options: {}) bookmark = Bookmark.find_by(id: bookmark_id) raise Discourse::NotFound if bookmark.blank? @@ -75,10 +77,12 @@ class BookmarkManager reminder_type = parse_reminder_type(reminder_type) success = bookmark.update( - name: name, - reminder_at: reminder_at, - reminder_type: reminder_type, - reminder_set_at: Time.zone.now + { + name: name, + reminder_at: reminder_at, + reminder_type: reminder_type, + reminder_set_at: Time.zone.now + }.merge(options) ) if bookmark.errors.any? diff --git a/lib/bookmark_reminder_notification_handler.rb b/lib/bookmark_reminder_notification_handler.rb index 223c55b2ac5..b1f32cee618 100644 --- a/lib/bookmark_reminder_notification_handler.rb +++ b/lib/bookmark_reminder_notification_handler.rb @@ -9,6 +9,11 @@ class BookmarkReminderNotificationHandler end create_notification(bookmark) + + if bookmark.delete_when_reminder_sent? + return bookmark.destroy + end + clear_reminder(bookmark) end end diff --git a/spec/lib/bookmark_manager_spec.rb b/spec/lib/bookmark_manager_spec.rb index d821e994465..24bdd6bd15c 100644 --- a/spec/lib/bookmark_manager_spec.rb +++ b/spec/lib/bookmark_manager_spec.rb @@ -42,6 +42,17 @@ RSpec.describe BookmarkManager do end end + context "when options are provided" do + let(:options) { { delete_when_reminder_sent: true } } + + 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 the bookmark already exists for the user & post" do before do Bookmark.create(post: post, user: user, topic: post.topic) @@ -142,14 +153,19 @@ RSpec.describe BookmarkManager do let(:new_name) { "Some new name" } let(:new_reminder_at) { 10.days.from_now } let(:new_reminder_type) { Bookmark.reminder_types[:custom] } + let(:options) { {} } def update_bookmark subject.update( - bookmark_id: bookmark.id, name: new_name, reminder_type: new_reminder_type, reminder_at: new_reminder_at + bookmark_id: bookmark.id, + name: new_name, + reminder_type: new_reminder_type, + reminder_at: new_reminder_at, + options: options ) end - it "saves the time and new reminder type sucessfully" do + it "saves the time and new reminder type and new name sucessfully" do update_bookmark bookmark.reload expect(bookmark.name).to eq(new_name) @@ -157,6 +173,16 @@ RSpec.describe BookmarkManager do expect(bookmark.reminder_type).to eq(new_reminder_type) end + context "when options are provided" do + let(:options) { { delete_when_reminder_sent: true } } + + it "saves any additional options successfully" do + update_bookmark + bookmark.reload + expect(bookmark.delete_when_reminder_sent).to eq(true) + end + end + context "if the new reminder type is a string" do let(:new_reminder_type) { "custom" } it "is parsed" do diff --git a/spec/lib/bookmark_reminder_notification_handler_spec.rb b/spec/lib/bookmark_reminder_notification_handler_spec.rb index d86ce73a5ed..6af860f2f04 100644 --- a/spec/lib/bookmark_reminder_notification_handler_spec.rb +++ b/spec/lib/bookmark_reminder_notification_handler_spec.rb @@ -28,6 +28,20 @@ RSpec.describe BookmarkReminderNotificationHandler do expect(data["bookmark_name"]).to eq(bookmark.name) end + it "clears the reminder" do + subject.send_notification(bookmark) + bookmark.reload + expect(bookmark.reload.no_reminder?).to eq(true) + end + + context "when the delete_when_reminder_sent boolean is true " do + it "deletes the bookmark after the reminder gets sent" do + bookmark.update(delete_when_reminder_sent: true) + subject.send_notification(bookmark) + expect(Bookmark.find_by(id: bookmark.id)).to eq(nil) + end + end + context "when the post has been deleted" do it "clears the reminder and does not send a notification" do bookmark.post.trash! diff --git a/test/javascripts/acceptance/bookmarks-test.js b/test/javascripts/acceptance/bookmarks-test.js index df8fb56e931..762cd1ee071 100644 --- a/test/javascripts/acceptance/bookmarks-test.js +++ b/test/javascripts/acceptance/bookmarks-test.js @@ -106,6 +106,30 @@ test("Saving a bookmark with a reminder", async assert => { assert.verifySteps(["tomorrow"]); }); +test("Opening the options panel and remembering the option", async assert => { + mockSuccessfulBookmarkPost(assert); + await visit("/t/internationalization-localization/280"); + await openBookmarkModal(); + await click(".bookmark-options-button"); + assert.ok( + exists(".bookmark-options-panel"), + "it should open the options panel" + ); + await click("#delete_when_reminder_sent"); + await click("#save-bookmark"); + await openEditBookmarkModal(); + + assert.ok( + 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.verifySteps(["none"]); +}); + test("Saving a bookmark with no reminder or name", async assert => { mockSuccessfulBookmarkPost(assert); await visit("/t/internationalization-localization/280");