Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: Add bulk action to bookmark (#26856) 

This PR aims to add bulk actions to the user's bookmarks.

After this feature, all users should be able to select multiple bookmarks and perform the actions of "deleting" or "clear reminders"
This commit is contained in:
Amanda Alves Branquinho 2024-05-22 12:50:21 -03:00 committed by GitHub
parent 4c10b2eb33
commit b0d95c8c78
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 602 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
app/assets/javascripts/discourse/app/components/bookmark-list.hbs
View File

@ -7,7 +7,55 @@
<thead class="topic-list-header">
{{#if this.site.desktopView}}
<PluginOutlet @name="bookmark-list-table-header">
<th class="topic-list-data">{{i18n "topic.title"}}</th>
{{#if this.bulkSelectEnabled}}
<th class="bulk-select topic-list-data">
<FlatButton
@action={{this.toggleBulkSelect}}
@class="bulk-select"
@icon="tasks"
@title="bookmarks.bulk.toggle"
/>
</th>
{{/if}}
<th class="topic-list-data">
{{#if this.bulkSelectEnabled}}
<span class="bulk-select-topics">
{{~#if this.canDoBulkActions}}
<div class="bulk-select-bookmarks-dropdown">
<span class="bulk-select-bookmark-dropdown__count">
{{i18n
"bookmarks.bulk.selected_count"
count=this.selectedCount
}}
</span>
<BulkSelectBookmarksDropdown
@bulkSelectHelper={{this.bulkSelectHelper}}
/>
</div>
{{/if~}}
<DButton
@action={{this.selectAll}}
class="btn btn-default bulk-select-all"
@label="bookmarks.bulk.select_all"
/>
<DButton
@action={{this.clearAll}}
class="btn btn-default bulk-clear-all"
@label="bookmarks.bulk.clear_all"
/>
</span>
{{else}}
<FlatButton
@action={{this.toggleBulkSelect}}
@class="bulk-select"
@icon="tasks"
@title="bookmarks.bulk.toggle"
/>
{{i18n "topic.title"}}
{{/if~}}
</th>
<th class="topic-list-data">&nbsp;</th>
<th class="post-metadata topic-list-data">{{i18n
"post.bookmarks.updated"
@ -20,6 +68,18 @@
<tbody class="topic-list-body">
{{#each this.content as |bookmark|}}
<tr class="topic-list-item bookmark-list-item">
{{#if this.bulkSelectEnabled}}
<td class="bulk-select bookmark-list-data">
<label for="bulk-select-{{bookmark.id}}">
<input
type="checkbox"
class="bulk-select"
id="bulk-select-{{bookmark.id}}"
data-id={{bookmark.id}}
/>
</label>
</td>
{{/if}}
<th scope="row" class="main-link topic-list-data">
<span class="link-top-line">
<div class="bookmark-metadata">

88
app/assets/javascripts/discourse/app/components/bookmark-list.js
View File

@ -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);
});
},
});

13
app/assets/javascripts/discourse/app/controllers/user-activity-bookmarks.js
View File

@ -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();

1
app/assets/javascripts/discourse/app/lib/bulk-select-helper.js
View File

@ -14,6 +14,7 @@ export default class BulkSelectHelper {
@tracked bulkSelectEnabled = false;
@tracked autoAddTopicsToBulkSelect = false;
@tracked autoAddBookmarksToBulkSelect = false;
selected = new TrackedArray();

12
app/assets/javascripts/discourse/app/models/bookmark.js
View File

@ -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);
}

1
app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs
View File

@ -28,6 +28,7 @@
<div class="alert alert-info">{{i18n "user.no_bookmarks_search"}}</div>
{{else}}
<BookmarkList
@bulkSelectHelper={{this.bulkSelectHelper}}
@loadMore={{action "loadMore"}}
@reload={{action "reload"}}
@loadingMore={{this.loadingMore}}

63
app/assets/javascripts/discourse/tests/acceptance/user-bookmark-bulk-actions-test.js Normal file
View File

@ -0,0 +1,63 @@
import { click, visit } from "@ember/test-helpers";
import { test } from "qunit";
import {
acceptance,
count,
exists,
queryAll,
} from "discourse/tests/helpers/qunit-helpers";
import selectKit from "discourse/tests/helpers/select-kit-helper";
import I18n from "discourse-i18n";
acceptance("Bookmark - Bulk Actions", function (needs) {
needs.user();
test("bulk select - modal", async function (assert) {
await visit("/u/eviltrout/activity/bookmarks");
assert.ok(exists("button.bulk-select"));
await click("button.bulk-select");
await click(queryAll("input.bulk-select")[0]);
await click(queryAll("input.bulk-select")[1]);
const dropdown = selectKit(".select-kit.bulk-select-bookmarks-dropdown");
await dropdown.expand();
await dropdown.selectRowByValue("clear-reminders");
assert.ok(exists(".dialog-container"), "it should show the modal");
assert.dom(".dialog-container .dialog-body").includesText(
I18n.t("js.bookmark_bulk_actions.clear_reminders.description", {
count: 2,
}).replaceAll(/\<.*?>/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, "")
);
});
});

101
app/assets/javascripts/select-kit/addon/components/bulk-select-bookmarks-dropdown.js Normal file
View File

@ -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);
},
});
}
},
});

13
app/assets/stylesheets/common/base/_bookmark-list.scss Normal file
View File

@ -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;
}
}
}

1
app/assets/stylesheets/common/base/_index.scss
View File

@ -1,4 +1,5 @@
@import "_topic-list";
@import "_bookmark-list";
@import "about";
@import "activation";
@import "alert";

20
app/controllers/bookmarks_controller.rb
View File

@ -74,4 +74,24 @@ class BookmarksController < ApplicationController
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

23
config/locales/client.en.yml
View File

@ -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 <b>%{count}</b> 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 <b>%{count}</b> 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

2
config/routes.rb
View File

@ -1101,6 +1101,8 @@ Discourse::Application.routes.draw do
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"

2
lib/bookmark_reminder_notification_handler.rb
View File

@ -24,8 +24,6 @@ class BookmarkReminderNotificationHandler
end
end
private
def clear_reminder
Rails.logger.debug(
"Clearing bookmark reminder for bookmark_id #{bookmark.id}. reminder at: #{bookmark.reminder_at}",

54
lib/bookmarks_bulk_action.rb Normal file
View File

@ -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

51
spec/lib/bookmarks_bulk_action_spec.rb Normal file
View File

@ -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

100
spec/requests/bookmarks_controller_spec.rb
View File

@ -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 @@ RSpec.describe BookmarksController do
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 @@ RSpec.describe BookmarksController do
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