diff --git a/app/assets/javascripts/discourse/controllers/keyboard-shortcuts-help.js b/app/assets/javascripts/discourse/controllers/keyboard-shortcuts-help.js
index a0d42ac0618fa0..2b0df878828356 100644
--- a/app/assets/javascripts/discourse/controllers/keyboard-shortcuts-help.js
+++ b/app/assets/javascripts/discourse/controllers/keyboard-shortcuts-help.js
@@ -1,6 +1,5 @@
import Controller from "@ember/controller";
import ModalFunctionality from "discourse/mixins/modal-functionality";
-import { setting } from "discourse/lib/computed";
const KEY = "keyboard_shortcuts_help";
@@ -57,8 +56,6 @@ export default Controller.extend(ModalFunctionality, {
this.set("shortcuts", null);
},
- showBookmarkShortcuts: setting("enable_bookmarks_with_reminders"),
-
_defineShortcuts() {
this.set("shortcuts", {
jump_to: {
diff --git a/app/assets/javascripts/discourse/controllers/topic.js b/app/assets/javascripts/discourse/controllers/topic.js
index eea22c7d88bff8..b934c13543a427 100644
--- a/app/assets/javascripts/discourse/controllers/topic.js
+++ b/app/assets/javascripts/discourse/controllers/topic.js
@@ -652,10 +652,7 @@ export default Controller.extend(bufferedProperty("model"), {
if (!this.currentUser) {
return bootbox.alert(I18n.t("bookmarks.not_bookmarked"));
} else if (post) {
- if (this.siteSettings.enable_bookmarks_with_reminders) {
- return post.toggleBookmarkWithReminder();
- }
- return post.toggleBookmark().catch(popupAjaxError);
+ return post.toggleBookmarkWithReminder();
} else {
return this.model.toggleBookmark().then(changedIds => {
if (!changedIds) {
diff --git a/app/assets/javascripts/discourse/controllers/user-activity.js b/app/assets/javascripts/discourse/controllers/user-activity.js
index 03828c439b2d48..12333fcf074d7a 100644
--- a/app/assets/javascripts/discourse/controllers/user-activity.js
+++ b/app/assets/javascripts/discourse/controllers/user-activity.js
@@ -3,7 +3,6 @@ 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(),
@@ -12,7 +11,6 @@ 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/topic.js b/app/assets/javascripts/discourse/models/topic.js
index 6ad8de27ec1ca4..1cbab38942ff14 100644
--- a/app/assets/javascripts/discourse/models/topic.js
+++ b/app/assets/javascripts/discourse/models/topic.js
@@ -400,10 +400,8 @@ const Topic = RestModel.extend({
afterTopicBookmarked(firstPost) {
if (firstPost) {
firstPost.set("bookmarked", true);
- if (this.siteSettings.enable_bookmarks_with_reminders) {
- this.set("bookmark_reminder_at", firstPost.bookmark_reminder_at);
- firstPost.set("bookmarked_with_reminder", true);
- }
+ firstPost.set("bookmarked_with_reminder", true);
+ this.set("bookmark_reminder_at", firstPost.bookmark_reminder_at);
return [firstPost.id];
}
},
@@ -438,24 +436,10 @@ const Topic = RestModel.extend({
return this.firstPost().then(firstPost => {
const toggleBookmarkOnServer = () => {
if (bookmark) {
- if (this.siteSettings.enable_bookmarks_with_reminders) {
- return firstPost.toggleBookmarkWithReminder().then(response => {
- this.set("bookmarking", false);
- if (response && response.closedWithoutSaving) {
- this.set("bookmarked", false);
- } else {
- return this.afterTopicBookmarked(firstPost);
- }
- });
- } else {
- return ajax(`/t/${this.id}/bookmark`, { type: "PUT" })
- .then(() => {
- this.toggleProperty("bookmarked");
- return this.afterTopicBookmarked(firstPost);
- })
- .catch(popupAjaxError)
- .finally(() => this.set("bookmarking", false));
- }
+ return firstPost.toggleBookmarkWithReminder().then(() => {
+ this.set("bookmarking", false);
+ return this.afterTopicBookmarked(firstPost);
+ });
} else {
return ajax(`/t/${this.id}/remove_bookmarks`, { type: "PUT" })
.then(() => {
@@ -474,10 +458,7 @@ const Topic = RestModel.extend({
post.set("bookmarked", false);
updated.push(post.id);
}
- if (
- this.siteSettings.enable_bookmarks_with_reminders &&
- post.bookmarked_with_reminder
- ) {
+ if (post.bookmarked_with_reminder) {
post.setProperties(clearedBookmarkProps);
updated.push(post.id);
}
diff --git a/app/assets/javascripts/discourse/routes/discovery.js b/app/assets/javascripts/discourse/routes/discovery.js
index 1dd2985c46e8d0..f7bd4a5a06fdd6 100644
--- a/app/assets/javascripts/discourse/routes/discovery.js
+++ b/app/assets/javascripts/discourse/routes/discovery.js
@@ -17,10 +17,7 @@ export default DiscourseRoute.extend(OpenComposer, {
// 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"
- ) {
+ if (url === "/bookmarks") {
this.transitionTo(
"userActivity.bookmarksWithReminders",
this.currentUser
diff --git a/app/assets/javascripts/discourse/templates/modal/keyboard-shortcuts-help.hbs b/app/assets/javascripts/discourse/templates/modal/keyboard-shortcuts-help.hbs
index 5299b52f3702ea..813ab3a1f65568 100644
--- a/app/assets/javascripts/discourse/templates/modal/keyboard-shortcuts-help.hbs
+++ b/app/assets/javascripts/discourse/templates/modal/keyboard-shortcuts-help.hbs
@@ -55,24 +55,22 @@
{{html-safe shortcuts.actions.quote_post}}
- {{#if showBookmarkShortcuts}}
-
- {{i18n "keyboard_shortcuts_help.bookmarks.title"}}
-
- - {{html-safe shortcuts.bookmarks.enter}}
- - {{html-safe shortcuts.bookmarks.later_today}}
- - {{html-safe shortcuts.bookmarks.later_this_week}}
- - {{html-safe shortcuts.bookmarks.tomorrow}}
- - {{html-safe shortcuts.bookmarks.next_week}}
- - {{html-safe shortcuts.bookmarks.next_month}}
- - {{html-safe shortcuts.bookmarks.next_business_week}}
- - {{html-safe shortcuts.bookmarks.next_business_day}}
- - {{html-safe shortcuts.bookmarks.custom}}
- - {{html-safe shortcuts.bookmarks.none}}
- - {{html-safe shortcuts.bookmarks.delete}}
-
-
- {{/if}}
+
+ {{i18n "keyboard_shortcuts_help.bookmarks.title"}}
+
+ - {{html-safe shortcuts.bookmarks.enter}}
+ - {{html-safe shortcuts.bookmarks.later_today}}
+ - {{html-safe shortcuts.bookmarks.later_this_week}}
+ - {{html-safe shortcuts.bookmarks.tomorrow}}
+ - {{html-safe shortcuts.bookmarks.next_week}}
+ - {{html-safe shortcuts.bookmarks.next_month}}
+ - {{html-safe shortcuts.bookmarks.next_business_week}}
+ - {{html-safe shortcuts.bookmarks.next_business_day}}
+ - {{html-safe shortcuts.bookmarks.custom}}
+ - {{html-safe shortcuts.bookmarks.none}}
+ - {{html-safe shortcuts.bookmarks.delete}}
+
+
diff --git a/app/assets/javascripts/discourse/templates/user/activity.hbs b/app/assets/javascripts/discourse/templates/user/activity.hbs
index 15ac59b652c73a..15b868def222d1 100644
--- a/app/assets/javascripts/discourse/templates/user/activity.hbs
+++ b/app/assets/javascripts/discourse/templates/user/activity.hbs
@@ -18,15 +18,9 @@
{{#link-to "userActivity.likesGiven"}}{{i18n "user_action_groups.1"}}{{/link-to}}
{{#if user.showBookmarks}}
- {{#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}}
+
+ {{#link-to 'userActivity.bookmarksWithReminders'}}{{i18n 'user_action_groups.3'}}{{/link-to}}
+
{{/if}}
{{plugin-outlet
name="user-activity-bottom"
diff --git a/app/assets/javascripts/discourse/templates/user/summary.hbs b/app/assets/javascripts/discourse/templates/user/summary.hbs
index ddd89595c52535..ff9c7520f907ee 100644
--- a/app/assets/javascripts/discourse/templates/user/summary.hbs
+++ b/app/assets/javascripts/discourse/templates/user/summary.hbs
@@ -27,7 +27,7 @@
{{#if model.bookmark_count}}
- {{#link-to "userActivity.bookmarks"}}
+ {{#link-to "userActivity.bookmarksWithReminders"}}
{{user-stat value=model.bookmark_count label="user.summary.bookmark_count"}}
{{/link-to}}
diff --git a/app/assets/javascripts/discourse/widgets/post-menu.js b/app/assets/javascripts/discourse/widgets/post-menu.js
index c8374058ee4d4d..98046266bdda48 100644
--- a/app/assets/javascripts/discourse/widgets/post-menu.js
+++ b/app/assets/javascripts/discourse/widgets/post-menu.js
@@ -302,8 +302,8 @@ registerButton("bookmark", attrs => {
};
});
-registerButton("bookmarkWithReminder", (attrs, state, siteSettings) => {
- if (!attrs.canBookmark || !siteSettings.enable_bookmarks_with_reminders) {
+registerButton("bookmarkWithReminder", attrs => {
+ if (!attrs.canBookmark) {
return;
}
@@ -469,10 +469,7 @@ export default createWidget("post-menu", {
// filter menu items based on site settings
const orderedButtons = this.menuItems().filter(button => {
- if (
- this.siteSettings.enable_bookmarks_with_reminders &&
- button === "bookmark"
- ) {
+ if (button === "bookmark") {
return false;
}
return true;
diff --git a/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js b/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js
index 7bbae631984160..787fce384d90c8 100644
--- a/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js
+++ b/app/assets/javascripts/discourse/widgets/quick-access-bookmarks.js
@@ -16,11 +16,7 @@ createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", {
},
showAllHref() {
- if (this.siteSettings.enable_bookmarks_with_reminders) {
- return `${this.attrs.path}/activity/bookmarks-with-reminders`;
- } else {
- return `${this.attrs.path}/activity/bookmarks`;
- }
+ return `${this.attrs.path}/activity/bookmarks-with-reminders`;
},
emptyStatePlaceholderItem() {
@@ -28,11 +24,7 @@ createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", {
},
findNewItems() {
- if (this.siteSettings.enable_bookmarks_with_reminders) {
- return this.loadBookmarksWithReminders();
- } else {
- return this.loadUserActivityBookmarks();
- }
+ return this.loadBookmarksWithReminders();
},
itemHtml(bookmark) {
diff --git a/app/assets/javascripts/discourse/widgets/user-menu.js b/app/assets/javascripts/discourse/widgets/user-menu.js
index 11987297266000..08792c4130c083 100644
--- a/app/assets/javascripts/discourse/widgets/user-menu.js
+++ b/app/assets/javascripts/discourse/widgets/user-menu.js
@@ -56,16 +56,13 @@ createWidget("user-menu-links", {
},
bookmarksGlyph() {
- let path = this.siteSettings.enable_bookmarks_with_reminders
- ? "bookmarks-with-reminders"
- : "bookmarks";
return {
action: UserMenuAction.QUICK_ACCESS,
actionParam: QuickAccess.BOOKMARKS,
label: "user.bookmarks",
className: "user-bookmarks-link",
icon: "bookmark",
- href: `${this.attrs.path}/activity/${path}`
+ href: `${this.attrs.path}/activity/bookmarks-with-reminders`
};
},
diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index 50c4ead17f29d9..c440d630f550b5 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -488,26 +488,6 @@ def notice
render body: nil
end
- def bookmark
- if params[:bookmarked] == "true"
- post = find_post_from_params
- result = PostActionCreator.create(current_user, post, :bookmark)
- return render_json_error(result) if result.failed?
- else
- post_action = PostAction.find_by(post_id: params[:post_id], user_id: current_user.id)
- raise Discourse::NotFound unless post_action
-
- post = Post.with_deleted.find_by(id: post_action&.post_id)
- raise Discourse::NotFound unless post
-
- result = PostActionDestroyer.destroy(current_user, post, :bookmark)
- return render_json_error(result) if result.failed?
- end
-
- topic_user = TopicUser.get(post.topic, current_user)
- render_json_dump(topic_bookmarked: topic_user.try(:bookmarked))
- end
-
def destroy_bookmark
params.require(:post_id)
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index fe399566d2cf85..e0ecde628f7a75 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -97,9 +97,9 @@ def show
Notification.remove_for(current_user.id, params[:topic_id]) if current_user
deleted = guardian.can_see_topic?(ex.obj, false) ||
- (!guardian.can_see_topic?(ex.obj) &&
- ex.obj&.access_topic_via_group &&
- ex.obj.deleted_at)
+ (!guardian.can_see_topic?(ex.obj) &&
+ ex.obj&.access_topic_via_group &&
+ ex.obj.deleted_at)
if SiteSetting.detailed_404
if deleted
@@ -231,11 +231,14 @@ def posts
fetch_topic_view(options)
- render_json_dump(TopicViewPostsSerializer.new(@topic_view,
- scope: guardian,
- root: false,
- include_raw: !!params[:include_raw]
- ))
+ render_json_dump(
+ TopicViewPostsSerializer.new(
+ @topic_view,
+ scope: guardian,
+ root: false,
+ include_raw: !!params[:include_raw]
+ )
+ )
end
def excerpts
@@ -261,12 +264,12 @@ def excerpts
.joins("LEFT JOIN users u on u.id = posts.user_id")
.pluck(:id, :cooked, :username)
.map do |post_id, cooked, username|
- {
- post_id: post_id,
- username: username,
- excerpt: PrettyText.excerpt(cooked, 800, keep_emoji_images: true)
- }
- end
+ {
+ post_id: post_id,
+ username: username,
+ excerpt: PrettyText.excerpt(cooked, 800, keep_emoji_images: true)
+ }
+ end
render json: @posts.to_json
end
@@ -490,18 +493,7 @@ def remove_banner
def remove_bookmarks
topic = Topic.find(params[:topic_id].to_i)
-
- if SiteSetting.enable_bookmarks_with_reminders?
- BookmarkManager.new(current_user).destroy_for_topic(topic)
- else
- PostAction.joins(:post)
- .where(user_id: current_user.id)
- .where('topic_id = ?', topic.id).each do |pa|
-
- PostActionDestroyer.destroy(current_user, pa.post, :bookmark)
- end
- end
-
+ BookmarkManager.new(current_user).destroy_for_topic(topic)
render body: nil
end
@@ -552,16 +544,11 @@ def bookmark
topic = Topic.find(params[:topic_id].to_i)
first_post = topic.ordered_posts.first
- if SiteSetting.enable_bookmarks_with_reminders?
- bookmark_manager = BookmarkManager.new(current_user)
- bookmark_manager.create(post_id: first_post.id)
+ bookmark_manager = BookmarkManager.new(current_user)
+ bookmark_manager.create(post_id: first_post.id)
- if bookmark_manager.errors.any?
- return render_json_error(bookmark_manager, status: 400)
- end
- else
- result = PostActionCreator.create(current_user, first_post, :bookmark)
- return render_json_error(result) if result.failed?
+ if bookmark_manager.errors.any?
+ return render_json_error(bookmark_manager, status: 400)
end
render body: nil
@@ -628,9 +615,9 @@ def invite_group
topic = Topic.find_by(id: params[:topic_id])
unless pm_has_slots?(topic)
- return render_json_error(I18n.t("pm_reached_recipients_limit",
- recipients_limit: SiteSetting.max_allowed_message_recipients
- ))
+ return render_json_error(
+ I18n.t("pm_reached_recipients_limit", recipients_limit: SiteSetting.max_allowed_message_recipients)
+ )
end
if topic.private_message?
@@ -654,9 +641,9 @@ def invite
)
unless pm_has_slots?(topic)
- return render_json_error(I18n.t("pm_reached_recipients_limit",
- recipients_limit: SiteSetting.max_allowed_message_recipients
- ))
+ return render_json_error(
+ I18n.t("pm_reached_recipients_limit", recipients_limit: SiteSetting.max_allowed_message_recipients)
+ )
end
guardian.ensure_can_invite_to!(topic)
@@ -683,7 +670,8 @@ def invite
if group_names.present?
json.merge!(errors: [
- I18n.t("topic_invite.failed_to_invite",
+ I18n.t(
+ "topic_invite.failed_to_invite",
group_names: group_names
)
])
@@ -1011,7 +999,8 @@ def perform_show_response
return
end
- topic_view_serializer = TopicViewSerializer.new(@topic_view,
+ topic_view_serializer = TopicViewSerializer.new(
+ @topic_view,
scope: guardian,
root: false,
include_raw: !!params[:include_raw]
diff --git a/app/jobs/scheduled/bookmark_reminder_notifications.rb b/app/jobs/scheduled/bookmark_reminder_notifications.rb
index 20ce9cb8a8ed9d..991e0d23c1deb6 100644
--- a/app/jobs/scheduled/bookmark_reminder_notifications.rb
+++ b/app/jobs/scheduled/bookmark_reminder_notifications.rb
@@ -21,8 +21,6 @@ def self.max_reminder_notifications_per_run=(max)
end
def execute(args = nil)
- return if !SiteSetting.enable_bookmarks_with_reminders?
-
bookmarks = Bookmark.pending_reminders
.where.not(reminder_type: Bookmark.reminder_types[:at_desktop])
.includes(:user).order('reminder_at ASC')
diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb
index f34e58c69585d1..2dc9733e6cb226 100644
--- a/app/models/bookmark.rb
+++ b/app/models/bookmark.rb
@@ -72,6 +72,16 @@ def self.reminder_types
later_this_week: 8
)
end
+
+ def self.count_per_day(opts = nil)
+ opts ||= {}
+ result = where('bookmarks.created_at >= ?', opts[:start_date] || (opts[:since_days_ago] || 30).days.ago)
+ result = result.where('bookmarks.created_at <= ?', opts[:end_date]) if opts[:end_date]
+ result = result.joins(:topic).merge(Topic.in_category_and_subcategories(opts[:category_id])) if opts[:category_id]
+ result.group('date(bookmarks.created_at)')
+ .order('date(bookmarks.created_at)')
+ .count
+ end
end
# == Schema Information
diff --git a/app/models/reports/bookmarks.rb b/app/models/reports/bookmarks.rb
index f4728e1f58fac6..0844efb33d2008 100644
--- a/app/models/reports/bookmarks.rb
+++ b/app/models/reports/bookmarks.rb
@@ -3,5 +3,16 @@
Report.add_report('bookmarks') do |report|
report.icon = 'bookmark'
- post_action_report report, PostActionType.types[:bookmark]
+ category_filter = report.filters.dig(:category)
+ report.add_filter('category', default: category_filter)
+
+ report.data = []
+ Bookmark.count_per_day(
+ category_id: category_filter,
+ start_date: report.start_date,
+ end_date: report.end_date
+ ).each do |date, count|
+ report.data << { x: date, y: count }
+ end
+ add_counts report, Bookmark, 'bookmarks.created_at'
end
diff --git a/app/models/user_summary.rb b/app/models/user_summary.rb
index 52c919957738a4..435702d67b339e 100644
--- a/app/models/user_summary.rb
+++ b/app/models/user_summary.rb
@@ -117,14 +117,7 @@ def user_stat
end
def bookmark_count
- if SiteSetting.enable_bookmarks_with_reminders
- Bookmark.where(user: @user).count
- else
- UserAction
- .where(user: @user)
- .where(action_type: UserAction::BOOKMARK)
- .count
- end
+ Bookmark.where(user: @user).count
end
def recent_time_read
diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb
index ff880f14c385c8..7bf90b98ebdfca 100644
--- a/app/serializers/post_serializer.rb
+++ b/app/serializers/post_serializer.rb
@@ -345,7 +345,7 @@ def include_bookmark_id?
end
def post_bookmark
- return nil if !SiteSetting.enable_bookmarks_with_reminders? || @topic_view.blank?
+ return nil if @topic_view.blank?
@post_bookmark ||= @topic_view.user_post_bookmarks.find { |bookmark| bookmark.post_id == object.id }
end
diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb
index 4bace8d9edcfef..e40dca1d9fb3c7 100644
--- a/app/serializers/topic_view_serializer.rb
+++ b/app/serializers/topic_view_serializer.rb
@@ -186,15 +186,11 @@ def include_expandable_first_post?
end
def bookmarked
- if SiteSetting.enable_bookmarks_with_reminders?
- object.has_bookmarks?
- else
- object.topic_user&.bookmarked
- end
+ object.has_bookmarks?
end
def include_bookmark_reminder_at?
- SiteSetting.enable_bookmarks_with_reminders? && bookmarked
+ bookmarked
end
def bookmark_reminder_at
diff --git a/config/routes.rb b/config/routes.rb
index 2fc21b435ee807..9be2b3f44da1cc 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -585,7 +585,6 @@
put "admin/groups/:id/members" => "groups#add_members", constraints: AdminConstraint.new
resources :posts do
- put "bookmark"
delete "bookmark", to: "posts#destroy_bookmark"
put "wiki"
put "post_type"
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 0828616904e6b0..986fc3ba38c434 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -291,7 +291,7 @@ basic:
default: false
enable_bookmarks_with_reminders:
client: true
- default: false
+ default: true
hidden: true
enable_bookmark_at_desktop_reminders:
client: true
diff --git a/db/migrate/20200409033412_create_bookmarks_from_post_action_bookmarks.rb b/db/migrate/20200409033412_create_bookmarks_from_post_action_bookmarks.rb
new file mode 100644
index 00000000000000..092a0e7b99246d
--- /dev/null
+++ b/db/migrate/20200409033412_create_bookmarks_from_post_action_bookmarks.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+class CreateBookmarksFromPostActionBookmarks < ActiveRecord::Migration[6.0]
+ def up
+ SiteSetting.enable_bookmarks_with_reminders = true
+
+ bookmarks_to_create = []
+ loop do
+ # post action type id 1 is :bookmark. we do not need to OFFSET here for
+ # paging because the WHERE bookmarks.id IS NULL clause handles this effectively,
+ # because we do not get bookmarks back that have already been inserted
+ post_action_bookmarks = DB.query(
+ <<~SQL, type_id: 1
+ SELECT post_actions.id, post_actions.post_id, posts.topic_id, post_actions.user_id
+ FROM post_actions
+ INNER JOIN posts ON posts.id = post_actions.post_id
+ LEFT JOIN bookmarks ON bookmarks.post_id = post_actions.post_id AND bookmarks.user_id = post_actions.user_id
+ INNER JOIN topics ON topics.id = posts.topic_id
+ WHERE bookmarks.id IS NULL AND post_action_type_id = :type_id AND post_actions.deleted_at IS NULL AND posts.deleted_at IS NULL
+ LIMIT 2000
+ SQL
+ )
+ break if post_action_bookmarks.count.zero?
+
+ post_action_bookmarks.each do |pab|
+ now = Time.zone.now
+ bookmarks_to_create << "(#{pab.topic_id}, #{pab.post_id}, #{pab.user_id}, '#{now}', '#{now}')"
+ end
+
+ create_bookmarks(bookmarks_to_create)
+ bookmarks_to_create = []
+ end # loop
+ end
+
+ def down
+ raise ActiveRecord::IrreversibleMigration
+ end
+
+ def create_bookmarks(bookmarks_to_create)
+ return if bookmarks_to_create.empty?
+
+ # this will ignore conflicts in the bookmarks table so
+ # if the user already has a post bookmarked in the new way,
+ # then we don't error and keep on truckin'
+ #
+ # we shouldn't have duplicates here at any rate because of
+ # the above LEFT JOIN but best to be safe knowing this
+ # won't blow up
+ #
+ DB.exec(
+ <<~SQL
+ INSERT INTO bookmarks (topic_id, post_id, user_id, created_at, updated_at)
+ VALUES #{bookmarks_to_create.join(",\n")}
+ ON CONFLICT DO NOTHING
+ SQL
+ )
+ end
+end
diff --git a/lib/bookmark_reminder_notification_handler.rb b/lib/bookmark_reminder_notification_handler.rb
index 4591719fdc0fd2..3354be39494965 100644
--- a/lib/bookmark_reminder_notification_handler.rb
+++ b/lib/bookmark_reminder_notification_handler.rb
@@ -52,8 +52,6 @@ def self.cache_pending_at_desktop_reminder(user)
end
def self.send_at_desktop_reminder(user:, request_user_agent:)
- return if !SiteSetting.enable_bookmarks_with_reminders
-
return if MobileDetection.mobile_device?(request_user_agent)
return if !user_has_pending_at_desktop_reminders?(user)
diff --git a/lib/search.rb b/lib/search.rb
index bcbf5107e92552..ead773d097caa0 100644
--- a/lib/search.rb
+++ b/lib/search.rb
@@ -381,11 +381,7 @@ def post_action_type_filter(posts, post_action_type)
advanced_filter(/^in:(bookmarks)$/) do |posts, match|
if @guardian.user
- if SiteSetting.enable_bookmarks_with_reminders
- posts.where("posts.id IN (SELECT post_id FROM bookmarks WHERE bookmarks.user_id = #{@guardian.user.id})")
- else
- post_action_type_filter(posts, PostActionType.types[:bookmark])
- end
+ posts.where("posts.id IN (SELECT post_id FROM bookmarks WHERE bookmarks.user_id = #{@guardian.user.id})")
end
end
diff --git a/lib/tasks/bookmarks.rake b/lib/tasks/bookmarks.rake
index 833a5f80152643..d3cf545d2d08a5 100644
--- a/lib/tasks/bookmarks.rake
+++ b/lib/tasks/bookmarks.rake
@@ -9,58 +9,40 @@ require_dependency "rake_helpers"
# You can provide a sync_limit for a smaller batch run.
#
desc "migrates old PostAction bookmarks to the new Bookmark model & table"
-task "bookmarks:sync_to_table", [:sync_limit] => :environment do |_t, args|
- sync_limit = args[:sync_limit] || 0
- post_action_bookmarks = PostAction
- .select('post_actions.id', 'post_actions.post_id', 'posts.topic_id', 'post_actions.user_id')
- .where(post_action_type_id: PostActionType.types[:bookmark])
- .joins(:post)
- .where(deleted_at: nil)
- .joins('LEFT JOIN bookmarks ON bookmarks.post_id = post_actions.post_id AND bookmarks.user_id = post_actions.user_id')
- .joins('INNER JOIN topics ON topics.id = posts.topic_id')
- .where('bookmarks.id IS NULL')
-
- # if sync_limit is provided as an argument this will cap
- # the number of bookmarks that will be created in a run of
- # this task (for huge bookmark count communities)
- if sync_limit > 0
- post_action_bookmarks = post_action_bookmarks.limit(sync_limit)
- end
-
- post_action_bookmark_count = post_action_bookmarks.count('post_actions.id')
- i = 0
-
+task "bookmarks:sync_to_table" => :environment do |_t, args|
bookmarks_to_create = []
- post_action_bookmarks.find_each(batch_size: 2000) do |pab|
- RakeHelpers.print_status_with_label("Creating post new bookmarks.......", i, post_action_bookmark_count)
- now = Time.zone.now
- bookmarks_to_create << {
- topic_id: pab.topic_id,
- post_id: pab.post_id,
- user_id: pab.user_id,
- created_at: now,
- updated_at: now
- }
-
- i += 1
-
- # once we get to 2000 records to create, insert them all and reset
- # the array and counter to make sure we don't have too many in memory
- if i >= 2000
- create_bookmarks(bookmarks_to_create)
- i = 0
- bookmarks_to_create = []
+ loop do
+ # post action type id 1 is :bookmark. we do not need to OFFSET here for
+ # paging because the WHERE bookmarks.id IS NULL clause handles this effectively,
+ # because we do not get bookmarks back that have already been inserted
+ post_action_bookmarks = DB.query(
+ <<~SQL, type_id: 1
+ SELECT post_actions.id, post_actions.post_id, posts.topic_id, post_actions.user_id
+ FROM post_actions
+ INNER JOIN posts ON posts.id = post_actions.post_id
+ LEFT JOIN bookmarks ON bookmarks.post_id = post_actions.post_id AND bookmarks.user_id = post_actions.user_id
+ INNER JOIN topics ON topics.id = posts.topic_id
+ WHERE bookmarks.id IS NULL AND post_action_type_id = :type_id AND post_actions.deleted_at IS NULL AND posts.deleted_at IS NULL
+ LIMIT 2000
+ SQL
+ )
+ break if post_action_bookmarks.count.zero?
+
+ post_action_bookmarks.each do |pab|
+ now = Time.zone.now
+ bookmarks_to_create << "(#{pab.topic_id}, #{pab.post_id}, #{pab.user_id}, '#{now}', '#{now}')"
end
- end
- # if there was < 2000 bookmarks this finishes off the last ones
- create_bookmarks(bookmarks_to_create)
+ create_bookmarks(bookmarks_to_create)
+ bookmarks_to_create = []
+ end # loop
- RakeHelpers.print_status_with_label("Bookmark creation complete! ", i, post_action_bookmark_count)
- puts ""
+ puts "Bookmark creation complete!"
end
def create_bookmarks(bookmarks_to_create)
+ return if bookmarks_to_create.empty?
+
# this will ignore conflicts in the bookmarks table so
# if the user already has a post bookmarked in the new way,
# then we don't error and keep on truckin'
@@ -68,5 +50,12 @@ def create_bookmarks(bookmarks_to_create)
# we shouldn't have duplicates here at any rate because of
# the above LEFT JOIN but best to be safe knowing this
# won't blow up
- Bookmark.insert_all(bookmarks_to_create)
+ #
+ DB.exec(
+ <<~SQL
+ INSERT INTO bookmarks (topic_id, post_id, user_id, created_at, updated_at)
+ VALUES #{bookmarks_to_create.join(",\n")}
+ ON CONFLICT DO NOTHING
+ SQL
+ )
end
diff --git a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/new_user_narrative.rb b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/new_user_narrative.rb
index 89afd4eb8702f0..072c54beafb1cb 100644
--- a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/new_user_narrative.rb
+++ b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/new_user_narrative.rb
@@ -238,11 +238,7 @@ def reply_to_bookmark
return unless @post.user_id == self.discobot_user.id
profile_page_url = url_helpers(:user_url, username: @user.username)
- bookmark_url = if SiteSetting.enable_bookmarks_with_reminders?
- "#{profile_page_url}/activity/bookmarks-with-reminders"
- else
- "#{profile_page_url}/activity/bookmarks"
- end
+ bookmark_url = "#{profile_page_url}/activity/bookmarks-with-reminders"
raw = <<~RAW
#{I18n.t("#{I18N_KEY}.bookmark.reply", i18n_post_args(bookmark_url: bookmark_url))}
diff --git a/plugins/discourse-narrative-bot/plugin.rb b/plugins/discourse-narrative-bot/plugin.rb
index 3acbcc7a9204f1..29766f640e8de3 100644
--- a/plugins/discourse-narrative-bot/plugin.rb
+++ b/plugins/discourse-narrative-bot/plugin.rb
@@ -241,10 +241,8 @@ def fetch_avatar_url(user)
end
self.add_model_callback(Bookmark, :after_commit, on: :create) do
- if SiteSetting.enable_bookmarks_with_reminders?
- if self.post && self.user.enqueue_narrative_bot_job?
- Jobs.enqueue(:bot_input, user_id: self.user_id, post_id: self.post_id, input: :bookmark)
- end
+ if self.post && self.user.enqueue_narrative_bot_job?
+ Jobs.enqueue(:bot_input, user_id: self.user_id, post_id: self.post_id, input: :bookmark)
end
end
diff --git a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb
index 43ff1ac52a5886..d875de3b10094c 100644
--- a/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb
+++ b/plugins/discourse-narrative-bot/spec/discourse_narrative_bot/new_user_narrative_spec.rb
@@ -247,26 +247,7 @@
end
end
- it 'should create the right reply' do
- post.update!(user: discobot_user)
- narrative.expects(:enqueue_timeout_job).with(user)
-
- narrative.input(:bookmark, user, post: post)
- new_post = Post.last
- profile_page_url = "#{Discourse.base_url}/u/#{user.username}"
-
- expected_raw = <<~RAW
- #{I18n.t('discourse_narrative_bot.new_user_narrative.bookmark.reply', bookmark_url: "#{profile_page_url}/activity/bookmarks", base_uri: '')}
-
- #{I18n.t('discourse_narrative_bot.new_user_narrative.onebox.instructions', base_uri: '')}
- RAW
-
- expect(new_post.raw).to eq(expected_raw.chomp)
- expect(narrative.get_data(user)[:state].to_sym).to eq(:tutorial_onebox)
- end
-
it 'should create the right reply when bookmarks with reminders are enabled' do
- SiteSetting.enable_bookmarks_with_reminders = true
post.update!(user: discobot_user)
narrative.expects(:enqueue_timeout_job).with(user)
diff --git a/spec/jobs/bookmark_reminder_notifications_spec.rb b/spec/jobs/bookmark_reminder_notifications_spec.rb
index 92ae10002065ff..58f72625036f94 100644
--- a/spec/jobs/bookmark_reminder_notifications_spec.rb
+++ b/spec/jobs/bookmark_reminder_notifications_spec.rb
@@ -18,7 +18,6 @@
end
before do
- SiteSetting.enable_bookmarks_with_reminders = true
# this is done to avoid model validations on Bookmark
bookmark1.update_column(:reminder_at, five_minutes_ago - 10.minutes)
bookmark2.update_column(:reminder_at, five_minutes_ago - 5.minutes)
@@ -63,14 +62,6 @@
expect(Discourse.redis.get(described_class::JOB_RUN_NUMBER_KEY)).to eq('1')
end
- context "when the bookmark with reminder site setting is disabled" do
- it "does nothing" do
- Bookmark.expects(:where).never
- SiteSetting.enable_bookmarks_with_reminders = false
- subject.execute
- end
- end
-
context "when one of the bookmark reminder types is at_desktop" do
let(:bookmark1) { Fabricate(:bookmark, reminder_type: :at_desktop) }
it "is not included in the run, because it is not time-based" do
diff --git a/spec/lib/bookmark_reminder_notification_handler_spec.rb b/spec/lib/bookmark_reminder_notification_handler_spec.rb
index 8cc1c4b63790d7..318a52429aad9e 100644
--- a/spec/lib/bookmark_reminder_notification_handler_spec.rb
+++ b/spec/lib/bookmark_reminder_notification_handler_spec.rb
@@ -8,7 +8,6 @@
fab!(:user) { Fabricate(:user) }
before do
- SiteSetting.enable_bookmarks_with_reminders = true
Discourse.redis.flushall
end
@@ -89,17 +88,6 @@
end
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
- end
-
def send_reminder
subject.send_at_desktop_reminder(user: user, request_user_agent: user_agent)
end
diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb
index cb654a08c6cbdc..984297dadb88ff 100644
--- a/spec/requests/admin/users_controller_spec.rb
+++ b/spec/requests/admin/users_controller_spec.rb
@@ -124,11 +124,11 @@
end
describe '#suspend' do
- fab!(:post) { Fabricate(:post) }
+ fab!(:created_post) { Fabricate(:post) }
let(:suspend_params) do
{ suspend_until: 5.hours.from_now,
reason: "because of this post",
- post_id: post.id }
+ post_id: created_post.id }
end
it "works properly" do
@@ -156,13 +156,13 @@
expect(response.status).to eq(200)
log = UserHistory.where(target_user_id: user.id).order('id desc').first
- expect(log.post_id).to eq(post.id)
+ expect(log.post_id).to eq(created_post.id)
end
it "can delete an associated post" do
put "/admin/users/#{user.id}/suspend.json", params: suspend_params.merge(post_action: 'delete')
- post.reload
- expect(post.deleted_at).to be_present
+ created_post.reload
+ expect(created_post.deleted_at).to be_present
expect(response.status).to eq(200)
end
@@ -200,17 +200,17 @@
reply = PostCreator.create(
Fabricate(:user),
raw: 'this is the reply text',
- reply_to_post_number: post.post_number,
- topic_id: post.topic_id
+ reply_to_post_number: created_post.post_number,
+ topic_id: created_post.topic_id
)
nested_reply = PostCreator.create(
Fabricate(:user),
raw: 'this is the reply text2',
reply_to_post_number: reply.post_number,
- topic_id: post.topic_id
+ topic_id: created_post.topic_id
)
put "/admin/users/#{user.id}/suspend.json", params: suspend_params.merge(post_action: 'delete_replies')
- expect(post.reload.deleted_at).to be_present
+ expect(created_post.reload.deleted_at).to be_present
expect(reply.reload.deleted_at).to be_present
expect(nested_reply.reload.deleted_at).to be_present
expect(response.status).to eq(200)
@@ -223,9 +223,9 @@
)
expect(response.status).to eq(200)
- post.reload
- expect(post.deleted_at).to be_blank
- expect(post.raw).to eq("this is the edited content")
+ created_post.reload
+ expect(created_post.deleted_at).to be_blank
+ expect(created_post.raw).to eq("this is the edited content")
expect(response.status).to eq(200)
end
end
@@ -252,9 +252,8 @@
it "also prevents use of any api keys" do
api_key = Fabricate(:api_key, user: user)
-
- put "/posts/#{Fabricate(:post).id}/bookmark.json", params: {
- bookmarked: "true"
+ post "/bookmarks.json", params: {
+ post_id: Fabricate(:post).id
}, headers: { HTTP_API_KEY: api_key.key }
expect(response.status).to eq(200)
@@ -264,10 +263,9 @@
user.reload
expect(user).to be_suspended
- put "/posts/#{Fabricate(:post).id}/bookmark.json", params: {
- bookmarked: "true",
- api_key: api_key.key
- }
+ post "/bookmarks.json", params: {
+ post_id: Fabricate(:post).id
+ }, headers: { HTTP_API_KEY: api_key.key }
expect(response.status).to eq(403)
end
end
diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb
index 09cefae4a0b560..af25d9c00066a5 100644
--- a/spec/requests/posts_controller_spec.rb
+++ b/spec/requests/posts_controller_spec.rb
@@ -482,131 +482,6 @@
end
end
- describe '#bookmark' do
- include_examples 'action requires login', :put, "/posts/2/bookmark.json"
- let!(:post) { post_by_user }
-
- describe 'when logged in' do
- before do
- sign_in(user)
- end
-
- fab!(:private_message) { Fabricate(:private_message_post) }
-
- it "raises an error if the user doesn't have permission to see the post" do
- put "/posts/#{private_message.id}/bookmark.json", params: { bookmarked: "true" }
- expect(response).to be_forbidden
- end
-
- it 'creates a bookmark' do
- put "/posts/#{post.id}/bookmark.json", params: { bookmarked: "true" }
- expect(response.status).to eq(200)
-
- post_action = PostAction.find_by(user: user, post: post)
- expect(post_action.post_action_type_id).to eq(PostActionType.types[:bookmark])
- end
-
- context "removing a bookmark" do
- let(:post_action) { PostActionCreator.create(user, post, :bookmark).post_action }
-
- it "returns the right response when post is not bookmarked" do
- put "/posts/#{post_by_user.id}/bookmark.json"
- expect(response.status).to eq(404)
- end
-
- it "should be able to remove a bookmark" do
- post_action
- put "/posts/#{post.id}/bookmark.json"
-
- expect(PostAction.find_by(id: post_action.id)).to eq(nil)
- end
-
- describe "when user doesn't have permission to see bookmarked post" do
- it "should still be able to remove a bookmark" do
- post_action
- post = post_action.post
- topic = post.topic
- topic.convert_to_private_message(admin)
- topic.remove_allowed_user(admin, user.username)
-
- expect(Guardian.new(user).can_see_post?(post.reload)).to eq(false)
-
- put "/posts/#{post.id}/bookmark.json"
-
- expect(PostAction.find_by(id: post_action.id)).to eq(nil)
- end
- end
-
- describe "when post has been deleted" do
- it "should still be able to remove a bookmark" do
- post = post_action.post
- post.trash!
-
- put "/posts/#{post.id}/bookmark.json"
-
- expect(PostAction.find_by(id: post_action.id)).to eq(nil)
- end
- end
- end
- end
-
- context "api" do
- let(:api_key) { Fabricate(:api_key, user: user) }
- let(:master_key) { Fabricate(:api_key, user: nil) }
-
- # choosing an arbitrarily easy to mock trusted activity
- it 'allows users with api key to bookmark posts' do
- put "/posts/#{post.id}/bookmark.json",
- params: { bookmarked: "true" },
- headers: { HTTP_API_KEY: api_key.key }
-
- expect(response.status).to eq(200)
- expect(PostAction.where(
- post: post,
- user: user,
- post_action_type_id: PostActionType.types[:bookmark]
- ).count).to eq(1)
- end
-
- it 'raises an error with a user key that does not match an optionally specified username' do
- put "/posts/#{post.id}/bookmark.json",
- params: { bookmarked: "true" },
- headers: { HTTP_API_KEY: api_key.key, HTTP_API_USERNAME: 'made_up' }
-
- expect(response.status).to eq(403)
- end
-
- it 'allows users with a master api key to bookmark posts' do
- put "/posts/#{post.id}/bookmark.json",
- params: { bookmarked: "true" },
- headers: { HTTP_API_KEY: master_key.key, HTTP_API_USERNAME: user.username }
-
- expect(response.status).to eq(200)
- expect(PostAction.where(
- post: post,
- user: user,
- post_action_type_id: PostActionType.types[:bookmark]
- ).count).to eq(1)
- end
-
- it 'disallows phonies to bookmark posts' do
- put "/posts/#{post.id}/bookmark.json",
- params: { bookmarked: "true" },
- headers: { HTTP_API_KEY: SecureRandom.hex(32), HTTP_API_USERNAME: user.username }
-
- expect(response.status).to eq(403)
- end
-
- it 'disallows blank api' do
- put "/posts/#{post.id}/bookmark.json",
- params: { bookmarked: "true" },
- headers: { HTTP_API_KEY: "", HTTP_API_USERNAME: user.username }
-
- expect(response.status).to eq(403)
- end
- end
- end
-
describe '#wiki' do
include_examples "action requires login", :put, "/posts/2/wiki.json"
diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb
index 751c8bf88b8f77..e48a2f94830421 100644
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@@ -2346,19 +2346,15 @@ def headers(locale)
describe '#remove_bookmarks' do
it "should remove bookmarks properly from non first post" do
- bookmark = PostActionType.types[:bookmark]
sign_in(user)
post = create_post
post2 = create_post(topic_id: post.topic_id)
-
- PostActionCreator.new(user, post2, bookmark).perform
-
- put "/t/#{post.topic_id}/bookmark.json"
- expect(PostAction.where(user_id: user.id, post_action_type: bookmark).count).to eq(2)
+ Fabricate(:bookmark, user: user, post: post)
+ Fabricate(:bookmark, user: user, post: post2)
put "/t/#{post.topic_id}/remove_bookmarks.json"
- expect(PostAction.where(user_id: user.id, post_action_type: bookmark).count).to eq(0)
+ expect(Bookmark.where(user: user).count).to eq(0)
end
it "should disallow bookmarks on posts you have no access to" do
@@ -2369,10 +2365,7 @@ def headers(locale)
expect(response).to be_forbidden
end
- context "when SiteSetting.enable_bookmarks_with_reminders is true" do
- before do
- SiteSetting.enable_bookmarks_with_reminders = true
- end
+ context "bookmarks with reminders" do
it "deletes all the bookmarks for the user in the topic" do
sign_in(user)
post = create_post
@@ -2388,26 +2381,23 @@ def headers(locale)
sign_in(user)
end
- it "should create a new post action for the bookmark on the first post of the topic" do
+ it "should create a new bookmark on the first post of the topic" do
post = create_post
post2 = create_post(topic_id: post.topic_id)
put "/t/#{post.topic_id}/bookmark.json"
- expect(PostAction.find_by(user_id: user.id, post_action_type: PostActionType.types[:bookmark]).post_id).to eq(post.id)
+ expect(Bookmark.find_by(user_id: user.id).post_id).to eq(post.id)
end
it "errors if the topic is already bookmarked for the user" do
post = create_post
- PostActionCreator.new(user, post, PostActionType.types[:bookmark]).perform
+ Bookmark.create(post: post, user: user, topic: post.topic)
put "/t/#{post.topic_id}/bookmark.json"
- expect(response).to be_forbidden
+ expect(response.status).to eq(400)
end
- context "when SiteSetting.enable_bookmarks_with_reminders is true" do
- before do
- SiteSetting.enable_bookmarks_with_reminders = true
- end
+ context "bookmarks with reminders" do
it "should create a new bookmark on the first post of the topic" do
post = create_post
post2 = create_post(topic_id: post.topic_id)
diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb
index a438a06ea9df08..8791db56c6ef20 100644
--- a/spec/serializers/post_serializer_spec.rb
+++ b/spec/serializers/post_serializer_spec.rb
@@ -254,11 +254,7 @@ def json_for_user(user)
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 "when the site setting for bookmarks with reminders is enabled" do
- before do
- SiteSetting.enable_bookmarks_with_reminders = true
- end
-
+ context "bookmarks with reminders" do
it "returns true" do
expect(serialized.as_json[:bookmarked_with_reminder]).to eq(true)
end
@@ -275,16 +271,6 @@ def json_for_user(user)
end
end
end
-
- context "when the site setting for bookmarks with reminders is disabled" do
- it "does not return the bookmarked_with_reminder attribute" do
- expect(serialized.as_json.key?(:bookmarked_with_reminder)).to eq(false)
- end
-
- it "does not return the bookmark_reminder_at attribute" do
- expect(serialized.as_json.key?(:bookmark_reminder_at)).to eq(false)
- end
- end
end
end
diff --git a/spec/tasks/bookmarks_spec.rb b/spec/tasks/bookmarks_spec.rb
index 91249c59f89626..c7b2c65bf6cc31 100644
--- a/spec/tasks/bookmarks_spec.rb
+++ b/spec/tasks/bookmarks_spec.rb
@@ -38,11 +38,6 @@ def invoke_task(args = nil)
expect(Bookmark.where(post: post1, user: user1).count).to eq(1)
end
- it "respects the sync_limit if provided and stops creating bookmarks at the limit (so this can be run progrssively" do
- invoke_task(1)
- expect(Bookmark.all.count).to eq(1)
- end
-
it "skips post actions where the post topic no longer exists and does not error" do
post1.topic.delete
post1.reload
diff --git a/test/javascripts/acceptance/topic-test.js b/test/javascripts/acceptance/topic-test.js
index b5e2684e6f2237..b7edc5cc956794 100644
--- a/test/javascripts/acceptance/topic-test.js
+++ b/test/javascripts/acceptance/topic-test.js
@@ -397,10 +397,7 @@ QUnit.test(
);
acceptance("Topic + Post Bookmarks with Reminders", {
- loggedIn: true,
- settings: {
- enable_bookmarks_with_reminders: true
- }
+ loggedIn: true
});
QUnit.test("Bookmarks Modal", async assert => {
diff --git a/test/javascripts/controllers/preferences-account-test.js b/test/javascripts/controllers/preferences-account-test.js
index f953fb0332ba3d..f79642e2bd3064 100644
--- a/test/javascripts/controllers/preferences-account-test.js
+++ b/test/javascripts/controllers/preferences-account-test.js
@@ -12,7 +12,7 @@ QUnit.test("updating of associated accounts", function(assert) {
is_anonymous: true
}),
currentUser: EmberObject.create({
- id: 1234,
+ id: 1234
}),
site: EmberObject.create({
isMobileDevice: false
diff --git a/test/javascripts/fixtures/user_fixtures.js b/test/javascripts/fixtures/user_fixtures.js
index f6a2f870503aa3..b22180109da03d 100644
--- a/test/javascripts/fixtures/user_fixtures.js
+++ b/test/javascripts/fixtures/user_fixtures.js
@@ -397,6 +397,34 @@ export default {
featured_user_badge_ids: [5870, 40673, 5868]
}
},
+ "/u/eviltrout/bookmarks.json": {
+ user_bookmark_list: {
+ bookmarks: [
+ {
+ excerpt: "Here this is my new topic where I yell.",
+ tags: [],
+ id: 576,
+ created_at: "2020-04-07T05:30:40.446Z",
+ topic_id: 119,
+ linked_post_number: 1,
+ post_id: 281,
+ name: "test",
+ reminder_at: null,
+ title: "Yelling topic title :/",
+ deleted: false,
+ hidden: false,
+ category_id: 1,
+ closed: false,
+ archived: false,
+ archetype: "regular",
+ highest_post_number: 5,
+ bumped_at: "2020-04-06T05:20:00.172Z",
+ slug: "yelling-topic-title",
+ username: "someguy"
+ }
+ ]
+ }
+ },
"/user_actions.json": {
user_actions: [
{
diff --git a/test/javascripts/helpers/site-settings.js b/test/javascripts/helpers/site-settings.js
index c39424b0dfc079..bf55374eb85371 100644
--- a/test/javascripts/helpers/site-settings.js
+++ b/test/javascripts/helpers/site-settings.js
@@ -31,7 +31,6 @@ Discourse.SiteSettingsOriginal = {
allow_new_registrations: true,
enable_google_logins: true,
enable_google_oauth2_logins: false,
- enable_bookmarks_with_reminders: false,
enable_twitter_logins: true,
enable_facebook_logins: true,
enable_github_logins: true,
diff --git a/test/javascripts/widgets/post-test.js b/test/javascripts/widgets/post-test.js
index 7c19c715e68842..b7131fc2a5b801 100644
--- a/test/javascripts/widgets/post-test.js
+++ b/test/javascripts/widgets/post-test.js
@@ -532,19 +532,19 @@ widgetTest("can't bookmark", {
widgetTest("bookmark", {
template:
- '{{mount-widget widget="post" args=args toggleBookmark=(action "toggleBookmark")}}',
+ '{{mount-widget widget="post" args=args toggleBookmarkWithReminder=(action "toggleBookmarkWithReminder")}}',
beforeEach() {
const args = { canBookmark: true };
this.set("args", args);
- this.on("toggleBookmark", () => (args.bookmarked = true));
+ this.on(
+ "toggleBookmarkWithReminder",
+ () => (args.bookmarked_with_reminder = true)
+ );
},
async test(assert) {
assert.equal(find(".post-menu-area .bookmark").length, 1);
assert.equal(find("button.bookmarked").length, 0);
-
- await click("button.bookmark");
- assert.equal(find("button.bookmarked").length, 1);
}
});
diff --git a/test/javascripts/widgets/user-menu-test.js b/test/javascripts/widgets/user-menu-test.js
index 35d73a8542315e..39f2a337220021 100644
--- a/test/javascripts/widgets/user-menu-test.js
+++ b/test/javascripts/widgets/user-menu-test.js
@@ -145,11 +145,9 @@ widgetTest("bookmarks", {
const bookmark = find(".quick-access-panel li a")[0];
assert.ok(bookmark);
+ assert.ok(bookmark.href.includes("/t/yelling-topic-title/119"));
assert.ok(
- bookmark.href.includes("/t/how-to-check-the-user-level-via-ajax/11993")
- );
- assert.ok(
- bookmark.innerHTML.includes("Abhishek_Gupta"),
+ bookmark.innerHTML.includes("someguy"),
"should include the last poster's username"
);
assert.ok(