From 8f0544137a392e22addb87b49ad76b516a929ece Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 17 Apr 2020 11:08:07 +1000 Subject: [PATCH] FEATURE: Allow editing bookmark reminders (#9437) Users can now edit the bookmark name and reminder time from their list of bookmarks. We use "Custom" for the date and time in the modal because if the user set a reminder for "tomorrow" then edit the reminder "tomorrow", the definition of what "tomorrow" is has changed. --- .../components/bookmark-actions-dropdown.js | 8 + .../discourse/controllers/bookmark.js | 100 ++++++++--- .../user-activity-bookmarks-with-reminders.js | 23 ++- .../javascripts/discourse/models/bookmark.js | 8 +- .../discourse/templates/modal/bookmark.hbs | 161 +++++++++--------- .../discourse/templates/user/bookmarks.hbs | 6 +- .../common/components/bookmark-modal.scss | 10 ++ app/controllers/bookmarks_controller.rb | 18 ++ config/locales/client.en.yml | 5 + config/routes.rb | 2 +- lib/bookmark_manager.rb | 18 ++ lib/guardian/bookmark_guardian.rb | 4 + spec/lib/bookmark_manager_spec.rb | 37 ++++ test/javascripts/controllers/bookmark-test.js | 18 ++ 14 files changed, 305 insertions(+), 113 deletions(-) diff --git a/app/assets/javascripts/discourse/components/bookmark-actions-dropdown.js b/app/assets/javascripts/discourse/components/bookmark-actions-dropdown.js index 6e72fafc448..84737c49734 100644 --- a/app/assets/javascripts/discourse/components/bookmark-actions-dropdown.js +++ b/app/assets/javascripts/discourse/components/bookmark-actions-dropdown.js @@ -20,6 +20,12 @@ export default DropdownSelectBoxComponent.extend({ description: I18n.t( "post.bookmarks.actions.delete_bookmark.description" ) + }, + { + id: "edit", + icon: "pencil", + name: I18n.t("post.bookmarks.actions.edit_bookmark.name"), + description: I18n.t("post.bookmarks.actions.edit_bookmark.description") } ]; }), @@ -28,6 +34,8 @@ export default DropdownSelectBoxComponent.extend({ onChange(selectedAction) { if (selectedAction === "remove") { this.removeBookmark(this.bookmark); + } else if (selectedAction === "edit") { + this.editBookmark(this.bookmark); } } }); diff --git a/app/assets/javascripts/discourse/controllers/bookmark.js b/app/assets/javascripts/discourse/controllers/bookmark.js index 39baaa07b26..f9bf306824b 100644 --- a/app/assets/javascripts/discourse/controllers/bookmark.js +++ b/app/assets/javascripts/discourse/controllers/bookmark.js @@ -1,4 +1,5 @@ import { and } from "@ember/object/computed"; +import { isPresent } from "@ember/utils"; import { next } from "@ember/runloop"; import Controller from "@ember/controller"; import { Promise } from "rsvp"; @@ -7,7 +8,7 @@ import discourseComputed from "discourse-common/utils/decorators"; import { popupAjaxError } from "discourse/lib/ajax-error"; import { ajax } from "discourse/lib/ajax"; import KeyboardShortcuts from "discourse/lib/keyboard-shortcuts"; -import { REMINDER_TYPES } from "discourse/lib/bookmark"; +import { formattedReminderTime, REMINDER_TYPES } from "discourse/lib/bookmark"; // global shortcuts that interfere with these modal shortcuts, they are rebound when the // modal is closed @@ -47,7 +48,6 @@ const BOOKMARK_BINDINGS = { export default Controller.extend(ModalFunctionality, { loading: false, errorMessage: null, - name: null, selectedReminderType: null, closeWithoutSaving: false, isSavingBookmarkManually: false, @@ -62,7 +62,6 @@ export default Controller.extend(ModalFunctionality, { onShow() { this.setProperties({ errorMessage: null, - name: null, selectedReminderType: REMINDER_TYPES.NONE, closeWithoutSaving: false, isSavingBookmarkManually: false, @@ -76,9 +75,47 @@ export default Controller.extend(ModalFunctionality, { this.bindKeyboardShortcuts(); this.loadLastUsedCustomReminderDatetime(); - // make sure the input is cleared, otherwise the keyboard shortcut to toggle - // bookmark for post ends up in the input - next(() => this.set("name", null)); + if (this.editingExistingBookmark()) { + this.initializeExistingBookmarkData(); + } else { + // make sure the input is cleared, otherwise the keyboard shortcut to toggle + // bookmark for post ends up in the input + next(() => this.set("model.name", null)); + } + }, + + /** + * We always want to save the bookmark unless the user specifically + * clicks the save or cancel button to mimic browser behaviour. + */ + onClose() { + this.unbindKeyboardShortcuts(); + this.restoreGlobalShortcuts(); + if (!this.closeWithoutSaving && !this.isSavingBookmarkManually) { + this.saveBookmark().catch(e => this.handleSaveError(e)); + } + if (this.onCloseWithoutSaving && this.closeWithoutSaving) { + this.onCloseWithoutSaving(); + } + }, + + initializeExistingBookmarkData() { + if (this.existingBookmarkHasReminder()) { + let parsedReminderAt = this.parseCustomDateTime(this.model.reminderAt); + this.setProperties({ + customReminderDate: parsedReminderAt.format("YYYY-MM-DD"), + customReminderTime: parsedReminderAt.format("HH:mm"), + selectedReminderType: REMINDER_TYPES.CUSTOM + }); + } + }, + + editingExistingBookmark() { + return isPresent(this.model) && isPresent(this.model.id); + }, + + existingBookmarkHasReminder() { + return isPresent(this.model) && isPresent(this.model.reminderAt); }, loadLastUsedCustomReminderDatetime() { @@ -88,6 +125,7 @@ export default Controller.extend(ModalFunctionality, { if (lastTime && lastDate) { let parsed = this.parseCustomDateTime(lastDate, lastTime); + // can't set reminders in the past if (parsed < this.now()) { return; } @@ -121,21 +159,11 @@ export default Controller.extend(ModalFunctionality, { KeyboardShortcuts.unpause(GLOBAL_SHORTCUTS_TO_PAUSE); }, - // we always want to save the bookmark unless the user specifically - // clicks the save or cancel button to mimic browser behaviour - onClose() { - this.unbindKeyboardShortcuts(); - this.restoreGlobalShortcuts(); - if (!this.closeWithoutSaving && !this.isSavingBookmarkManually) { - this.saveBookmark().catch(e => this.handleSaveError(e)); - } - if (this.onCloseWithoutSaving && this.closeWithoutSaving) { - this.onCloseWithoutSaving(); - } + @discourseComputed("model.reminderAt") + showExistingReminderAt(existingReminderAt) { + return isPresent(existingReminderAt); }, - showBookmarkReminderControls: true, - @discourseComputed() showAtDesktop() { return ( @@ -177,6 +205,11 @@ export default Controller.extend(ModalFunctionality, { ); }, + @discourseComputed("model.reminderAt") + existingReminderAtFormatted(existingReminderAt) { + return formattedReminderTime(existingReminderAt, this.userTimezone); + }, + @discourseComputed() startNextBusinessWeekFormatted() { return this.nextWeek() @@ -244,19 +277,32 @@ export default Controller.extend(ModalFunctionality, { const data = { reminder_type: reminderType, reminder_at: reminderAtISO, - name: this.name, - post_id: this.model.postId + name: this.model.name, + post_id: this.model.postId, + id: this.model.id }; - return ajax("/bookmarks", { type: "POST", data }).then(() => { - if (this.afterSave) { - this.afterSave(reminderAtISO, this.selectedReminderType); - } - }); + if (this.editingExistingBookmark()) { + return ajax("/bookmarks/" + this.model.id, { + type: "PUT", + data + }).then(() => { + if (this.afterSave) { + this.afterSave(reminderAtISO, this.selectedReminderType); + } + }); + } else { + return ajax("/bookmarks", { type: "POST", data }).then(() => { + if (this.afterSave) { + this.afterSave(reminderAtISO, this.selectedReminderType); + } + }); + } }, parseCustomDateTime(date, time) { - return moment.tz(date + " " + time, this.userTimezone); + let dateTime = isPresent(time) ? date + " " + time : date; + return moment.tz(dateTime, this.userTimezone); }, defaultCustomReminderTime() { diff --git a/app/assets/javascripts/discourse/controllers/user-activity-bookmarks-with-reminders.js b/app/assets/javascripts/discourse/controllers/user-activity-bookmarks-with-reminders.js index 11cc0bb6225..a2176a7a2b9 100644 --- a/app/assets/javascripts/discourse/controllers/user-activity-bookmarks-with-reminders.js +++ b/app/assets/javascripts/discourse/controllers/user-activity-bookmarks-with-reminders.js @@ -1,4 +1,5 @@ import Controller from "@ember/controller"; +import showModal from "discourse/lib/show-modal"; import { Promise } from "rsvp"; import { inject } from "@ember/controller"; import discourseComputed from "discourse-common/utils/decorators"; @@ -35,9 +36,9 @@ export default Controller.extend({ ); }, - @discourseComputed("loaded", "content.length") - noContent(loaded, contentLength) { - return loaded && contentLength === 0; + @discourseComputed("loaded", "content.length", "noResultsHelp") + noContent(loaded, contentLength, noResultsHelp) { + return loaded && contentLength === 0 && noResultsHelp !== null; }, processLoadResponse(response) { @@ -63,6 +64,22 @@ export default Controller.extend({ return bookmark.destroy().then(() => this.loadItems()); }, + editBookmark(bookmark) { + let controller = showModal("bookmark", { + model: { + postId: bookmark.post_id, + id: bookmark.id, + reminderAt: bookmark.reminder_at, + name: bookmark.name + }, + title: "post.bookmarks.edit", + modalClass: "bookmark-with-reminder" + }); + controller.setProperties({ + afterSave: () => this.loadItems() + }); + }, + loadMore() { if (this.loadingMore) { return Promise.resolve(); diff --git a/app/assets/javascripts/discourse/models/bookmark.js b/app/assets/javascripts/discourse/models/bookmark.js index 8a8c09f35f1..44daca7cdd4 100644 --- a/app/assets/javascripts/discourse/models/bookmark.js +++ b/app/assets/javascripts/discourse/models/bookmark.js @@ -10,6 +10,7 @@ import { ajax } from "discourse/lib/ajax"; import { Promise } from "rsvp"; import RestModel from "discourse/models/rest"; import discourseComputed from "discourse-common/utils/decorators"; +import { formattedReminderTime } from "discourse/lib/bookmark"; const Bookmark = RestModel.extend({ newBookmark: none("id"), @@ -110,9 +111,10 @@ const Bookmark = RestModel.extend({ @discourseComputed("reminder_at", "currentUser") formattedReminder(bookmarkReminderAt, currentUser) { - return moment - .tz(bookmarkReminderAt, currentUser.resolvedTimezone()) - .format(I18n.t("dates.long_with_year")); + return formattedReminderTime( + bookmarkReminderAt, + currentUser.resolvedTimezone() + ).capitalize(); }, loadItems() { diff --git a/app/assets/javascripts/discourse/templates/modal/bookmark.hbs b/app/assets/javascripts/discourse/templates/modal/bookmark.hbs index 51d6c2b7344..8e20711ef39 100644 --- a/app/assets/javascripts/discourse/templates/modal/bookmark.hbs +++ b/app/assets/javascripts/discourse/templates/modal/bookmark.hbs @@ -9,89 +9,94 @@ {{/if}}
- {{input value=name name="name" class="bookmark-name" placeholder=(i18n "post.bookmarks.name_placeholder")}} + {{input value=model.name name="name" class="bookmark-name" placeholder=(i18n "post.bookmarks.name_placeholder")}}
- {{#if showBookmarkReminderControls}} -
- - - {{#if userHasTimezoneSet}} - {{#tap-tile-grid activeTile=selectedReminderType as |grid|}} - {{#if showAtDesktop}} - {{#tap-tile icon="desktop" tileId=reminderTypes.AT_DESKTOP activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.at_desktop"}}
- {{/tap-tile}} - {{/if}} - - {{#if showLaterToday}} - {{#tap-tile icon="far-moon" tileId=reminderTypes.LATER_TODAY activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.later_today"}}
-
{{laterTodayFormatted}}
- {{/tap-tile}} - {{/if}} - {{#tap-tile icon="far-sun" tileId=reminderTypes.TOMORROW activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.tomorrow"}}
-
{{tomorrowFormatted}}
- {{/tap-tile}} - {{#if showLaterThisWeek}} - {{#tap-tile icon="angle-double-right" tileId=reminderTypes.LATER_THIS_WEEK activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.later_this_week"}}
-
{{laterThisWeekFormatted}}
- {{/tap-tile}} - {{/if}} - {{#tap-tile icon="briefcase" tileId=reminderTypes.START_OF_NEXT_BUSINESS_WEEK activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.start_of_next_business_week"}}
-
{{startNextBusinessWeekFormatted}}
- {{/tap-tile}} - {{#tap-tile icon="far-clock" tileId=reminderTypes.NEXT_WEEK activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.next_week"}}
-
{{nextWeekFormatted}}
- {{/tap-tile}} - {{#tap-tile icon="far-calendar-plus" tileId=reminderTypes.NEXT_MONTH activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.next_month"}}
-
{{nextMonthFormatted}}
- {{/tap-tile}} - {{#tap-tile icon="calendar-alt" tileId=reminderTypes.CUSTOM activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.custom"}}
- {{/tap-tile}} - - {{#if customDateTimeSelected}} -
-
- {{d-icon "calendar-alt"}} - {{date-picker-future - value=customReminderDate - onSelect=(action (mut customReminderDate)) - }} -
-
- {{d-icon "far-clock"}} - {{input placeholder="--:--" type="time" class="time-input" value=customReminderTime}} -
-
- {{/if}} - - {{#if showLastCustom}} - {{#tap-tile icon="undo" tileId=reminderTypes.LAST_CUSTOM activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.last_custom"}}
-
{{lastCustomFormatted}}
- {{/tap-tile}} - {{/if}} - - {{#tap-tile icon="ban" tileId=reminderTypes.NONE activeTile=grid.activeTile onChange=(action "selectReminderType")}} -
{{i18n "bookmarks.reminders.none"}}
- {{/tap-tile}} - {{/tap-tile-grid}} - - {{else}} -
{{html-safe (i18n "bookmarks.no_timezone" basePath=basePath)}}
- {{/if}} + {{#if showExistingReminderAt }} +
+ {{d-icon "far-clock"}} + {{i18n "bookmarks.reminders.existing_reminder"}} {{existingReminderAtFormatted}}
{{/if}} +
+ + + {{#if userHasTimezoneSet}} + {{#tap-tile-grid activeTile=selectedReminderType as |grid|}} + {{#if showAtDesktop}} + {{#tap-tile icon="desktop" tileId=reminderTypes.AT_DESKTOP activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.at_desktop"}}
+ {{/tap-tile}} + {{/if}} + + {{#if showLaterToday}} + {{#tap-tile icon="far-moon" tileId=reminderTypes.LATER_TODAY activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.later_today"}}
+
{{laterTodayFormatted}}
+ {{/tap-tile}} + {{/if}} + {{#tap-tile icon="far-sun" tileId=reminderTypes.TOMORROW activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.tomorrow"}}
+
{{tomorrowFormatted}}
+ {{/tap-tile}} + {{#if showLaterThisWeek}} + {{#tap-tile icon="angle-double-right" tileId=reminderTypes.LATER_THIS_WEEK activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.later_this_week"}}
+
{{laterThisWeekFormatted}}
+ {{/tap-tile}} + {{/if}} + {{#tap-tile icon="briefcase" tileId=reminderTypes.START_OF_NEXT_BUSINESS_WEEK activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.start_of_next_business_week"}}
+
{{startNextBusinessWeekFormatted}}
+ {{/tap-tile}} + {{#tap-tile icon="far-clock" tileId=reminderTypes.NEXT_WEEK activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.next_week"}}
+
{{nextWeekFormatted}}
+ {{/tap-tile}} + {{#tap-tile icon="far-calendar-plus" tileId=reminderTypes.NEXT_MONTH activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.next_month"}}
+
{{nextMonthFormatted}}
+ {{/tap-tile}} + {{#tap-tile icon="calendar-alt" tileId=reminderTypes.CUSTOM activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.custom"}}
+ {{/tap-tile}} + + {{#if customDateTimeSelected}} +
+
+ {{d-icon "calendar-alt"}} + {{date-picker-future + value=customReminderDate + onSelect=(action (mut customReminderDate)) + }} +
+
+ {{d-icon "far-clock"}} + {{input placeholder="--:--" type="time" class="time-input" value=customReminderTime}} +
+
+ {{/if}} + + {{#if showLastCustom}} + {{#tap-tile icon="undo" tileId=reminderTypes.LAST_CUSTOM activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.last_custom"}}
+
{{lastCustomFormatted}}
+ {{/tap-tile}} + {{/if}} + + {{#tap-tile icon="ban" tileId=reminderTypes.NONE activeTile=grid.activeTile onChange=(action "selectReminderType")}} +
{{i18n "bookmarks.reminders.none"}}
+ {{/tap-tile}} + {{/tap-tile-grid}} + + {{else}} +
{{html-safe (i18n "bookmarks.no_timezone" basePath=basePath)}}
+ {{/if}} +
+
{{d-button label="bookmarks.save" class="btn-primary" action=(action "saveAndClose")}} {{d-modal-cancel close=(action "closeWithoutSavingBookmark")}} diff --git a/app/assets/javascripts/discourse/templates/user/bookmarks.hbs b/app/assets/javascripts/discourse/templates/user/bookmarks.hbs index fd6a5c5d1a1..77ecf912317 100644 --- a/app/assets/javascripts/discourse/templates/user/bookmarks.hbs +++ b/app/assets/javascripts/discourse/templates/user/bookmarks.hbs @@ -42,7 +42,11 @@ {{format-date bookmark.created_at format="tiny"}} {{raw "list/activity-column" topic=bookmark class="num" tagName="td"}} - {{bookmark-actions-dropdown bookmark=bookmark removeBookmark=(action "removeBookmark")}} + {{bookmark-actions-dropdown + bookmark=bookmark + removeBookmark=(action "removeBookmark") + editBookmark=(action "editBookmark") + }} {{/each}} diff --git a/app/assets/stylesheets/common/components/bookmark-modal.scss b/app/assets/stylesheets/common/components/bookmark-modal.scss index 2c15181cf65..45ad12fc189 100644 --- a/app/assets/stylesheets/common/components/bookmark-modal.scss +++ b/app/assets/stylesheets/common/components/bookmark-modal.scss @@ -19,6 +19,16 @@ } } + .existing-reminder-at-alert { + display: flex; + flex-direction: row; + align-items: center; + + .d-icon { + margin-right: 1em; + } + } + .custom-date-time-wrap { padding: 1em 1em 0.5em; border: 1px solid $primary-low; diff --git a/app/controllers/bookmarks_controller.rb b/app/controllers/bookmarks_controller.rb index c47dfdc0254..4fd077d45e4 100644 --- a/app/controllers/bookmarks_controller.rb +++ b/app/controllers/bookmarks_controller.rb @@ -26,4 +26,22 @@ class BookmarksController < ApplicationController BookmarkManager.new(current_user).destroy(params[:id]) render json: success_json end + + def update + params.require(:id) + + bookmark_manager = BookmarkManager.new(current_user) + bookmark_manager.update( + bookmark_id: params[:id], + name: params[:name], + reminder_type: params[:reminder_type], + reminder_at: params[:reminder_at] + ) + + if bookmark_manager.errors.empty? + return render json: success_json + end + + render json: failed_json.merge(errors: bookmark_manager.errors.full_messages), status: 400 + end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 64492a2bc7f..9bd07e68199 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -331,6 +331,7 @@ en: today_with_time: "today at %{time}" tomorrow_with_time: "tomorrow at %{time}" at_time: "at %{date_time}" + existing_reminder: "You have a reminder set for this bookmark which will be sent" drafts: resume: "Resume" @@ -2716,6 +2717,7 @@ en: bookmarks: create: "Create bookmark" + edit: "Edit bookmark" created: "Created" name: "Name" name_placeholder: "What is this bookmark for?" @@ -2724,6 +2726,9 @@ en: delete_bookmark: name: "Delete bookmark" description: "Removes the bookmark from your profile and stops all reminders for the bookmark" + edit_bookmark: + name: "Edit bookmark" + description: "Edit the bookmark name or change the reminder date and time" category: can: "can… " diff --git a/config/routes.rb b/config/routes.rb index 1a92a5440d1..2fc21b435ee 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -606,7 +606,7 @@ Discourse::Application.routes.draw do end end - resources :bookmarks, only: %i[create destroy] + resources :bookmarks, only: %i[create destroy update] resources :notifications, except: :show do collection do diff --git a/lib/bookmark_manager.rb b/lib/bookmark_manager.rb index 33c2758c125..9da2dc257b1 100644 --- a/lib/bookmark_manager.rb +++ b/lib/bookmark_manager.rb @@ -66,6 +66,24 @@ class BookmarkManager BookmarkReminderNotificationHandler.send_notification(bookmark) end + def update(bookmark_id:, name:, reminder_type:, reminder_at:) + bookmark = Bookmark.find_by(id: bookmark_id) + + raise Discourse::NotFound if bookmark.blank? + raise Discourse::InvalidAccess.new if !Guardian.new(@user).can_edit?(bookmark) + + if bookmark.errors.any? + return add_errors_from(bookmark) + end + + bookmark.update( + name: name, + reminder_at: reminder_at, + reminder_type: reminder_type, + reminder_set_at: Time.zone.now + ) + end + private def clear_at_desktop_cache_if_required diff --git a/lib/guardian/bookmark_guardian.rb b/lib/guardian/bookmark_guardian.rb index 1754c280e23..7f0b63c6209 100644 --- a/lib/guardian/bookmark_guardian.rb +++ b/lib/guardian/bookmark_guardian.rb @@ -5,6 +5,10 @@ module BookmarkGuardian @user == bookmark.user end + def can_edit_bookmark?(bookmark) + @user == bookmark.user + end + def can_create_bookmark?(bookmark) can_see_topic?(bookmark.topic) end diff --git a/spec/lib/bookmark_manager_spec.rb b/spec/lib/bookmark_manager_spec.rb index 2130e50610b..de25969f2cb 100644 --- a/spec/lib/bookmark_manager_spec.rb +++ b/spec/lib/bookmark_manager_spec.rb @@ -160,6 +160,43 @@ RSpec.describe BookmarkManager do end end + describe ".update" do + let!(:bookmark) { Fabricate(:bookmark_next_business_day_reminder, user: user, post: post, name: "Old name") } + let(:new_name) { "Some new name" } + let(:new_reminder_at) { 10.days.from_now } + let(:new_reminder_type) { Bookmark.reminder_types[:custom] } + + def update_bookmark + subject.update( + bookmark_id: bookmark.id, name: new_name, reminder_type: new_reminder_type, reminder_at: new_reminder_at + ) + end + + it "saves the time and new reminder type sucessfully" do + update_bookmark + bookmark.reload + expect(bookmark.name).to eq(new_name) + expect(bookmark.reminder_at).to eq_time(new_reminder_at) + expect(bookmark.reminder_type).to eq(new_reminder_type) + end + + context "if the bookmark is belonging to some other user" do + let!(:bookmark) { Fabricate(:bookmark, user: Fabricate(:admin), post: post) } + it "raises an invalid access error" do + expect { update_bookmark }.to raise_error(Discourse::InvalidAccess) + end + end + + context "if the bookmark no longer exists" do + before do + bookmark.destroy! + end + it "raises an invalid access error" do + expect { update_bookmark }.to raise_error(Discourse::NotFound) + end + end + end + describe ".destroy_for_topic" do let!(:topic) { Fabricate(:topic) } let!(:bookmark1) { Fabricate(:bookmark, topic: topic, post: Fabricate(:post, topic: topic), user: user) } diff --git a/test/javascripts/controllers/bookmark-test.js b/test/javascripts/controllers/bookmark-test.js index 756e91d672c..7a687392895 100644 --- a/test/javascripts/controllers/bookmark-test.js +++ b/test/javascripts/controllers/bookmark-test.js @@ -1,6 +1,7 @@ import { logIn } from "helpers/qunit-helpers"; import User from "discourse/models/user"; import KeyboardShortcutInitializer from "discourse/initializers/keyboard-shortcuts"; +import { REMINDER_TYPES } from "discourse/lib/bookmark"; let BookmarkController; moduleFor("controller:bookmark", { @@ -263,3 +264,20 @@ QUnit.test("user timezone updates when the modal is shown", function(assert) { ); stub.restore(); }); + +QUnit.test( + "opening the modal with an existing bookmark with reminder at prefills the custom reminder type", + function(assert) { + let name = "test"; + let reminderAt = "2020-05-15T09:45:00"; + BookmarkController.model = { id: 1, name: name, reminderAt: reminderAt }; + BookmarkController.onShow(); + assert.equal( + BookmarkController.selectedReminderType, + REMINDER_TYPES.CUSTOM + ); + assert.equal(BookmarkController.customReminderDate, "2020-05-15"); + assert.equal(BookmarkController.customReminderTime, "09:45"); + assert.equal(BookmarkController.model.name, name); + } +);