diff --git a/app/assets/javascripts/discourse/app/components/bookmark-list.hbs b/app/assets/javascripts/discourse/app/components/bookmark-list.hbs index 69c5dadac97b..2f42a9a49ad8 100644 --- a/app/assets/javascripts/discourse/app/components/bookmark-list.hbs +++ b/app/assets/javascripts/discourse/app/components/bookmark-list.hbs @@ -7,7 +7,55 @@ {{#if this.site.desktopView}} - {{i18n "topic.title"}} + {{#if this.bulkSelectEnabled}} + + + + {{/if}} + + + {{#if this.bulkSelectEnabled}} + + {{~#if this.canDoBulkActions}} +
+ + {{i18n + "bookmarks.bulk.selected_count" + count=this.selectedCount + }} + + +
+ + {{/if~}} + + + + {{else}} + + {{i18n "topic.title"}} + {{/if~}} +   {{i18n "post.bookmarks.updated" @@ -20,6 +68,18 @@ {{#each this.content as |bookmark|}} + {{#if this.bulkSelectEnabled}} + + + + {{/if}}
diff --git a/app/assets/javascripts/discourse/app/components/bookmark-list.js b/app/assets/javascripts/discourse/app/components/bookmark-list.js index 9518a62b6912..b25b658463db 100644 --- a/app/assets/javascripts/discourse/app/components/bookmark-list.js +++ b/app/assets/javascripts/discourse/app/components/bookmark-list.js @@ -1,5 +1,6 @@ import Component from "@ember/component"; import { action } from "@ember/object"; +import { dependentKeyCompat } from "@ember/object/compat"; import { service } from "@ember/service"; import { Promise } from "rsvp"; import BookmarkModal from "discourse/components/modal/bookmark"; @@ -16,6 +17,18 @@ export default Component.extend({ modal: service(), classNames: ["bookmark-list-wrapper"], + get canDoBulkActions() { + return this.bulkSelectHelper?.selected.length; + }, + + get selected() { + return this.bulkSelectHelper?.selected; + }, + + get selectedCount() { + return this.selected?.length || 0; + }, + @action removeBookmark(bookmark) { return new Promise((resolve, reject) => { @@ -90,7 +103,82 @@ export default Component.extend({ bookmark.togglePin().then(this.reload); }, + @action + toggleBulkSelect() { + this.bulkSelectHelper?.toggleBulkSelect(); + this.rerender(); + }, + + @action + selectAll() { + this.bulkSelectHelper.autoAddBookmarksToBulkSelect = true; + document + .querySelectorAll("input.bulk-select:not(:checked)") + .forEach((el) => el.click()); + }, + + @action + clearAll() { + this.bulkSelectHelper.autoAddBookmarksToBulkSelect = false; + document + .querySelectorAll("input.bulk-select:checked") + .forEach((el) => el.click()); + }, + + @dependentKeyCompat // for the classNameBindings + get bulkSelectEnabled() { + return this.bulkSelectHelper?.bulkSelectEnabled; + }, + _removeBookmarkFromList(bookmark) { this.content.removeObject(bookmark); }, + + _toggleSelection(target, bookmark, isSelectingRange) { + const selected = this.selected; + + if (target.checked) { + selected.addObject(bookmark); + + if (isSelectingRange) { + const bulkSelects = Array.from( + document.querySelectorAll("input.bulk-select") + ), + from = bulkSelects.indexOf(target), + to = bulkSelects.findIndex((el) => el.id === this.lastChecked.id), + start = Math.min(from, to), + end = Math.max(from, to); + + bulkSelects + .slice(start, end) + .filter((el) => el.checked !== true) + .forEach((checkbox) => { + checkbox.click(); + }); + } + this.set("lastChecked", target); + } else { + selected.removeObject(bookmark); + this.set("lastChecked", null); + } + }, + + click(e) { + const onClick = (sel, callback) => { + let target = e.target.closest(sel); + + if (target) { + callback(target); + } + }; + + onClick("input.bulk-select", () => { + const target = e.target; + const bookmarkId = target.dataset.id; + const bookmark = this.content.find( + (item) => item.id.toString() === bookmarkId + ); + this._toggleSelection(target, bookmark, this.lastChecked && e.shiftKey); + }); + }, }); diff --git a/app/assets/javascripts/discourse/app/controllers/user-activity-bookmarks.js b/app/assets/javascripts/discourse/app/controllers/user-activity-bookmarks.js index afcfec005c82..ed54292fffff 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-activity-bookmarks.js +++ b/app/assets/javascripts/discourse/app/controllers/user-activity-bookmarks.js @@ -5,6 +5,7 @@ import { service } from "@ember/service"; import { htmlSafe } from "@ember/template"; import { Promise } from "rsvp"; import { ajax } from "discourse/lib/ajax"; +import BulkSelectHelper from "discourse/lib/bulk-select-helper"; import Bookmark from "discourse/models/bookmark"; import { iconHTML } from "discourse-common/lib/icon-library"; import discourseComputed from "discourse-common/utils/decorators"; @@ -23,6 +24,13 @@ export default Controller.extend({ inSearchMode: notEmpty("q"), noContent: equal("model.bookmarks.length", 0), + bulkSelectHelper: null, + + init() { + this._super(...arguments); + this.bulkSelectHelper = new BulkSelectHelper(this); + }, + searchTerm: computed("q", { get() { return this.q; @@ -77,6 +85,11 @@ export default Controller.extend({ .finally(() => this.set("loadingMore", false)); }, + @action + updateAutoAddBookmarksToBulkSelect(value) { + this.bulkSelectHelper.autoAddBookmarksToBulkSelect = value; + }, + _loadMoreBookmarks(searchQuery) { if (!this.model.loadMoreUrl) { return Promise.resolve(); diff --git a/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js b/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js index 743ad117dc7c..d2edd724a6d9 100644 --- a/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js +++ b/app/assets/javascripts/discourse/app/lib/bulk-select-helper.js @@ -14,6 +14,7 @@ export default class BulkSelectHelper { @tracked bulkSelectEnabled = false; @tracked autoAddTopicsToBulkSelect = false; + @tracked autoAddBookmarksToBulkSelect = false; selected = new TrackedArray(); diff --git a/app/assets/javascripts/discourse/app/models/bookmark.js b/app/assets/javascripts/discourse/app/models/bookmark.js index 8898862a8aeb..22e5c963476b 100644 --- a/app/assets/javascripts/discourse/app/models/bookmark.js +++ b/app/assets/javascripts/discourse/app/models/bookmark.js @@ -42,6 +42,18 @@ export default class Bookmark extends RestModel { }); } + static bulkOperation(bookmarks, operation) { + const data = { + bookmark_ids: bookmarks.mapBy("id"), + operation, + }; + + return ajax("/bookmarks/bulk", { + type: "PUT", + data, + }); + } + static async applyTransformations(bookmarks) { await applyModelTransformations("bookmark", bookmarks); } diff --git a/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs b/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs index d86e21490db3..2a7850319eae 100644 --- a/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs @@ -28,6 +28,7 @@
{{i18n "user.no_bookmarks_search"}}
{{else}} /g, "") + ); + + await click("button.bulk-clear-all"); + + assert.strictEqual( + count("input.bulk-select:checked"), + 0, + "Clear all should clear all selection" + ); + + await click("button.bulk-select-all"); + + assert.strictEqual( + count("input.bulk-select:checked"), + 2, + "Select all should select all topics" + ); + + await dropdown.expand(); + await dropdown.selectRowByValue("delete-bookmarks"); + + assert.ok(exists(".dialog-container"), "it should show the modal"); + + assert.dom(".dialog-container .dialog-body").includesText( + I18n.t("js.bookmark_bulk_actions.delete_bookmarks.description", { + count: 2, + }).replaceAll(/\<.*?>/g, "") + ); + }); +}); diff --git a/app/assets/javascripts/select-kit/addon/components/bulk-select-bookmarks-dropdown.js b/app/assets/javascripts/select-kit/addon/components/bulk-select-bookmarks-dropdown.js new file mode 100644 index 000000000000..f9a30095fdd0 --- /dev/null +++ b/app/assets/javascripts/select-kit/addon/components/bulk-select-bookmarks-dropdown.js @@ -0,0 +1,101 @@ +import { action } from "@ember/object"; +import { service } from "@ember/service"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import Bookmark from "discourse/models/bookmark"; +import i18n from "discourse-common/helpers/i18n"; +import DropdownSelectBoxComponent from "select-kit/components/dropdown-select-box"; + +const _customButtons = []; +const _customActions = {}; + +export function addBulkDropdownAction(name, customAction) { + _customActions[name] = customAction; +} + +export default DropdownSelectBoxComponent.extend({ + classNames: ["bulk-select-bookmarks-dropdown"], + headerIcon: null, + showFullTitle: true, + selectKitOptions: { + showCaret: true, + showFullTitle: true, + none: "select_kit.components.bulk_select_bookmarks_dropdown.title", + }, + + router: service(), + toasts: service(), + dialog: service(), + + get content() { + let options = []; + options = options.concat([ + { + id: "clear-reminders", + icon: "tag", + name: i18n("bookmark_bulk_actions.clear_reminders.name"), + }, + { + id: "delete-bookmarks", + icon: "trash-alt", + name: i18n("bookmark_bulk_actions.delete_bookmarks.name"), + }, + ]); + + return [...options, ..._customButtons]; + }, + + getSelectedBookmarks() { + return this.bulkSelectHelper.selected; + }, + + @action + onSelect(id) { + switch (id) { + case "clear-reminders": + this.dialog.yesNoConfirm({ + message: i18n( + `js.bookmark_bulk_actions.clear_reminders.description`, + { + count: this.getSelectedBookmarks().length, + } + ), + didConfirm: () => { + Bookmark.bulkOperation(this.getSelectedBookmarks(), { + type: "clear_reminder", + }) + .then(() => { + this.router.refresh(); + this.toasts.success({ + duration: 3000, + data: { message: i18n("bookmarks.bulk.reminders_cleared") }, + }); + }) + .catch(popupAjaxError); + }, + }); + break; + case "delete-bookmarks": + this.dialog.deleteConfirm({ + message: i18n( + `js.bookmark_bulk_actions.delete_bookmarks.description`, + { + count: this.getSelectedBookmarks().length, + } + ), + didConfirm: () => { + Bookmark.bulkOperation(this.getSelectedBookmarks(), { + type: "delete", + }) + .then(() => { + this.router.refresh(); + this.toasts.success({ + duration: 3000, + data: { message: i18n("bookmarks.bulk.delete_completed") }, + }); + }) + .catch(popupAjaxError); + }, + }); + } + }, +}); diff --git a/app/assets/stylesheets/common/base/_bookmark-list.scss b/app/assets/stylesheets/common/base/_bookmark-list.scss new file mode 100644 index 000000000000..d4c8d07d25c3 --- /dev/null +++ b/app/assets/stylesheets/common/base/_bookmark-list.scss @@ -0,0 +1,13 @@ +.bulk-select-bookmarks-dropdown { + .select-kit.single-select.dropdown-select-box .select-kit-row { + .texts .name { + font-weight: normal; + } + .icons { + font-size: var(--font-down-2); + margin-right: 0.75em; + position: relative; + top: 0.15em; + } + } +} diff --git a/app/assets/stylesheets/common/base/_index.scss b/app/assets/stylesheets/common/base/_index.scss index 5f4cea26a9d1..993aa21dff28 100644 --- a/app/assets/stylesheets/common/base/_index.scss +++ b/app/assets/stylesheets/common/base/_index.scss @@ -1,4 +1,5 @@ @import "_topic-list"; +@import "_bookmark-list"; @import "about"; @import "activation"; @import "alert"; diff --git a/app/controllers/bookmarks_controller.rb b/app/controllers/bookmarks_controller.rb index a3e385e585c7..d5414fec6f85 100644 --- a/app/controllers/bookmarks_controller.rb +++ b/app/controllers/bookmarks_controller.rb @@ -74,4 +74,24 @@ def toggle_pin render json: failed_json.merge(errors: bookmark_manager.errors.full_messages), status: 400 end + + def bulk + if params[:bookmark_ids].present? + unless Array === params[:bookmark_ids] + raise Discourse::InvalidParameters.new( + "Expecting bookmark_ids to contain a list of bookmark ids", + ) + end + bookmark_ids = params[:bookmark_ids].map { |t| t.to_i } + else + raise ActionController::ParameterMissing.new(:bookmark_ids) + end + + operation = params.require(:operation).permit(:type).to_h.symbolize_keys + + raise ActionController::ParameterMissing.new(:operation_type) if operation[:type].blank? + operator = BookmarksBulkAction.new(current_user, bookmark_ids, operation) + changed_bookmark_ids = operator.perform! + render_json_dump bookmark_ids: changed_bookmark_ids + end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f3056a3e92c0..e8153a5f0ead 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -387,12 +387,33 @@ en: search_placeholder: "Search bookmarks by name, topic title, or post content" search: "Search" bookmark: "Bookmark" + bulk: + delete_completed: "Bookmarks successfully deleted." + reminders_cleared: "Bookmark reminders successfully cleared." + toggle: "toggle bulk selection of bookmarks" + select_all: "Select All" + clear_all: "Clear All" + selected_count: + one: "%{count} selected" + other: "%{count} selected" reminders: 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 %{at_date_time}" + bookmark_bulk_actions: + clear_reminders: + name: "Clear Reminders" + description: + one: "Are you sure you want to clear the reminder for this bookmark?" + other: "Are you sure you want to clear the reminder for these %{count} bookmarks." + delete_bookmarks: + name: "Delete Bookmark" + description: + one: "Are you sure you want to delete this bookmark?" + other: "Are you sure you want to delete these %{count} bookmarks." + copy_codeblock: copied: "copied!" copy: "copy code to clipboard" @@ -2407,6 +2428,8 @@ en: title: "Manage categories" bulk_select_topics_dropdown: title: "Bulk Actions" + bulk_select_bookmarks_dropdown: + title: "Bulk Actions" date_time_picker: from: From diff --git a/config/routes.rb b/config/routes.rb index c43394049495..02128934166e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1101,6 +1101,8 @@ def patch(*) delete "admin/groups/:id/members" => "groups#remove_member", :constraints => AdminConstraint.new put "admin/groups/:id/members" => "groups#add_members", :constraints => AdminConstraint.new + put "bookmarks/bulk" + resources :posts, only: %i[show update create destroy] do delete "bookmark", to: "posts#destroy_bookmark" put "wiki" diff --git a/lib/bookmark_reminder_notification_handler.rb b/lib/bookmark_reminder_notification_handler.rb index 247cc4637db3..9e041c28e257 100644 --- a/lib/bookmark_reminder_notification_handler.rb +++ b/lib/bookmark_reminder_notification_handler.rb @@ -24,8 +24,6 @@ def send_notification end end - private - def clear_reminder Rails.logger.debug( "Clearing bookmark reminder for bookmark_id #{bookmark.id}. reminder at: #{bookmark.reminder_at}", diff --git a/lib/bookmarks_bulk_action.rb b/lib/bookmarks_bulk_action.rb new file mode 100644 index 000000000000..c433c3bdf8de --- /dev/null +++ b/lib/bookmarks_bulk_action.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +class BookmarksBulkAction + def initialize(user, bookmark_ids, operation, options = {}) + @user = user + @bookmark_ids = bookmark_ids + @operation = operation + @changed_ids = [] + @options = options + end + + def self.operations + @operations ||= %w[clear_reminder delete] + end + + def perform! + unless BookmarksBulkAction.operations.include?(@operation[:type]) + raise Discourse::InvalidParameters.new(:operation) + end + # careful these are private methods, we need send + send(@operation[:type]) + @changed_ids.sort + end + + private + + def delete + @bookmark_ids.each do |b_id| + if guardian.can_delete?(b_id) + BookmarkManager.new(@user).destroy(b_id) + @changed_ids << b_id + end + end + end + + def clear_reminder + bookmarks.each do |b| + if guardian.can_edit?(b) + BookmarkReminderNotificationHandler.new(b).clear_reminder + @changed_ids << b.id + else + raise Discourse::InvalidAccess.new + end + end + end + + def guardian + @guardian ||= Guardian.new(@user) + end + + def bookmarks + @bookmarks ||= Bookmark.where(id: @bookmark_ids) + end +end diff --git a/spec/lib/bookmarks_bulk_action_spec.rb b/spec/lib/bookmarks_bulk_action_spec.rb new file mode 100644 index 000000000000..851c183c7268 --- /dev/null +++ b/spec/lib/bookmarks_bulk_action_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +RSpec.describe BookmarksBulkAction do + fab!(:user) { Fabricate(:user, refresh_auto_groups: true) } + fab!(:user_2) { Fabricate(:user, refresh_auto_groups: true) } + fab!(:bookmark_1) { Fabricate(:bookmark, user: user) } + fab!(:bookmark_2) { Fabricate(:bookmark, user: user) } + + describe "#delete" do + describe "when user is not the bookmark owner" do + it "does NOT delete the bookmarks" do + bba = BookmarksBulkAction.new(user_2, [bookmark_1.id, bookmark_2.id], type: "delete") + expect { bba.perform! }.to raise_error Discourse::InvalidAccess + + expect(Bookmark.where(id: bookmark_1.id)).to_not be_empty + expect(Bookmark.where(id: bookmark_2.id)).to_not be_empty + end + end + + describe "when user is the bookmark owner" do + it "deletes the bookmarks" do + bba = BookmarksBulkAction.new(user, [bookmark_1.id, bookmark_2.id], type: "delete") + bba.perform! + + expect(Bookmark.where(id: bookmark_1.id)).to be_empty + expect(Bookmark.where(id: bookmark_2.id)).to be_empty + end + end + end + + describe "#clear_reminder" do + fab!(:bookmark_with_reminder) { Fabricate(:bookmark_next_business_day_reminder, user: user) } + + describe "when user is not the bookmark owner" do + it "does NOT clear the reminder" do + bba = BookmarksBulkAction.new(user_2, [bookmark_with_reminder], type: "clear_reminder") + expect { bba.perform! }.to raise_error Discourse::InvalidAccess + expect(Bookmark.find_by_id(bookmark_with_reminder).reminder_set_at).to_not be_nil + end + end + + describe "when user is the bookmark owner" do + it "deletes the bookmarks" do + expect do + bba = BookmarksBulkAction.new(user, [bookmark_with_reminder.id], type: "clear_reminder") + bba.perform! + end.to change { Bookmark.find_by_id(bookmark_with_reminder.id).reminder_set_at }.to(nil) + end + end + end +end diff --git a/spec/requests/bookmarks_controller_spec.rb b/spec/requests/bookmarks_controller_spec.rb index aa7708475efd..aa4c25447520 100644 --- a/spec/requests/bookmarks_controller_spec.rb +++ b/spec/requests/bookmarks_controller_spec.rb @@ -2,13 +2,15 @@ RSpec.describe BookmarksController do let(:current_user) { Fabricate(:user) } + let(:user_2) { Fabricate(:user) } let(:bookmark_post) { Fabricate(:post) } + let(:bookmark_post_2) { Fabricate(:post) } let(:bookmark_topic) { Fabricate(:topic) } let(:bookmark_user) { current_user } - before { sign_in(current_user) } - describe "#create" do + before { sign_in(current_user) } + use_redis_snapshotting it "rate limits creates" do @@ -95,6 +97,8 @@ end describe "#destroy" do + before { sign_in(current_user) } + let!(:bookmark) { Fabricate(:bookmark, bookmarkable: bookmark_post, user: bookmark_user) } it "destroys the bookmark" do @@ -142,4 +146,96 @@ end end end + + describe "#bulk" do + it "needs you to be logged in" do + put "/bookmarks/bulk.json" + expect(response.status).to eq(403) + end + + describe "when logged in" do + before { sign_in(bookmark_user) } + + let!(:bookmark) { Fabricate(:bookmark, bookmarkable: bookmark_post, user: bookmark_user) } + let!(:bookmark_2) { Fabricate(:bookmark, bookmarkable: bookmark_post_2, user: bookmark_user) } + + let!(:operation) { { type: "clear_reminder" } } + let!(:bookmark_ids) { [bookmark.id, bookmark_2.id] } + + it "requires a list of bookmark_ids" do + put "/bookmarks/bulk.json", params: { operation: operation } + expect(response.status).to eq(400) + end + + it "requires an operation param" do + put "/bookmarks/bulk.json", params: { bookmark_ids: bookmark_ids } + expect(response.status).to eq(400) + end + + it "can clear reminder for the given bookmarks" do + expect do + put "/bookmarks/bulk.json", + params: { + operation: { + type: "clear_reminder", + }, + bookmark_ids: [bookmark.id], + } + expect(response.status).to eq(200) + end.to change { Bookmark.find(bookmark.id).reminder_set_at }.to(nil) + end + + it "can delete bookmarks" do + expect do + put "/bookmarks/bulk.json", + params: { + operation: { + type: "delete", + }, + bookmark_ids: [bookmark.id, bookmark_2.id], + } + + expect(response.status).to eq(200) + end.to change { Bookmark.where(id: [bookmark, bookmark_2]).count }.from(2).to(0) + end + end + + describe "can't update other user's bookmarks" do + before { sign_in(user_2) } + + let!(:bookmark) { Fabricate(:bookmark, bookmarkable: bookmark_post, user: bookmark_user) } + let!(:bookmark_2) { Fabricate(:bookmark, bookmarkable: bookmark_post_2, user: bookmark_user) } + + let!(:operation) { { type: "clear_reminder" } } + let!(:bookmark_ids) { [bookmark.id, bookmark_2.id] } + + it "CAN'T clear reminder if the bookmark does not belong to the user" do + expect do + put "/bookmarks/bulk.json", + params: { + operation: { + type: "clear_reminder", + }, + bookmark_ids: [bookmark.id, bookmark_2.id], + } + expect(response.status).to eq(403) + expect(response.parsed_body["errors"].first).to include(I18n.t("invalid_access")) + end.to_not change { Bookmark.find(bookmark.id).reminder_set_at } + end + + it "CAN'T delete bookmarks that does not belong to the user" do + expect do + put "/bookmarks/bulk.json", + params: { + operation: { + type: "delete", + }, + bookmark_ids: [bookmark.id, bookmark_2.id], + } + expect(response.status).to eq(403) + expect(response.parsed_body["errors"].first).to include(I18n.t("invalid_access")) + end.to_not change { Bookmark.where(id: [bookmark, bookmark_2]).count }.from(2) + end + end + end end