Skip to content

Commit

Permalink
FEATURE: Polymorphic bookmarks pt. 1 (CRUD) (discourse#16308)
Browse files Browse the repository at this point in the history 
This commit introduces a new use_polymorphic_bookmarks site setting
that is default false and hidden, that will be used to help continuous
development of polymorphic bookmarks. This setting **should not** be
enabled anywhere in production yet, it is purely for local development.

This commit uses the setting to enable create/update/delete actions
for polymorphic bookmarks on the server and client side. The bookmark
interactions on topics/posts are all usable. Listing, searching,
sending bookmark reminders, and other edge cases will be handled
in subsequent PRs.

Comprehensive UI tests will be added in the final PR -- we already
have them for regular bookmarks, so it will just be a matter of
changing them to be for polymorphic bookmarks.
  • Loading branch information
@martin-brennan
martin-brennan authored Mar 30, 2022
1 parent ff93833 commit b8828d4
Show file tree
Hide file tree
Showing 27 changed files with 711 additions and 127 deletions.
28 changes: 16 additions & 12 deletions app/assets/javascripts/discourse/app/components/bookmark-list.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,19 +93,23 @@ export default Component.extend(Scrolling, {

@action
editBookmark(bookmark) {
openBookmarkModal(bookmark, {
onAfterSave: (savedData) => {
this.appEvents.trigger(
"bookmarks:changed",
savedData,
bookmark.attachedTo()
);
this.reload();
},
onAfterDelete: () => {
this.reload();
openBookmarkModal(
bookmark,
{
onAfterSave: (savedData) => {
this.appEvents.trigger(
"bookmarks:changed",
savedData,
bookmark.attachedTo()
);
this.reload();
},
onAfterDelete: () => {
this.reload();
},
},
});
{ use_polymorphic_bookmarks: this.siteSettings.use_polymorphic_bookmarks }
);
},

@action
Expand Down
31 changes: 24 additions & 7 deletions app/assets/javascripts/discourse/app/components/bookmark.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,19 @@ export default Component.extend({
const data = {
reminder_at: reminderAtISO,
name: this.model.name,
post_id: this.model.postId,
id: this.model.id,
auto_delete_preference: this.autoDeletePreference,
for_topic: this.model.forTopic,
};

if (this.siteSettings.use_polymorphic_bookmarks) {
data.bookmarkable_id = this.model.bookmarkableId;
data.bookmarkable_type = this.model.bookmarkableType;
} else {
// TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
data.post_id = this.model.postId;
data.for_topic = this.model.forTopic;
}

if (this.editingExistingBookmark) {
return ajax(`/bookmarks/${this.model.id}`, {
type: "PUT",
Expand All @@ -173,15 +180,25 @@ export default Component.extend({
if (!this.afterSave) {
return;
}
this.afterSave({

const data = {
reminder_at: reminderAtISO,
for_topic: this.model.forTopic,
auto_delete_preference: this.autoDeletePreference,
post_id: this.model.postId,
id: this.model.id || response.id,
name: this.model.name,
topic_id: this.model.topicId,
});
};

if (this.siteSettings.use_polymorphic_bookmarks) {
data.bookmarkable_id = this.model.bookmarkableId;
data.bookmarkable_type = this.model.bookmarkableType;
} else {
// TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
data.post_id = this.model.postId;
data.for_topic = this.model.forTopic;
data.topic_id = this.model.topicId;
}

this.afterSave(data);
},

_deleteBookmark() {
Expand Down
56 changes: 41 additions & 15 deletions app/assets/javascripts/discourse/app/controllers/bookmark.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Controller from "@ember/controller";
import I18n from "I18n";
import ModalFunctionality from "discourse/mixins/modal-functionality";
import { action } from "@ember/object";
import { Promise } from "rsvp";
Expand All @@ -10,28 +11,53 @@ export function openBookmarkModal(
onCloseWithoutSaving: null,
onAfterSave: null,
onAfterDelete: null,
},
options = {
use_polymorphic_bookmarks: false,
}
) {
return new Promise((resolve) => {
const modalTitle = () => {
if (bookmark.for_topic) {
return bookmark.id
? "post.bookmarks.edit_for_topic"
: "post.bookmarks.create_for_topic";
if (options.use_polymorphic_bookmarks) {
return I18n.t(
bookmark.id ? "bookmarks.edit_for" : "bookmarks.create_for",
{
type: bookmark.bookmarkable_type,
}
);
} else if (bookmark.for_topic) {
return I18n.t(
bookmark.id
? "post.bookmarks.edit_for_topic"
: "post.bookmarks.create_for_topic"
);
} else {
return I18n.t(
bookmark.id ? "post.bookmarks.edit" : "post.bookmarks.create"
);
}
return bookmark.id ? "post.bookmarks.edit" : "post.bookmarks.create";
};

const model = {
id: bookmark.id,
reminderAt: bookmark.reminder_at,
autoDeletePreference: bookmark.auto_delete_preference,
name: bookmark.name,
};

if (options.use_polymorphic_bookmarks) {
model.bookmarkableId = bookmark.bookmarkable_id;
model.bookmarkableType = bookmark.bookmarkable_type;
} else {
// TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
model.postId = bookmark.post_id;
model.topicId = bookmark.topic_id;
model.forTopic = bookmark.for_topic;
}

let modalController = showModal("bookmark", {
model: {
postId: bookmark.post_id,
topicId: bookmark.topic_id,
id: bookmark.id,
reminderAt: bookmark.reminder_at,
autoDeletePreference: bookmark.auto_delete_preference,
name: bookmark.name,
forTopic: bookmark.for_topic,
},
title: modalTitle(),
model,
titleTranslated: modalTitle(),
modalClass: "bookmark-with-reminder",
});
modalController.setProperties({
Expand Down
148 changes: 114 additions & 34 deletions app/assets/javascripts/discourse/app/controllers/topic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,7 @@ export default Controller.extend(bufferedProperty("model"), {
}
},

// TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
toggleBookmark(post) {
if (!this.currentUser) {
return bootbox.alert(I18n.t("bookmarks.not_bookmarked"));
Expand Down Expand Up @@ -784,6 +785,39 @@ export default Controller.extend(bufferedProperty("model"), {
}
},

toggleBookmarkPolymorphic(post) {
if (!this.currentUser) {
return bootbox.alert(I18n.t("bookmarks.not_bookmarked"));
} else if (post) {
const bookmarkForPost = this.model.bookmarks.find(
(bookmark) =>
bookmark.bookmarkable_id === post.id &&
bookmark.bookmarkable_type === "Post"
);
return this._modifyPostBookmark(
bookmarkForPost ||
Bookmark.create({
bookmarkable_id: post.id,
bookmarkable_type: "Post",
auto_delete_preference: this.currentUser
.bookmark_auto_delete_preference,
}),
post
);
} else {
return this._toggleTopicLevelBookmarkPolymorphic().then(
(changedIds) => {
if (!changedIds) {
return;
}
changedIds.forEach((id) =>
this.appEvents.trigger("post-stream:refresh", { id })
);
}
);
}
},

jumpToIndex(index) {
this._jumpToIndex(index);
},
Expand Down Expand Up @@ -1238,46 +1272,56 @@ export default Controller.extend(bufferedProperty("model"), {
},

_modifyTopicBookmark(bookmark) {
return openBookmarkModal(bookmark, {
onAfterSave: (savedData) => {
this._syncBookmarks(savedData);
this.model.set("bookmarking", false);
this.model.set("bookmarked", true);
this.model.incrementProperty("bookmarksWereChanged");
this.appEvents.trigger(
"bookmarks:changed",
savedData,
bookmark.attachedTo()
);
return openBookmarkModal(
bookmark,
{
onAfterSave: (savedData) => {
this._syncBookmarks(savedData);
this.model.set("bookmarking", false);
this.model.set("bookmarked", true);
this.model.incrementProperty("bookmarksWereChanged");
this.appEvents.trigger(
"bookmarks:changed",
savedData,
bookmark.attachedTo()
);

// TODO (martin) (2022-02-01) Remove these old bookmark events, replaced by bookmarks:changed.
this.appEvents.trigger("topic:bookmark-toggled");
},
onAfterDelete: (topicBookmarked, bookmarkId) => {
this.model.removeBookmark(bookmarkId);
// TODO (martin) (2022-02-01) Remove these old bookmark events, replaced by bookmarks:changed.
this.appEvents.trigger("topic:bookmark-toggled");
},
onAfterDelete: (topicBookmarked, bookmarkId) => {
this.model.removeBookmark(bookmarkId);
},
},
});
{ use_polymorphic_bookmarks: this.siteSettings.use_polymorphic_bookmarks }
);
},

_modifyPostBookmark(bookmark, post) {
return openBookmarkModal(bookmark, {
onCloseWithoutSaving: () => {
post.appEvents.trigger("post-stream:refresh", {
id: bookmark.post_id,
});
},
onAfterSave: (savedData) => {
this._syncBookmarks(savedData);
this.model.set("bookmarking", false);
post.createBookmark(savedData);
this.model.afterPostBookmarked(post, savedData);
return [post.id];
},
onAfterDelete: (topicBookmarked, bookmarkId) => {
this.model.removeBookmark(bookmarkId);
post.deleteBookmark(topicBookmarked);
return openBookmarkModal(
bookmark,
{
onCloseWithoutSaving: () => {
post.appEvents.trigger("post-stream:refresh", {
id: this.siteSettings.use_polymorphic_bookmarks
? bookmark.bookmarkable_id
: bookmark.post_id,
});
},
onAfterSave: (savedData) => {
this._syncBookmarks(savedData);
this.model.set("bookmarking", false);
post.createBookmark(savedData);
this.model.afterPostBookmarked(post, savedData);
return [post.id];
},
onAfterDelete: (topicBookmarked, bookmarkId) => {
this.model.removeBookmark(bookmarkId);
post.deleteBookmark(topicBookmarked);
},
},
});
{ use_polymorphic_bookmarks: this.siteSettings.use_polymorphic_bookmarks }
);
},

_syncBookmarks(data) {
Expand All @@ -1295,6 +1339,7 @@ export default Controller.extend(bufferedProperty("model"), {
}
},

// TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
async _toggleTopicLevelBookmark() {
if (this.model.bookmarking) {
return Promise.resolve();
Expand Down Expand Up @@ -1329,6 +1374,41 @@ export default Controller.extend(bufferedProperty("model"), {
}
},

async _toggleTopicLevelBookmarkPolymorphic() {
if (this.model.bookmarking) {
return Promise.resolve();
}

if (this.model.bookmarkCount > 1) {
return this._maybeClearAllBookmarks();
}

if (this.model.bookmarkCount === 1) {
const topicBookmark = this.model.bookmarks.findBy(
"bookmarkable_type",
"Topic"
);
if (topicBookmark) {
return this._modifyTopicBookmark(topicBookmark);
} else {
const bookmark = this.model.bookmarks[0];
const post = await this.model.postById(bookmark.bookmarkable_id);
return this._modifyPostBookmark(bookmark, post);
}
}

if (this.model.bookmarkCount === 0) {
return this._modifyTopicBookmark(
Bookmark.create({
bookmarkable_id: this.model.id,
bookmarkable_type: "Topic",
auto_delete_preference: this.currentUser
.bookmark_auto_delete_preference,
})
);
}
},

_maybeClearAllBookmarks() {
return new Promise((resolve) => {
bootbox.confirm(
Expand Down
19 changes: 14 additions & 5 deletions app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const DEFER_PRIORITY = 500;
export default {
name: "topic-footer-buttons",

initialize() {
initialize(container) {
const siteSettings = container.lookup("site-settings:main");
registerTopicFooterButton({
id: "share-and-invite",
icon: "d-topic-share",
Expand Down Expand Up @@ -98,9 +99,14 @@ export default {
if (this.topic.bookmarkCount === 0) {
return I18n.t("bookmarked.help.bookmark");
} else if (this.topic.bookmarkCount === 1) {
if (
this.topic.bookmarks.filter((bookmark) => bookmark.for_topic).length
) {
// TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
const anyTopicBookmarks = this.topic.bookmarks.some((bookmark) => {
return siteSettings.use_polymorphic_bookmarks
? bookmark.for_topic
: bookmark.bookmarkable_type === "Topic";
});

if (anyTopicBookmarks) {
return I18n.t("bookmarked.help.edit_bookmark_for_topic");
} else {
return I18n.t("bookmarked.help.edit_bookmark");
Expand All @@ -113,7 +119,10 @@ export default {
return I18n.t("bookmarked.help.unbookmark");
}
},
action: "toggleBookmark",
// TODO (martin) [POLYBOOK] Not relevant once polymorphic bookmarks are implemented.
action: siteSettings.use_polymorphic_bookmarks
? "toggleBookmarkPolymorphic"
: "toggleBookmark",
dropdown() {
return this.site.mobileView;
},
Expand Down
Loading

0 comments on commit b8828d4

Please sign in to comment.