FIX: Rename all instances of bookmarkWithReminder to just bookmark (#9579)

* Rename all instances of bookmarkWithReminder and bookmark_with_reminder to just bookmark
* Delete old bookmark code at the same time
* Add migration to remove the bookmarkWithReminder post menu item if people have it set in site settings
This commit is contained in:
Martin Brennan 2020-04-30 10:09:22 +10:00 committed by GitHub
parent 4f5ed8e781
commit ca539fdccf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 55 additions and 134 deletions

View File

@ -652,7 +652,7 @@ export default Controller.extend(bufferedProperty("model"), {
if (!this.currentUser) {
return bootbox.alert(I18n.t("bookmarks.not_bookmarked"));
} else if (post) {
return post.toggleBookmarkWithReminder();
return post.toggleBookmark();
} else {
return this.model.toggleBookmark().then(changedIds => {
if (!changedIds) {
@ -665,16 +665,6 @@ export default Controller.extend(bufferedProperty("model"), {
}
},
toggleBookmarkWithReminder(post) {
if (!this.currentUser) {
return bootbox.alert(I18n.t("bookmarks.not_bookmarked"));
} else if (post) {
return post.toggleBookmarkWithReminder();
} else {
return this.model.toggleBookmarkWithReminder();
}
},
jumpToIndex(index) {
this._jumpToIndex(index);
},

View File

@ -35,7 +35,6 @@ export function transformBasicPost(post) {
username: post.username,
avatar_template: post.avatar_template,
bookmarked: post.bookmarked,
bookmarkedWithReminder: post.bookmarked_with_reminder,
bookmarkReminderAt: post.bookmark_reminder_at,
bookmarkReminderType: post.bookmark_reminder_type,
yours: post.yours,

View File

@ -310,32 +310,6 @@ const Post = RestModel.extend({
},
toggleBookmark() {
let bookmarkedTopic;
this.toggleProperty("bookmarked");
if (this.bookmarked && !this.get("topic.bookmarked")) {
this.set("topic.bookmarked", true);
bookmarkedTopic = true;
}
// need to wait to hear back from server (stuff may not be loaded)
return Post.updateBookmark(this.id, this.bookmarked)
.then(result => {
this.set("topic.bookmarked", result.topic_bookmarked);
this.appEvents.trigger("page:bookmark-post-toggled", this);
})
.catch(error => {
this.toggleProperty("bookmarked");
if (bookmarkedTopic) {
this.set("topic.bookmarked", false);
}
throw new Error(error);
});
},
toggleBookmarkWithReminder() {
return new Promise(resolve => {
let controller = showModal("bookmark", {
model: {
@ -357,7 +331,7 @@ const Post = RestModel.extend({
afterSave: savedData => {
this.setProperties({
"topic.bookmarked": true,
bookmarked_with_reminder: true,
bookmarked: true,
bookmark_reminder_at: savedData.reminderAt,
bookmark_reminder_type: savedData.reminderType,
bookmark_name: savedData.name,
@ -373,7 +347,7 @@ const Post = RestModel.extend({
bookmark_reminder_type: null,
bookmark_name: null,
bookmark_id: null,
bookmarked_with_reminder: false
bookmarked: false
});
this.appEvents.trigger("page:bookmark-post-toggled", this);
}

View File

@ -400,7 +400,6 @@ const Topic = RestModel.extend({
afterTopicBookmarked(firstPost) {
if (firstPost) {
firstPost.set("bookmarked", true);
firstPost.set("bookmarked_with_reminder", true);
this.set("bookmark_reminder_at", firstPost.bookmark_reminder_at);
return [firstPost.id];
}
@ -439,7 +438,7 @@ const Topic = RestModel.extend({
return this.firstPost().then(firstPost => {
const toggleBookmarkOnServer = () => {
if (bookmark) {
return firstPost.toggleBookmarkWithReminder().then(() => {
return firstPost.toggleBookmark().then(() => {
this.set("bookmarking", false);
return this.afterTopicBookmarked(firstPost);
});
@ -449,7 +448,7 @@ const Topic = RestModel.extend({
this.toggleProperty("bookmarked");
this.set("bookmark_reminder_at", null);
let clearedBookmarkProps = {
bookmarked_with_reminder: false,
bookmarked: false,
bookmark_id: null,
bookmark_name: null,
bookmark_reminder_at: null
@ -458,10 +457,6 @@ const Topic = RestModel.extend({
const updated = [];
posts.forEach(post => {
if (post.bookmarked) {
post.set("bookmarked", false);
updated.push(post.id);
}
if (post.bookmarked_with_reminder) {
post.setProperties(clearedBookmarkProps);
updated.push(post.id);
}
@ -477,11 +472,7 @@ const Topic = RestModel.extend({
const unbookmarkedPosts = [];
if (!bookmark && posts) {
posts.forEach(
post =>
(post.bookmarked || post.bookmarked_with_reminder) &&
unbookmarkedPosts.push(post)
);
posts.forEach(post => post.bookmarked && unbookmarkedPosts.push(post));
}
return new Promise(resolve => {

View File

@ -225,7 +225,6 @@
recoverPost=(action "recoverPost")
expandHidden=(action "expandHidden")
toggleBookmark=(action "toggleBookmark")
toggleBookmarkWithReminder=(action "toggleBookmarkWithReminder")
togglePostType=(action "togglePostType")
rebakePost=(action "rebakePost")
changePostOwner=(action "changePostOwner")

View File

@ -287,31 +287,11 @@ registerButton("bookmark", attrs => {
return;
}
let className = "bookmark";
if (attrs.bookmarked) {
className += " bookmarked";
}
return {
id: attrs.bookmarked ? "unbookmark" : "bookmark",
action: "toggleBookmark",
title: attrs.bookmarked ? "bookmarks.created" : "bookmarks.not_bookmarked",
className,
icon: "bookmark"
};
});
registerButton("bookmarkWithReminder", attrs => {
if (!attrs.canBookmark) {
return;
}
let classNames = ["bookmark", "with-reminder"];
let title = "bookmarks.not_bookmarked";
let titleOptions = {};
if (attrs.bookmarkedWithReminder) {
if (attrs.bookmarked) {
classNames.push("bookmarked");
if (attrs.bookmarkReminderAt) {
@ -331,8 +311,8 @@ registerButton("bookmarkWithReminder", attrs => {
}
return {
id: attrs.bookmarkedWithReminder ? "unbookmark" : "bookmark",
action: "toggleBookmarkWithReminder",
id: attrs.bookmarked ? "unbookmark" : "bookmark",
action: "toggleBookmark",
title,
titleOptions,
className: classNames.join(" "),
@ -451,10 +431,7 @@ export default createWidget("post-menu", {
const hiddenSetting = siteSettings.post_menu_hidden_items || "";
const hiddenButtons = hiddenSetting
.split("|")
.filter(s => !attrs.bookmarked || s !== "bookmark")
.filter(
s => !attrs.bookmarkedWithReminder || s !== "bookmarkWithReminder"
);
.filter(s => !attrs.bookmarked || s !== "bookmark");
if (currentUser && keyValueStore) {
const likedPostId = keyValueStore.getInt("likedPostId");
@ -468,12 +445,7 @@ export default createWidget("post-menu", {
let visibleButtons = [];
// filter menu items based on site settings
const orderedButtons = this.menuItems().filter(button => {
if (button === "bookmark") {
return false;
}
return true;
});
const orderedButtons = this.menuItems();
// If the post is a wiki, make Edit more prominent
if (attrs.wiki && attrs.canEdit) {

View File

@ -49,7 +49,6 @@ class PostSerializer < BasicPostSerializer
:user_title,
:reply_to_user,
:bookmarked,
:bookmarked_with_reminder,
:bookmark_reminder_at,
:bookmark_id,
:bookmark_reminder_type,
@ -316,32 +315,24 @@ class PostSerializer < BasicPostSerializer
true
end
def bookmarked_with_reminder
true
end
def include_bookmarked?
(actions.present? && actions.keys.include?(PostActionType.types[:bookmark]))
end
def include_bookmarked_with_reminder?
post_bookmark.present?
end
def include_bookmark_reminder_at?
include_bookmarked_with_reminder?
include_bookmarked?
end
def include_bookmark_reminder_type?
include_bookmarked_with_reminder?
include_bookmarked?
end
def include_bookmark_name?
include_bookmarked_with_reminder?
include_bookmarked?
end
def include_bookmark_id?
include_bookmarked_with_reminder?
include_bookmarked?
end
def post_bookmark

View File

@ -189,7 +189,7 @@ basic:
post_menu:
client: true
type: list
default: "read|like|share|flag|edit|bookmark|bookmarkWithReminder|delete|admin|reply"
default: "read|like|share|flag|edit|bookmark|delete|admin|reply"
allow_any: false
choices:
- read
@ -201,11 +201,10 @@ basic:
- bookmark
- admin
- reply
- bookmarkWithReminder
post_menu_hidden_items:
client: true
type: list
default: "flag|bookmark|bookmarkWithReminder|edit|delete|admin"
default: "flag|bookmark|edit|delete|admin"
allow_any: false
choices:
- like
@ -216,7 +215,6 @@ basic:
- bookmark
- admin
- reply
- bookmarkWithReminder
share_links:
client: true
type: list

View File

@ -0,0 +1,28 @@
# frozen_string_literal: true
class RemoveBookmarksWithReminderPostMenuItem < ActiveRecord::Migration[6.0]
def up
execute <<~SQL
UPDATE site_settings SET value = REPLACE(value, '|bookmarkWithReminder|', '|') WHERE name = 'post_menu';
SQL
execute <<~SQL
UPDATE site_settings SET value = REPLACE(value, 'bookmarkWithReminder|', '') WHERE name = 'post_menu';
SQL
execute <<~SQL
UPDATE site_settings SET value = REPLACE(value, '|bookmarkWithReminder', '') WHERE name = 'post_menu';
SQL
execute <<~SQL
UPDATE site_settings SET value = REPLACE(value, '|bookmarkWithReminder|', '|') WHERE name = 'post_menu_hidden';
SQL
execute <<~SQL
UPDATE site_settings SET value = REPLACE(value, 'bookmarkWithReminder|', '') WHERE name = 'post_menu_hidden';
SQL
execute <<~SQL
UPDATE site_settings SET value = REPLACE(value, '|bookmarkWithReminder', '') WHERE name = 'post_menu_hidden';
SQL
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -14,19 +14,19 @@ function initialize(api) {
});
api.modifyClass("model:post", {
toggleBookmarkWithReminder() {
toggleBookmark() {
// if we are talking to discobot then any bookmarks should just
// be created without reminder options, to streamline the new user
// narrative.
const discobotUserId = -2;
if (this.user_id === discobotUserId && !this.bookmarked_with_reminder) {
if (this.user_id === discobotUserId && !this.bookmarked) {
return ajax("/bookmarks", {
type: "POST",
data: { post_id: this.id }
}).then(response => {
this.setProperties({
"topic.bookmarked": true,
bookmarked_with_reminder: true,
bookmarked: true,
bookmark_id: response.id
});
this.appEvents.trigger("post-stream:refresh", { id: this.id });

View File

@ -235,28 +235,12 @@ describe PostSerializer do
s
end
context "when a user post action for the bookmark exists" do
before do
PostActionCreator.create(current_user, post, :bookmark)
end
it "returns true" do
expect(serialized.as_json[:bookmarked]).to eq(true)
end
end
context "when a user post action for the bookmark does not exist" do
it "does not return the bookmarked attribute" do
expect(serialized.as_json.key?(:bookmarked)).to eq(false)
end
end
context "when a Bookmark record exists for the user on the post" do
let!(:bookmark) { Fabricate(:bookmark_next_business_day_reminder, user: current_user, post: post) }
context "bookmarks with reminders" do
it "returns true" do
expect(serialized.as_json[:bookmarked_with_reminder]).to eq(true)
expect(serialized.as_json[:bookmarked]).to eq(true)
end
it "returns the reminder_at for the bookmark" do
@ -266,8 +250,8 @@ describe PostSerializer do
context "if topic_view is blank" do
let(:topic_view) { nil }
it "does not return the bookmarked_with_reminder attribute" do
expect(serialized.as_json.key?(:bookmarked_with_reminder)).to eq(false)
it "does not return the bookmarked attribute" do
expect(serialized.as_json.key?(:bookmarked)).to eq(false)
end
end
end

View File

@ -13,10 +13,8 @@ Discourse.SiteSettingsOriginal = {
ga_universal_tracking_code: "",
ga_universal_domain_name: "auto",
top_menu: "latest|new|unread|categories|top",
post_menu:
"like|share|flag|edit|bookmark|bookmarkWithReminder|delete|admin|reply",
post_menu_hidden_items:
"flag|bookmark|bookmarkWithReminder|edit|delete|admin",
post_menu: "like|share|flag|edit|bookmark|delete|admin|reply",
post_menu_hidden_items: "flag|bookmark|edit|delete|admin",
share_links: "twitter|facebook|email",
category_colors:
"BF1E2E|F1592A|F7941D|9EB83B|3AB54A|12A89D|25AAE2|0E76BD|652D90|92278F|ED207B|8C6238|231F20|27AA5B|B3B5B4|E45735",

View File

@ -532,15 +532,12 @@ widgetTest("can't bookmark", {
widgetTest("bookmark", {
template:
'{{mount-widget widget="post" args=args toggleBookmarkWithReminder=(action "toggleBookmarkWithReminder")}}',
'{{mount-widget widget="post" args=args toggleBookmark=(action "toggleBookmark")}}',
beforeEach() {
const args = { canBookmark: true };
this.set("args", args);
this.on(
"toggleBookmarkWithReminder",
() => (args.bookmarked_with_reminder = true)
);
this.on("toggleBookmark", () => (args.bookmarked = true));
},
async test(assert) {
assert.equal(find(".post-menu-area .bookmark").length, 1);