From e1eb5fb9b3d487c1b24e95eed38cb917a8887245 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 12 Mar 2020 15:20:56 +1000 Subject: [PATCH] FEATURE: MVP Bookmarks with reminders user list changes (#8999) * This PR changes the user activity bookmarks stream to show a new list of bookmarks based on the Bookmark record. * If a bookmark has a name or reminder it will be shown as metadata above the topic title in the list * The categories, tags, topic status, and assigned show for each bookmarked post based on the post topic * Bookmarks can be deleted from the [...] menu in the list * As well as this, the list of bookmarks from the quick access panel is now drawn from the Bookmarks table for a user: * All of this new functionality is gated behind the enable_bookmarks_with_reminders site setting The /bookmarks/ route now redirects directly to /user/:username/activity/bookmarks-with-reminders * The structure of the Ember for the list of bookmarks is not ideal, this is an MVP PR so we can start testing this functionality internally. There is a little repeated code from topic.js.es6. There is an ongoing effort to start standardizing these lists that will be addressed in future PRs. * This PR also fixes issues with feature detection for at_desktop bookmark reminders --- .../bookmark-actions-dropdown.js.es6 | 33 ++++++ .../discourse/controllers/bookmark.js.es6 | 2 +- ...r-activity-bookmarks-with-reminders.js.es6 | 54 +++++++++ .../controllers/user-activity.js.es6 | 2 + .../discourse/models/bookmark.js.es6 | 109 ++++++++++++++++++ .../javascripts/discourse/models/topic.js.es6 | 9 +- .../javascripts/discourse/models/user.js.es6 | 6 + .../discourse/routes/app-route-map.js.es6 | 3 + .../discourse/routes/discovery.js.es6 | 19 ++- ...r-activity-bookmarks-with-reminders.js.es6 | 29 +++++ .../discourse/templates/user/activity.hbs | 12 +- .../discourse/templates/user/bookmarks.hbs | 51 ++++++++ .../discourse/widgets/post-menu.js.es6 | 2 + .../widgets/quick-access-bookmarks.js.es6 | 59 ++++++++-- .../stylesheets/common/base/_topic-list.scss | 8 ++ .../stylesheets/common/base/tagging.scss | 10 +- .../common/components/bookmark-list-item.scss | 19 +++ app/controllers/users_controller.rb | 19 ++- app/serializers/topic_view_serializer.rb | 6 +- app/serializers/user_bookmark_serializer.rb | 91 +++++++++++++++ config/locales/client.en.yml | 4 + config/routes.rb | 1 + lib/bookmark_query.rb | 32 +++++ lib/bookmark_reminder_notification_handler.rb | 2 +- lib/topic_view.rb | 5 + ...mark_reminder_notification_handler_spec.rb | 71 ++++++------ 26 files changed, 597 insertions(+), 61 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/bookmark-actions-dropdown.js.es6 create mode 100644 app/assets/javascripts/discourse/controllers/user-activity-bookmarks-with-reminders.js.es6 create mode 100644 app/assets/javascripts/discourse/routes/user-activity-bookmarks-with-reminders.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/user/bookmarks.hbs create mode 100644 app/assets/stylesheets/common/components/bookmark-list-item.scss create mode 100644 app/serializers/user_bookmark_serializer.rb create mode 100644 lib/bookmark_query.rb diff --git a/app/assets/javascripts/discourse/components/bookmark-actions-dropdown.js.es6 b/app/assets/javascripts/discourse/components/bookmark-actions-dropdown.js.es6 new file mode 100644 index 00000000000..6e72fafc448 --- /dev/null +++ b/app/assets/javascripts/discourse/components/bookmark-actions-dropdown.js.es6 @@ -0,0 +1,33 @@ +import { computed } from "@ember/object"; +import DropdownSelectBoxComponent from "select-kit/components/dropdown-select-box"; +import { action } from "@ember/object"; + +export default DropdownSelectBoxComponent.extend({ + classNames: ["bookmark-actions-dropdown"], + pluginApiIdentifiers: ["bookmark-actions-dropdown"], + selectKitOptions: { + icon: null, + translatedNone: "...", + showFullTitle: true + }, + + content: computed(() => { + return [ + { + id: "remove", + icon: "trash-alt", + name: I18n.t("post.bookmarks.actions.delete_bookmark.name"), + description: I18n.t( + "post.bookmarks.actions.delete_bookmark.description" + ) + } + ]; + }), + + @action + onChange(selectedAction) { + if (selectedAction === "remove") { + this.removeBookmark(this.bookmark); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/bookmark.js.es6 b/app/assets/javascripts/discourse/controllers/bookmark.js.es6 index 109678a7b2c..b4b74ec57ef 100644 --- a/app/assets/javascripts/discourse/controllers/bookmark.js.es6 +++ b/app/assets/javascripts/discourse/controllers/bookmark.js.es6 @@ -124,7 +124,7 @@ export default Controller.extend(ModalFunctionality, { const reminderAt = this.reminderAt(); const reminderAtISO = reminderAt ? reminderAt.toISOString() : null; - if (!reminderAt) { + if (!reminderAt && this.selectedReminderType === REMINDER_TYPES.CUSTOM) { return Promise.reject(I18n.t("bookmarks.invalid_custom_datetime")); } diff --git a/app/assets/javascripts/discourse/controllers/user-activity-bookmarks-with-reminders.js.es6 b/app/assets/javascripts/discourse/controllers/user-activity-bookmarks-with-reminders.js.es6 new file mode 100644 index 00000000000..49982b12449 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/user-activity-bookmarks-with-reminders.js.es6 @@ -0,0 +1,54 @@ +import Controller from "@ember/controller"; +import { inject } from "@ember/controller"; +import discourseComputed from "discourse-common/utils/decorators"; +import Bookmark from "discourse/models/bookmark"; + +export default Controller.extend({ + application: inject(), + user: inject(), + + content: null, + loading: false, + noResultsHelp: null, + + loadItems() { + this.setProperties({ + content: [], + loading: true, + noResultsHelp: null + }); + + return this.model + .loadItems() + .then(response => { + if (response && response.no_results_help) { + this.set("noResultsHelp", response.no_results_help); + } + + if (response && response.bookmarks) { + let bookmarks = []; + response.bookmarks.forEach(bookmark => { + bookmarks.push(Bookmark.create(bookmark)); + }); + this.content.pushObjects(bookmarks); + } + }) + .finally(() => + this.setProperties({ + loaded: true, + loading: false + }) + ); + }, + + @discourseComputed("loaded", "content.length") + noContent(loaded, content) { + return loaded && content.length === 0; + }, + + actions: { + removeBookmark(bookmark) { + return bookmark.destroy().then(() => this.loadItems()); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/user-activity.js.es6 b/app/assets/javascripts/discourse/controllers/user-activity.js.es6 index 12333fcf074..03828c439b2 100644 --- a/app/assets/javascripts/discourse/controllers/user-activity.js.es6 +++ b/app/assets/javascripts/discourse/controllers/user-activity.js.es6 @@ -3,6 +3,7 @@ import { inject as service } from "@ember/service"; import Controller, { inject as controller } from "@ember/controller"; import { exportUserArchive } from "discourse/lib/export-csv"; import { observes } from "discourse-common/utils/decorators"; +import { setting } from "discourse/lib/computed"; export default Controller.extend({ application: controller(), @@ -11,6 +12,7 @@ export default Controller.extend({ userActionType: null, canDownloadPosts: alias("user.viewingSelf"), + bookmarksWithRemindersEnabled: setting("enable_bookmarks_with_reminders"), @observes("userActionType", "model.stream.itemsLoaded") _showFooter: function() { diff --git a/app/assets/javascripts/discourse/models/bookmark.js.es6 b/app/assets/javascripts/discourse/models/bookmark.js.es6 index 2ae4e63e4cd..0473c108118 100644 --- a/app/assets/javascripts/discourse/models/bookmark.js.es6 +++ b/app/assets/javascripts/discourse/models/bookmark.js.es6 @@ -1,8 +1,16 @@ +import Category from "discourse/models/category"; +import { isRTL } from "discourse/lib/text-direction"; +import { censor } from "pretty-text/censored-words"; +import { emojiUnescape } from "discourse/lib/text"; +import Site from "discourse/models/site"; +import { longDate } from "discourse/lib/formatter"; +import PreloadStore from "preload-store"; import { none } from "@ember/object/computed"; import { computed } from "@ember/object"; import { ajax } from "discourse/lib/ajax"; import { Promise } from "rsvp"; import RestModel from "discourse/models/rest"; +import discourseComputed from "discourse-common/utils/decorators"; const Bookmark = RestModel.extend({ newBookmark: none("id"), @@ -18,6 +26,107 @@ const Bookmark = RestModel.extend({ return ajax(this.url, { type: "DELETE" }); + }, + + @discourseComputed("highest_post_number", "url") + lastPostUrl(highestPostNumber) { + return this.urlForPostNumber(highestPostNumber); + }, + + // Helper to build a Url with a post number + urlForPostNumber(postNumber) { + let url = Discourse.getURL(`/t/${this.topic_id}`); + if (postNumber > 0) { + url += `/${postNumber}`; + } + return url; + }, + + // returns createdAt if there's no bumped date + @discourseComputed("bumped_at", "createdAt") + bumpedAt(bumped_at, createdAt) { + if (bumped_at) { + return new Date(bumped_at); + } else { + return createdAt; + } + }, + + @discourseComputed("bumpedAt", "createdAt") + bumpedAtTitle(bumpedAt, createdAt) { + const firstPost = I18n.t("first_post"); + const lastPost = I18n.t("last_post"); + const createdAtDate = longDate(createdAt); + const bumpedAtDate = longDate(bumpedAt); + + return I18n.messageFormat("topic.bumped_at_title_MF", { + FIRST_POST: firstPost, + CREATED_AT: createdAtDate, + LAST_POST: lastPost, + BUMPED_AT: bumpedAtDate + }); + }, + + @discourseComputed("title") + fancyTitle(title) { + let fancyTitle = censor( + emojiUnescape(title) || "", + Site.currentProp("censored_regexp") + ); + + if (this.siteSettings.support_mixed_text_direction) { + const titleDir = isRTL(title) ? "rtl" : "ltr"; + return `${fancyTitle}`; + } + return fancyTitle; + }, + + @discourseComputed("created_at") + createdAt(created_at) { + return new Date(created_at); + }, + + @discourseComputed("tags") + visibleListTags(tags) { + if (!tags || !this.siteSettings.suppress_overlapping_tags_in_list) { + return tags; + } + + const title = this.title; + const newTags = []; + + tags.forEach(function(tag) { + if (title.toLowerCase().indexOf(tag) === -1) { + newTags.push(tag); + } + }); + + return newTags; + }, + + @discourseComputed("category_id") + category(categoryId) { + return Category.findById(categoryId); + }, + + @discourseComputed("reminder_at") + formattedReminder(bookmarkReminderAt) { + const currentUser = PreloadStore.get("currentUser"); + return moment + .tz(bookmarkReminderAt, currentUser.timezone || moment.tz.guess()) + .format(I18n.t("dates.long_with_year")); + }, + + loadItems() { + return ajax(`/u/${this.user.username}/bookmarks.json`, { cache: "false" }); + } +}); + +Bookmark.reopenClass({ + create(args) { + args = args || {}; + args.siteSettings = args.siteSettings || Discourse.SiteSettings; + return this._super(args); } }); diff --git a/app/assets/javascripts/discourse/models/topic.js.es6 b/app/assets/javascripts/discourse/models/topic.js.es6 index fa97e11ea77..a3c9ee26c66 100644 --- a/app/assets/javascripts/discourse/models/topic.js.es6 +++ b/app/assets/javascripts/discourse/models/topic.js.es6 @@ -136,7 +136,12 @@ const Topic = RestModel.extend({ const createdAtDate = longDate(createdAt); const bumpedAtDate = longDate(bumpedAt); - return `${firstPost}: ${createdAtDate}\n${lastPost}: ${bumpedAtDate}`; + return I18n.messageFormat("topic.bumped_at_title_MF", { + FIRST_POST: firstPost, + CREATED_AT: createdAtDate, + LAST_POST: lastPost, + BUMPED_AT: bumpedAtDate + }); }, @discourseComputed("created_at") @@ -259,7 +264,7 @@ const Topic = RestModel.extend({ // Helper to build a Url with a post number urlForPostNumber(postNumber) { let url = this.url; - if (postNumber && postNumber > 0) { + if (postNumber > 0) { url += `/${postNumber}`; } return url; diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 7af461aa309..026b27926a0 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -5,6 +5,7 @@ import EmberObject, { computed, getProperties } from "@ember/object"; import { ajax } from "discourse/lib/ajax"; import { url } from "discourse/lib/computed"; import RestModel from "discourse/models/rest"; +import Bookmark from "discourse/models/bookmark"; import UserStream from "discourse/models/user-stream"; import UserPostsStream from "discourse/models/user-posts-stream"; import Singleton from "discourse/mixins/singleton"; @@ -52,6 +53,11 @@ const User = RestModel.extend({ return UserStream.create({ user: this }); }, + @discourseComputed() + bookmarks() { + return Bookmark.create({ user: this }); + }, + @discourseComputed() postsStream() { return UserPostsStream.create({ user: this }); diff --git a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 index 16720a568ef..fb13822bb26 100644 --- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 +++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 @@ -122,6 +122,9 @@ export default function() { this.route("replies"); this.route("likesGiven", { path: "likes-given" }); this.route("bookmarks"); + this.route("bookmarksWithReminders", { + path: "bookmarks-with-reminders" + }); this.route("pending"); this.route("drafts"); } diff --git a/app/assets/javascripts/discourse/routes/discovery.js.es6 b/app/assets/javascripts/discourse/routes/discovery.js.es6 index d8129506668..1dd2985c46e 100644 --- a/app/assets/javascripts/discourse/routes/discovery.js.es6 +++ b/app/assets/javascripts/discourse/routes/discovery.js.es6 @@ -13,16 +13,27 @@ export default DiscourseRoute.extend(OpenComposer, { }, beforeModel(transition) { - const user = User; + // the new bookmark list is radically different to this topic-based one, + // including being able to show links to multiple posts to the same topic + // and being based on a different model. better to just redirect const url = transition.intent.url; + if ( + this.siteSettings.enable_bookmarks_with_reminders && + url === "/bookmarks" + ) { + this.transitionTo( + "userActivity.bookmarksWithReminders", + this.currentUser + ); + } if ( (url === "/" || url === "/latest" || url === "/categories") && transition.targetName.indexOf("discovery.top") === -1 && - user.currentProp("should_be_redirected_to_top") + User.currentProp("should_be_redirected_to_top") ) { - user.currentProp("should_be_redirected_to_top", false); - const period = user.currentProp("redirected_to_top.period") || "all"; + User.currentProp("should_be_redirected_to_top", false); + const period = User.currentProp("redirected_to_top.period") || "all"; this.replaceWith(`discovery.top${period.capitalize()}`); } }, diff --git a/app/assets/javascripts/discourse/routes/user-activity-bookmarks-with-reminders.js.es6 b/app/assets/javascripts/discourse/routes/user-activity-bookmarks-with-reminders.js.es6 new file mode 100644 index 00000000000..fc8bd078264 --- /dev/null +++ b/app/assets/javascripts/discourse/routes/user-activity-bookmarks-with-reminders.js.es6 @@ -0,0 +1,29 @@ +import DiscourseRoute from "discourse/routes/discourse"; + +export default DiscourseRoute.extend({ + noContentHelpKey: "user_activity.no_bookmarks", + + queryParams: { + acting_username: { refreshModel: true } + }, + + model() { + return this.modelFor("user").get("bookmarks"); + }, + + renderTemplate() { + this.render("user_bookmarks"); + }, + + setupController(controller, model) { + controller.set("model", model); + controller.loadItems(); + }, + + actions: { + didTransition() { + this.controllerFor("user-activity")._showFooter(); + return true; + } + } +}); diff --git a/app/assets/javascripts/discourse/templates/user/activity.hbs b/app/assets/javascripts/discourse/templates/user/activity.hbs index 6c39cc46181..2623e57c050 100644 --- a/app/assets/javascripts/discourse/templates/user/activity.hbs +++ b/app/assets/javascripts/discourse/templates/user/activity.hbs @@ -18,9 +18,15 @@ {{#link-to 'userActivity.likesGiven'}}{{i18n 'user_action_groups.1'}}{{/link-to}} {{#if user.showBookmarks}} -
  • - {{#link-to 'userActivity.bookmarks'}}{{i18n 'user_action_groups.3'}}{{/link-to}} -
    • + {{#if bookmarksWithRemindersEnabled}} +
  • + {{#link-to 'userActivity.bookmarksWithReminders'}}{{i18n 'user_action_groups.3'}}{{/link-to}} +
    • + {{else}} +
  • + {{#link-to 'userActivity.bookmarks'}}{{i18n 'user_action_groups.3'}}{{/link-to}} +
    • + {{/if}} {{/if}} {{plugin-outlet name="user-activity-bottom" connectorTagName='li' diff --git a/app/assets/javascripts/discourse/templates/user/bookmarks.hbs b/app/assets/javascripts/discourse/templates/user/bookmarks.hbs new file mode 100644 index 00000000000..228490bfa4f --- /dev/null +++ b/app/assets/javascripts/discourse/templates/user/bookmarks.hbs @@ -0,0 +1,51 @@ +{{#if noContent}} +
    {{noResultsHelp}}
    +{{else}} + {{#conditional-loading-spinner condition=loading}} + + + + + + + + + {{#each content as |bookmark|}} + + + + {{raw "list/activity-column" topic=bookmark class="num" tagName="td"}} + + + {{/each}} + +
    {{i18n "topic.title"}}{{i18n "post.bookmarks.created"}}{{i18n "activity"}} 
    + +
    + {{#if bookmark.name}} + + {{d-icon "info-circle"}}{{bookmark.name}} + + {{/if}} + {{#if bookmark.reminder_at}} + + {{d-icon "far-clock"}}{{bookmark.formattedReminder}} + + {{/if}} +
    + + {{topic-status topic=bookmark}} + {{topic-link bookmark}} +     + {{#if bookmark.excerpt}} +

    {{html-safe bookmark.excerpt}}

    + {{/if}} +
    + {{category-link bookmark.category}} + {{discourse-tags bookmark mode="list" tagsForUser=tagsForUser}} +
    +     		{{format-date bookmark.created_at format="tiny"}} + {{bookmark-actions-dropdown bookmark=bookmark removeBookmark=(action "removeBookmark")}} +
    + {{/conditional-loading-spinner}} +{{/if}} diff --git a/app/assets/javascripts/discourse/widgets/post-menu.js.es6 b/app/assets/javascripts/discourse/widgets/post-menu.js.es6 index 6ae6460fac4..95c3477ffd6 100644 --- a/app/assets/javascripts/discourse/widgets/post-menu.js.es6 +++ b/app/assets/javascripts/discourse/widgets/post-menu.js.es6 @@ -321,6 +321,8 @@ registerButton("bookmarkWithReminder", (attrs, state, siteSettings) => { titleOptions = { date: reminderAtDate.format(I18n.t("dates.long_with_year")) }; + } else if (attrs.bookmarkReminderType === "at_desktop") { + title = "bookmarks.created_with_at_desktop_reminder"; } else { title = "bookmarks.created"; } diff --git a/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js.es6 b/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js.es6 index ca1642b8975..b63c2b527b6 100644 --- a/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js.es6 +++ b/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js.es6 @@ -16,7 +16,11 @@ createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", { }, showAllHref() { - return `${this.attrs.path}/activity/bookmarks`; + if (this.siteSettings.enable_bookmarks_with_reminders) { + return `${this.attrs.path}/activity/bookmarks-with-reminders`; + } else { + return `${this.attrs.path}/activity/bookmarks`; + } }, emptyStatePlaceholderItem() { @@ -24,6 +28,50 @@ createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", { }, findNewItems() { + if (this.siteSettings.enable_bookmarks_with_reminders) { + return this.loadBookmarksWithReminders(); + } else { + return this.loadUserActivityBookmarks(); + } + }, + + itemHtml(bookmark) { + return this.attach("quick-access-item", { + icon: this.icon(bookmark), + href: postUrl( + bookmark.slug, + bookmark.topic_id, + bookmark.post_number || bookmark.linked_post_number + ), + content: bookmark.title, + username: bookmark.username + }); + }, + + icon(bookmark) { + if (bookmark.reminder_at) { + return "discourse-bookmark-clock"; + } + return ICON; + }, + + loadBookmarksWithReminders() { + return ajax(`/u/${this.currentUser.username}/bookmarks.json`, { + cache: "false", + data: { + limit: this.estimateItemLimit() + } + }).then(result => { + // The empty state help text for bookmarks page is localized on the + // server. + if (result.no_results_help) { + this.state.emptyStatePlaceholderItemText = result.no_results_help; + } + return result.bookmarks; + }); + }, + + loadUserActivityBookmarks() { return ajax("/user_actions.json", { cache: "false", data: { @@ -38,14 +86,5 @@ createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", { this.state.emptyStatePlaceholderItemText = no_results_help; return user_actions; }); - }, - - itemHtml(bookmark) { - return this.attach("quick-access-item", { - icon: ICON, - href: postUrl(bookmark.slug, bookmark.topic_id, bookmark.post_number), - content: bookmark.title, - username: bookmark.username - }); } }); diff --git a/app/assets/stylesheets/common/base/_topic-list.scss b/app/assets/stylesheets/common/base/_topic-list.scss index bd4f95a01ea..2ac61a44871 100644 --- a/app/assets/stylesheets/common/base/_topic-list.scss +++ b/app/assets/stylesheets/common/base/_topic-list.scss @@ -54,6 +54,14 @@ } } +.topic-list-item { + .post-excerpt { + margin-top: 0.5em; + margin-bottom: 0.5em; + font-size: $font-down-2; + } +} + .topic-list-main-link { font-size: $font-up-1; a.title { diff --git a/app/assets/stylesheets/common/base/tagging.scss b/app/assets/stylesheets/common/base/tagging.scss index adb1631a9b3..e2fd9158a81 100644 --- a/app/assets/stylesheets/common/base/tagging.scss +++ b/app/assets/stylesheets/common/base/tagging.scss @@ -153,10 +153,12 @@ $tag-color: $primary-medium; display: inline-block; } -.topic-list-item .discourse-tags { - display: inline-flex; - font-weight: normal; - font-size: $font-down-1; +.topic-list-item { + .discourse-tags { + display: inline-flex; + font-weight: normal; + font-size: $font-down-1; + } } .categories-list .topic-list-latest .discourse-tags { diff --git a/app/assets/stylesheets/common/components/bookmark-list-item.scss b/app/assets/stylesheets/common/components/bookmark-list-item.scss new file mode 100644 index 00000000000..7de562d1d31 --- /dev/null +++ b/app/assets/stylesheets/common/components/bookmark-list-item.scss @@ -0,0 +1,19 @@ +.bookmark-list-item { + .bookmark-metadata { + font-size: $font-down-2; + white-space: nowrap; + display: flex; + align-items: center; + margin-bottom: 0.2em; + + &-item { + margin-right: 0.5em; + display: flex; + align-items: center; + } + + .d-icon { + margin-right: 0.2em; + } + } +} diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9405620286f..837f19b8281 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -10,14 +10,15 @@ class UsersController < ApplicationController :enable_second_factor_totp, :disable_second_factor, :list_second_factors, :update_second_factor, :create_second_factor_backup, :select_avatar, :notification_level, :revoke_auth_token, :register_second_factor_security_key, - :create_second_factor_security_key, :feature_topic, :clear_featured_topic + :create_second_factor_security_key, :feature_topic, :clear_featured_topic, + :bookmarks ] skip_before_action :check_xhr, only: [ :show, :badges, :password_reset_show, :password_reset_update, :update, :account_created, :activate_account, :perform_account_activation, :user_preferences_redirect, :avatar, :my_redirect, :toggle_anon, :admin_login, :confirm_admin, :email_login, :summary, - :feature_topic, :clear_featured_topic + :feature_topic, :clear_featured_topic, :bookmarks ] before_action :second_factor_check_confirmed_password, only: [ @@ -1378,6 +1379,20 @@ class UsersController < ApplicationController render json: success_json end + def bookmarks + user = fetch_user_from_params + bookmarks = BookmarkQuery.new(user, params).list_all + + if bookmarks.empty? + render json: { + bookmarks: [], + no_results_help: I18n.t("user_activity.no_bookmarks.self") + } + else + render_serialized(bookmarks, UserBookmarkSerializer, root: 'bookmarks') + end + end + HONEYPOT_KEY ||= 'HONEYPOT_KEY' CHALLENGE_KEY ||= 'CHALLENGE_KEY' diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 780a004d6dd..a0f37d3d70e 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -182,7 +182,11 @@ class TopicViewSerializer < ApplicationSerializer end def bookmarked - object.topic_user&.bookmarked + if SiteSetting.enable_bookmarks_with_reminders? + object.has_bookmarks? + else + object.topic_user&.bookmarked + end end def topic_timer diff --git a/app/serializers/user_bookmark_serializer.rb b/app/serializers/user_bookmark_serializer.rb new file mode 100644 index 00000000000..adef053d308 --- /dev/null +++ b/app/serializers/user_bookmark_serializer.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require_relative 'post_item_excerpt' + +class UserBookmarkSerializer < ApplicationSerializer + include PostItemExcerpt + include TopicTagsMixin + + attributes :id, + :created_at, + :topic_id, + :linked_post_number, + :post_id, + :name, + :reminder_at, + :title, + :deleted, + :hidden, + :category_id, + :closed, + :archived, + :archetype, + :highest_post_number, + :bumped_at, + :slug, + :username + + def closed + object.topic_closed + end + + def archived + object.topic_archived + end + + def linked_post_number + object.post.post_number + end + + def title + object.topic.title + end + + def deleted + object.topic.deleted_at.present? || object.post.deleted_at.present? + end + + def hidden + object.post.hidden + end + + def category_id + object.topic.category_id + end + + def archetype + object.topic.archetype + end + + def archived + object.topic.archived + end + + def closed + object.topic.closed + end + + def highest_post_number + object.topic.highest_post_number + end + + def bumped_at + object.topic.bumped_at + end + + def raw + object.post.raw + end + + def cooked + object.post.cooked + end + + def slug + object.topic.slug + end + + def username + object.post.user.username + end +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d8ff29639d7..4d61f5a28a7 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -307,6 +307,7 @@ en: created: "you've bookmarked this post" not_bookmarked: "bookmark this post" created_with_reminder: "you've bookmarked this post with a reminder at %{date}" + created_with_at_desktop_reminder: "you've bookmarked this post and will be reminded next time you are at your desktop" remove: "Remove Bookmark" confirm_clear: "Are you sure you want to clear all your bookmarks from this topic?" save: "Save" @@ -2116,6 +2117,8 @@ en: other { {BOTH, select, true{and } false {are } other{}} # new topics} } remaining, or {CATEGORY, select, true {browse other topics in {catLink}} false {{latestLink}} other {}}" + bumped_at_title_MF: "{FIRST_POST}: {CREATED_AT}\n{LAST_POST}: {BUMPED_AT}" + browse_all_categories: Browse all categories view_latest_topics: view latest topics @@ -2667,6 +2670,7 @@ en: bookmarks: create: "Create bookmark" + created: "Created" name: "Name" name_placeholder: "Name the bookmark to help jog your memory" set_reminder: "Set a reminder" diff --git a/config/routes.rb b/config/routes.rb index 38047b91eb7..7f7a457af3f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -469,6 +469,7 @@ Discourse::Application.routes.draw do get "#{root_path}/:username/activity" => "users#show", constraints: { username: RouteFormat.username } get "#{root_path}/:username/activity/:filter" => "users#show", constraints: { username: RouteFormat.username } get "#{root_path}/:username/badges" => "users#badges", constraints: { username: RouteFormat.username } + get "#{root_path}/:username/bookmarks" => "users#bookmarks", constraints: { username: RouteFormat.username } get "#{root_path}/:username/notifications" => "users#show", constraints: { username: RouteFormat.username } get "#{root_path}/:username/notifications/:filter" => "users#show", constraints: { username: RouteFormat.username } delete "#{root_path}/:username" => "users#destroy", constraints: { username: RouteFormat.username } diff --git a/lib/bookmark_query.rb b/lib/bookmark_query.rb new file mode 100644 index 00000000000..ab18ea9edb5 --- /dev/null +++ b/lib/bookmark_query.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +## +# Allows us to query Bookmark records for lists. Used mainly +# in the user/activity/bookmarks page. + +class BookmarkQuery + def initialize(user, params) + @user = user + @params = params + end + + def list_all + results = user_bookmarks + .joins('INNER JOIN topics ON topics.id = bookmarks.topic_id') + .joins('INNER JOIN posts ON posts.id = bookmarks.post_id') + .joins('INNER JOIN users ON users.id = posts.user_id') + .order('created_at DESC') + + if @params[:limit] + results = results.limit(@params[:limit]) + end + + results + end + + private + + def user_bookmarks + Bookmark.where(user: @user).includes(:topic).includes(post: :user) + end +end diff --git a/lib/bookmark_reminder_notification_handler.rb b/lib/bookmark_reminder_notification_handler.rb index 166c46274ea..83f0b9fb3d0 100644 --- a/lib/bookmark_reminder_notification_handler.rb +++ b/lib/bookmark_reminder_notification_handler.rb @@ -54,7 +54,7 @@ class BookmarkReminderNotificationHandler def self.send_at_desktop_reminder(user:, request_user_agent:) return if !SiteSetting.enable_bookmarks_with_reminders - return if MobileDetection.mobile_device?(BrowserDetection.device(request_user_agent).to_s) + return if MobileDetection.mobile_device?(request_user_agent) return if !user_has_pending_at_desktop_reminders?(user) diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 62f953b6fd5..3123e19b46e 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -349,6 +349,11 @@ class TopicView end end + def has_bookmarks? + return false if @user.blank? + @topic.bookmarks.exists?(user_id: @user.id) + end + MAX_PARTICIPANTS = 24 def post_counts_by_user diff --git a/spec/lib/bookmark_reminder_notification_handler_spec.rb b/spec/lib/bookmark_reminder_notification_handler_spec.rb index e64c9b1c704..a2bf4a30af5 100644 --- a/spec/lib/bookmark_reminder_notification_handler_spec.rb +++ b/spec/lib/bookmark_reminder_notification_handler_spec.rb @@ -10,31 +10,36 @@ RSpec.describe BookmarkReminderNotificationHandler do before do SiteSetting.enable_bookmarks_with_reminders = true end + fab!(:reminder) do + Fabricate( + :bookmark, + user: user, + reminder_type: Bookmark.reminder_types[:at_desktop], + reminder_at: nil, + reminder_set_at: Time.zone.now + ) + end + let(:user_agent) { "Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 Safari/537.36" } - context "when the user agent is for mobile" do - let(:user_agent) { "Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13B143 Safari/601.1" } - it "does not attempt to send any reminders" do - DistributedMutex.expects(:synchronize).never - send_reminder - end + before do + Discourse.redis.flushall end - context "when the user agent is for desktop" do - let(:user_agent) { "Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 Safari/537.36" } - fab!(:reminder) do - Fabricate( - :bookmark, - user: user, - reminder_type: Bookmark.reminder_types[:at_desktop], - reminder_at: nil, - reminder_set_at: Time.zone.now - ) + context "when there are pending bookmark at desktop reminders" do + before do + described_class.cache_pending_at_desktop_reminder(user) end - context "when there are pending bookmark at desktop reminders" do - before do - described_class.cache_pending_at_desktop_reminder(user) + context "when the user agent is for mobile" do + let(:user_agent) { "Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13B143 Safari/601.1" } + it "does not attempt to send any reminders" do + DistributedMutex.expects(:synchronize).never + send_reminder end + end + + context "when the user agent is for desktop" do + let(:user_agent) { "Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 Safari/537.36" } it "deletes the key in redis" do send_reminder @@ -49,23 +54,23 @@ RSpec.describe BookmarkReminderNotificationHandler do expect(reminder.reload.reminder_set_at).to eq(nil) end end + end - context "when there are no pending bookmark at desktop reminders" do - it "does nothing" do - DistributedMutex.expects(:synchronize).never - send_reminder - end + context "when there are no pending bookmark at desktop reminders" do + it "does nothing" do + DistributedMutex.expects(:synchronize).never + send_reminder + end + end + + context "when enable bookmarks with reminders is disabled" do + before do + SiteSetting.enable_bookmarks_with_reminders = false end - context "when enable bookmarks with reminders is disabled" do - before do - SiteSetting.enable_bookmarks_with_reminders = false - end - - it "does nothing" do - BrowserDetection.expects(:device).never - send_reminder - end + it "does nothing" do + BrowserDetection.expects(:device).never + send_reminder end end