From 0e4db91870fba37e5584a9f20813f6029612d085 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Sat, 2 May 2020 10:31:44 +0200 Subject: [PATCH] FIX: save bookmark reminder on tap unless custom (#9611) --- .../discourse/app/controllers/bookmark.js | 94 ++++++++++--------- test/javascripts/acceptance/bookmarks-test.js | 87 ++++++++++++----- 2 files changed, 114 insertions(+), 67 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/bookmark.js b/app/assets/javascripts/discourse/app/controllers/bookmark.js index 30dc412e567..cd071c6bb41 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 { action } from "@ember/object"; import { isPresent } from "@ember/utils"; import Controller from "@ember/controller"; import { Promise } from "rsvp"; @@ -424,53 +425,60 @@ export default Controller.extend(ModalFunctionality, { } }, - actions: { - saveAndClose() { - if (this._saving || this._deleting) { - return; - } + @action + saveAndClose() { + if (this._saving || this._deleting) { + return; + } - this._saving = true; - this._savingBookmarkManually = true; - this._saveBookmark() - .then(() => this.send("closeModal")) - .catch(e => this._handleSaveError(e)) - .finally(() => (this._saving = false)); - }, + this._saving = true; + this._savingBookmarkManually = true; + return this._saveBookmark() + .then(() => this.send("closeModal")) + .catch(e => this._handleSaveError(e)) + .finally(() => (this._saving = false)); + }, - delete() { - this._deleting = true; - let deleteAction = () => { - this._closeWithoutSaving = true; - this._deleteBookmark() - .then(() => { - this._deleting = false; - this.send("closeModal"); - }) - .catch(e => this._handleSaveError(e)); - }; - - if (this._existingBookmarkHasReminder()) { - bootbox.confirm(I18n.t("bookmarks.confirm_delete"), result => { - if (result) { - deleteAction(); - } - }); - } else { - deleteAction(); - } - }, - - closeWithoutSavingBookmark() { + @action + delete() { + this._deleting = true; + let deleteAction = () => { this._closeWithoutSaving = true; - this.send("closeModal"); - }, + this._deleteBookmark() + .then(() => { + this._deleting = false; + this.send("closeModal"); + }) + .catch(e => this._handleSaveError(e)); + }; - selectReminderType(type) { - if (type === REMINDER_TYPES.LATER_TODAY && !this.showLaterToday) { - return; - } - this.set("selectedReminderType", type); + if (this._existingBookmarkHasReminder()) { + bootbox.confirm(I18n.t("bookmarks.confirm_delete"), result => { + if (result) { + deleteAction(); + } + }); + } else { + deleteAction(); + } + }, + + @action + closeWithoutSavingBookmark() { + this._closeWithoutSaving = true; + this.send("closeModal"); + }, + + @action + selectReminderType(type) { + if (type === REMINDER_TYPES.LATER_TODAY && !this.showLaterToday) { + return; + } + + this.set("selectedReminderType", type); + + if (type !== REMINDER_TYPES.CUSTOM) { + return this.saveAndClose(); } } }); diff --git a/test/javascripts/acceptance/bookmarks-test.js b/test/javascripts/acceptance/bookmarks-test.js index 2c2008f3e2d..6361d432614 100644 --- a/test/javascripts/acceptance/bookmarks-test.js +++ b/test/javascripts/acceptance/bookmarks-test.js @@ -1,14 +1,17 @@ import { acceptance, loggedInUser } from "helpers/qunit-helpers"; import pretender from "helpers/create-pretender"; -acceptance("Bookmarking", { - loggedIn: true, +acceptance("Bookmarking", { loggedIn: true }); - beforeEach() {} -}); +function handleRequest(assert, request) { + const body = request.requestBody; + const reminderType = body + .substr(0, body.indexOf("&")) + .replace("reminder_type=", ""); -function mockSuccessfulBookmarkPost() { - pretender.post("/bookmarks", () => [ + assert.step(reminderType || "none"); + + return [ 200, { "Content-Type": "application/json" @@ -17,15 +20,24 @@ function mockSuccessfulBookmarkPost() { id: 999, success: "OK" } - ]); + ]; +} + +function mockSuccessfulBookmarkPost(assert) { + pretender.post("/bookmarks", request => handleRequest(assert, request)); + pretender.put("/bookmarks/999", request => handleRequest(assert, request)); } async function openBookmarkModal() { - await click(".topic-post:first-child button.show-more-actions"); - return await click(".topic-post:first-child button.bookmark"); + if (exists(".topic-post:first-child button.show-more-actions")) { + await click(".topic-post:first-child button.show-more-actions"); + } + + await click(".topic-post:first-child button.bookmark"); } + async function openEditBookmarkModal() { - return await click(".topic-post:first-child button.bookmarked"); + await click(".topic-post:first-child button.bookmarked"); } test("Bookmarks modal opening", async assert => { @@ -35,32 +47,45 @@ test("Bookmarks modal opening", async assert => { }); test("Bookmarks modal selecting reminder type", async assert => { + mockSuccessfulBookmarkPost(assert); + await visit("/t/internationalization-localization/280"); + await openBookmarkModal(); await click("#tap_tile_tomorrow"); - assert.ok(exists("#tap_tile_tomorrow.active"), "it selects tomorrow"); + + await openBookmarkModal(); await click("#tap_tile_start_of_next_business_week"); - assert.ok( - exists("#tap_tile_start_of_next_business_week.active"), - "it selects next monday" - ); + + await openBookmarkModal(); await click("#tap_tile_next_week"); - assert.ok(exists("#tap_tile_next_week.active"), "it selects next week"); + + await openBookmarkModal(); await click("#tap_tile_next_month"); - assert.ok(exists("#tap_tile_next_month.active"), "it selects next month"); + + await openBookmarkModal(); await click("#tap_tile_custom"); assert.ok(exists("#tap_tile_custom.active"), "it selects custom"); assert.ok(exists(".tap-tile-date-input"), "it shows the custom date input"); assert.ok(exists(".tap-tile-time-input"), "it shows the custom time input"); + await click("#save-bookmark"); + + assert.verifySteps([ + "tomorrow", + "start_of_next_business_week", + "next_week", + "next_month", + "custom" + ]); }); test("Saving a bookmark with a reminder", async assert => { - mockSuccessfulBookmarkPost(); + mockSuccessfulBookmarkPost(assert); await visit("/t/internationalization-localization/280"); await openBookmarkModal(); await fillIn("input#bookmark-name", "Check this out later"); await click("#tap_tile_tomorrow"); - await click("#save-bookmark"); + assert.ok( exists(".topic-post:first-child button.bookmark.bookmarked"), "it shows the bookmarked icon on the post" @@ -71,13 +96,15 @@ test("Saving a bookmark with a reminder", async assert => { ), "it shows the bookmark clock icon because of the reminder" ); + assert.verifySteps(["tomorrow"]); }); test("Saving a bookmark with no reminder or name", async assert => { - mockSuccessfulBookmarkPost(); + mockSuccessfulBookmarkPost(assert); await visit("/t/internationalization-localization/280"); await openBookmarkModal(); await click("#save-bookmark"); + assert.ok( exists(".topic-post:first-child button.bookmark.bookmarked"), "it shows the bookmarked icon on the post" @@ -88,6 +115,7 @@ test("Saving a bookmark with no reminder or name", async assert => { ), "it shows the regular bookmark active icon" ); + assert.verifySteps(["none"]); }); test("Deleting a bookmark with a reminder", async assert => { @@ -101,14 +129,21 @@ test("Deleting a bookmark with a reminder", async assert => { topic_bookmarked: false } ]); - mockSuccessfulBookmarkPost(); + + mockSuccessfulBookmarkPost(assert); + await visit("/t/internationalization-localization/280"); await openBookmarkModal(); await click("#tap_tile_tomorrow"); - await click("#save-bookmark"); + + assert.verifySteps(["tomorrow"]); + await openEditBookmarkModal(); + assert.ok(exists("#bookmark-reminder-modal"), "it shows the bookmark modal"); + await click("#delete-bookmark"); + assert.ok(exists(".bootbox.modal"), "it asks for delete confirmation"); assert.ok( find(".bootbox.modal") @@ -116,7 +151,9 @@ test("Deleting a bookmark with a reminder", async assert => { .includes(I18n.t("bookmarks.confirm_delete")), "it shows delete confirmation message" ); + await click(".bootbox.modal .btn-primary"); + assert.not( exists(".topic-post:first-child button.bookmark.bookmarked"), "it no longer shows the bookmarked icon on the post after bookmark is deleted" @@ -134,14 +171,15 @@ test("Cancelling saving a bookmark", async assert => { }); test("Editing a bookmark", async assert => { - mockSuccessfulBookmarkPost(); + mockSuccessfulBookmarkPost(assert); + await visit("/t/internationalization-localization/280"); let now = moment.tz(loggedInUser().resolvedTimezone()); let tomorrow = now.add(1, "day").format("YYYY-MM-DD"); await openBookmarkModal(); await fillIn("input#bookmark-name", "Test name"); await click("#tap_tile_tomorrow"); - await click("#save-bookmark"); + await openEditBookmarkModal(); assert.equal( find("#bookmark-name").val(), @@ -158,4 +196,5 @@ test("Editing a bookmark", async assert => { "08:00", "it should prefill the bookmark time" ); + assert.verifySteps(["tomorrow"]); });