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

DEV: Remove all code referencing at_desktop bookmark reminders (#9650) 

We have found no need for these reminder types, so we are removing the code for them.
This commit is contained in:
Martin Brennan 2020-05-06 15:22:43 +10:00 committed by GitHub
parent 4239ca1f8d
commit fa572d3a7a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
30 changed files with 36 additions and 371 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
app/assets/javascripts/discourse/app/controllers/bookmark.js
View File

@ -175,14 +175,6 @@ export default Controller.extend(ModalFunctionality, {
return isPresent(id);
},
@discourseComputed()
showAtDesktop() {
return (
this.siteSettings.enable_bookmark_at_desktop_reminders &&
this.site.mobileView
);
},
@discourseComputed("selectedReminderType")
customDateTimeSelected(selectedReminderType) {
return selectedReminderType === REMINDER_TYPES.CUSTOM;
@ -346,8 +338,6 @@ export default Controller.extend(ModalFunctionality, {
}
switch (this.selectedReminderType) {
case REMINDER_TYPES.AT_DESKTOP:
return null;
case REMINDER_TYPES.LATER_TODAY:
return this.laterToday();
case REMINDER_TYPES.NEXT_BUSINESS_DAY:

1
app/assets/javascripts/discourse/app/lib/bookmark.js
View File

@ -17,7 +17,6 @@ export function formattedReminderTime(reminderAt, timezone) {
}
export const REMINDER_TYPES = {
AT_DESKTOP: "at_desktop",
LATER_TODAY: "later_today",
NEXT_BUSINESS_DAY: "next_business_day",
TOMORROW: "tomorrow",

6
app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs
View File

@ -26,12 +26,6 @@
{{#if userHasTimezoneSet}}
{{#tap-tile-grid activeTile=selectedReminderType as |grid|}}
{{#if showAtDesktop}}
{{#tap-tile icon="desktop" tileId=reminderTypes.AT_DESKTOP activeTile=grid.activeTile onChange=(action "selectReminderType")}}
<div class="tap-tile-title">{{i18n "bookmarks.reminders.at_desktop"}}</div>
{{/tap-tile}}
{{/if}}
{{#if showLaterToday}}
{{#tap-tile icon="far-moon" tileId=reminderTypes.LATER_TODAY activeTile=grid.activeTile onChange=(action "selectReminderType")}}
<div class="tap-tile-title">{{i18n "bookmarks.reminders.later_today"}}</div>

2
app/assets/javascripts/discourse/app/widgets/post-menu.js
View File

@ -306,8 +306,6 @@ registerButton("bookmark", attrs => {
);
title = "bookmarks.created_with_reminder";
titleOptions.date = formattedReminder;
} else if (attrs.bookmarkReminderType === "at_desktop") {
title = "bookmarks.created_with_at_desktop_reminder";
} else {
title = "bookmarks.created";
}

83
app/jobs/scheduled/bookmark_reminder_notifications.rb
View File

@ -6,9 +6,6 @@ module Jobs
# Any leftovers will be caught in the next run, because the reminder_at column
# is set to NULL once a reminder has been sent.
class BookmarkReminderNotifications < ::Jobs::Scheduled
JOB_RUN_NUMBER_KEY ||= 'jobs_bookmark_reminder_notifications_job_run_num'
AT_DESKTOP_CONSISTENCY_RUN_NUMBER ||= 6
every 5.minutes
def self.max_reminder_notifications_per_run
@ -21,88 +18,10 @@ module Jobs
end
def execute(args = nil)
bookmarks = Bookmark.pending_reminders
.where.not(reminder_type: Bookmark.reminder_types[:at_desktop])
.includes(:user).order('reminder_at ASC')
bookmarks = Bookmark.pending_reminders.includes(:user).order('reminder_at ASC')
bookmarks.limit(BookmarkReminderNotifications.max_reminder_notifications_per_run).each do |bookmark|
BookmarkReminderNotificationHandler.send_notification(bookmark)
end
# we only want to ensure the desktop consistency every X runs of this job
# (every 30 mins in this case) so we don't bother redis too much, and the
# at desktop consistency problem should not really happen unless people
# are setting the "at desktop" reminder, going out for milk, and never coming
# back
current_job_run_number = Discourse.redis.get(JOB_RUN_NUMBER_KEY).to_i
if current_job_run_number == AT_DESKTOP_CONSISTENCY_RUN_NUMBER
ensure_at_desktop_consistency
end
increment_job_run_number(current_job_run_number)
end
def increment_job_run_number(current_job_run_number)
if current_job_run_number.zero? || current_job_run_number == AT_DESKTOP_CONSISTENCY_RUN_NUMBER
new_job_run_number = 1
else
new_job_run_number = current_job_run_number + 1
end
Discourse.redis.set(JOB_RUN_NUMBER_KEY, new_job_run_number)
end
def ensure_at_desktop_consistency
pending_at_desktop_bookmark_reminders = \
Bookmark.includes(:user)
.references(:user)
.pending_at_desktop_reminders
.where('users.last_seen_at >= :one_day_ago', one_day_ago: 1.day.ago.utc)
return if pending_at_desktop_bookmark_reminders.count.zero?
unique_users = pending_at_desktop_bookmark_reminders.map(&:user).uniq.map { |u| [u.id, u] }.flatten
unique_users = Hash[*unique_users]
pending_reminders_for_redis_check = unique_users.keys.map do |user_id|
"#{BookmarkReminderNotificationHandler::PENDING_AT_DESKTOP_KEY_PREFIX}#{user_id}"
end
Discourse.redis.mget(pending_reminders_for_redis_check).each.with_index do |value, idx|
next if value.present?
user_id = pending_reminders_for_redis_check[idx][/\d+/].to_i
user = unique_users[user_id]
user_pending_bookmark_reminders = pending_at_desktop_bookmark_reminders.select do |bookmark|
bookmark.user == user
end
user_expired_bookmark_reminders = user_pending_bookmark_reminders.select do |bookmark|
bookmark.reminder_set_at <= expiry_limit_datetime
end
next if user_pending_bookmark_reminders.length == user_expired_bookmark_reminders.length
# only tell the cache-gods that this user has pending "at desktop" reminders
# if they haven't let them all expire before coming back to their desktop
#
# the next time they visit the desktop the reminders will be cleared out once
# the notifications are sent
BookmarkReminderNotificationHandler.cache_pending_at_desktop_reminder(user)
end
clear_expired_at_desktop_reminders
end
def clear_expired_at_desktop_reminders
Bookmark
.pending_at_desktop_reminders
.where('reminder_set_at <= :expiry_limit_datetime', expiry_limit_datetime: expiry_limit_datetime)
.update_all(
reminder_set_at: nil, reminder_type: nil
)
end
def expiry_limit_datetime
BookmarkReminderNotificationHandler::PENDING_AT_DESKTOP_EXPIRY_DAYS.days.ago.utc
end
end
end

13
app/models/bookmark.rb
View File

@ -6,8 +6,8 @@ class Bookmark < ActiveRecord::Base
belongs_to :topic
validates :reminder_at, presence: {
message: I18n.t("bookmarks.errors.time_must_be_provided", reminder_type: I18n.t("bookmarks.reminders.at_desktop")),
if: -> { reminder_type.present? && reminder_type != Bookmark.reminder_types[:at_desktop] }
message: I18n.t("bookmarks.errors.time_must_be_provided"),
if: -> { reminder_type.present? }
}
validate :unique_per_post_for_user
@ -47,21 +47,12 @@ class Bookmark < ActiveRecord::Base
where("reminder_at IS NOT NULL AND reminder_at <= :before_time", before_time: before_time)
end
scope :pending_at_desktop_reminders, ->(before_time = Time.now.utc) do
where("reminder_at IS NULL AND reminder_type = :at_desktop", at_desktop: reminder_types[:at_desktop])
end
scope :pending_reminders_for_user, ->(user) do
pending_reminders.where(user: user)
end
scope :at_desktop_reminders_for_user, ->(user) do
where("reminder_type = :at_desktop AND user_id = :user_id", at_desktop: reminder_types[:at_desktop], user_id: user.id)
end
def self.reminder_types
@reminder_type = Enum.new(
at_desktop: 0,
later_today: 1,
next_business_day: 2,
tomorrow: 3,

2
config/locales/server.de.yml
View File

@ -325,7 +325,7 @@ de:
bookmarks:
errors:
already_bookmarked_post: "Du kannst denselben Beitrag nicht zweimal zu deinen Lesezeichen hinzufügen."
time_must_be_provided: "Zeit muss eingestellt werden für alle Erinnerungen ausser '%{reminder_type}'"
time_must_be_provided: "Zeit muss eingestellt werden für alle Erinnerungen"
reminders:
at_desktop: "Nächstes mal wenn ich an meinem PC bin"
later_today: "Im Laufe des Tages"

2
config/locales/server.en.yml
View File

@ -391,7 +391,7 @@ en:
already_bookmarked_post: "You cannot bookmark the same post twice."
cannot_set_past_reminder: "You cannot set a bookmark reminder in the past."
cannot_set_reminder_in_distant_future: "You cannot set a bookmark reminder more than 10 years in the future."
time_must_be_provided: "time must be provided for all reminders except '%{reminder_type}'"
time_must_be_provided: "time must be provided for all reminders"
reminders:
at_desktop: "Next time I'm at my desktop"

2
config/locales/server.es.yml
View File

@ -335,7 +335,7 @@ es:
already_bookmarked_post: "No puedes guardar la misma publicación en marcadores dos veces."
cannot_set_past_reminder: "No puedes establecer un recordatorio de marcador en el pasado."
cannot_set_reminder_in_distant_future: "No puedes establecer un recordatorio de marcador más de 10 años en el futuro."
time_must_be_provided: "se debe especificar la hora para todos los recordatorios excepto '%{reminder_type}'"
time_must_be_provided: "se debe especificar la hora para todos los recordatorios"
reminders:
at_desktop: "La próxima vez que esté en mi ordenador"
later_today: "Más tarde, hoy"

2
config/locales/server.fi.yml
View File

@ -326,7 +326,7 @@ fi:
already_bookmarked_post: "Samaa viestiä ei voi kirjanmerkitä kahdesti."
cannot_set_past_reminder: "Kirjanmerkkimuistutusta ei voi asettaa menneisyyteen."
cannot_set_reminder_in_distant_future: "Kirjanmerkkimuistutusta ei voi asettaa yli 10 vuoden päähän tulevaisuuteen."
time_must_be_provided: "kaikkiin muistutuksiin paitsi '%{reminder_type}' tarvitaan aika"
time_must_be_provided: "kaikkiin muistutuksiin tarvitaan aika"
reminders:
at_desktop: "Ensi kerralla kun olen työpöytäympäristössäni"
later_today: "Myöhemmin tänään"

2
config/locales/server.he.yml
View File

@ -369,7 +369,7 @@ he:
already_bookmarked_post: "אי אפשר לסמן את אותו הפוסט פעמיים"
cannot_set_past_reminder: "אי אפשר להגדיר תזכורת סימנייה למועד שעבר."
cannot_set_reminder_in_distant_future: "לא ניתן להגדיר תזכורת סימנייה למעבר ל־10 שנים בעתיד."
time_must_be_provided: "צריך לציין זמן לכל התזכורות למעט ‚%{reminder_type}’"
time_must_be_provided: "צריך לציין זמן לכל התזכורות"
reminders:
at_desktop: "בשימוש הבא דרך שולחן העבודה"
later_today: "בהמשך היום"

2
config/locales/server.it.yml
View File

@ -334,7 +334,7 @@ it:
already_bookmarked_post: "Non puoi aggiungere due volte lo stesso messaggio nei segnalibri."
cannot_set_past_reminder: "Non è possibile impostare un promemoria per un segnalibro nel passato."
cannot_set_reminder_in_distant_future: "Non è possibile impostare un promemoria per un segnalibro per più di 10 anni nel futuro."
time_must_be_provided: "l'ora deve essere fornita per tutti i promemoria tranne '%{reminder_type}'"
time_must_be_provided: "l'ora deve essere fornita per tutti i promemoria"
reminders:
at_desktop: "La prossima volta che sarò al mio desktop"
later_today: "Più tardi oggi"

2
config/locales/server.ko.yml
View File

@ -302,7 +302,7 @@ ko:
already_bookmarked_post: "동일한 게시물을 두 번 북마크 할 수 없습니다."
cannot_set_past_reminder: "과거에는 북마크 미리 알림을 설정할 수 없습니다."
cannot_set_reminder_in_distant_future: "앞으로 10 년 이상 책갈피 알림을 설정할 수 없습니다."
time_must_be_provided: "&#39;%{reminder_type}&#39;을 제외한 모든 알림에 시간을 제공해야합니다."
time_must_be_provided: "을 모든 알림에 시간을 제공해야합니다."
reminders:
at_desktop: "다음에 저는 데스크탑에 있습니다"
later_today: "오늘 늦게"

2
config/locales/server.nl.yml
View File

@ -335,7 +335,7 @@ nl:
already_bookmarked_post: "U kunt geen twee bladwijzers voor hetzelfde bericht maken."
cannot_set_past_reminder: "U kunt geen bladwijzerherinnering in het verleden instellen."
cannot_set_reminder_in_distant_future: "U kunt geen bladwijzerherinnering later dan over 10 jaar instellen."
time_must_be_provided: "tijd moet voor alle herinneringen behalve '%{reminder_type}' worden opgegeven"
time_must_be_provided: "tijd moet voor alle herinneringen worden opgegeven"
reminders:
at_desktop: "De volgende keer vanaf mijn computer"
later_today: "Later vandaag"

2
config/locales/server.pl_PL.yml
View File

@ -352,7 +352,7 @@ pl_PL:
already_bookmarked_post: "Nie możesz dodać zakładki do tego samego postu dwa razy."
cannot_set_past_reminder: "W przeszłości nie można ustawić przypomnienia o zakładce."
cannot_set_reminder_in_distant_future: "Nie możesz ustawić przypomnienia o zakładce na więcej niż 10 lat w przyszłości."
time_must_be_provided: "dla wszystkich przypomnień należy przewidzieć czas z wyjątkiem „%{reminder_type}”"
time_must_be_provided: "dla wszystkich przypomnień należy przewidzieć czas"
reminders:
at_desktop: "Następnym razem będę na swoim pulpicie"
later_today: "Później dzisiaj"

2
config/locales/server.ru.yml
View File

@ -359,7 +359,7 @@ ru:
already_bookmarked_post: "Вы не можете дважды добавить сообщение в закладки"
cannot_set_past_reminder: "Нельзя указать уже прошедшую дату напоминания."
cannot_set_reminder_in_distant_future: "Нельзя указать дату напоминания, отличающуюся от текущей более чем на 10 лет. "
time_must_be_provided: "время должно быть установлено для всех напоминаний, кроме '%{reminder_type}'"
time_must_be_provided: "время должно быть установлено для всех напоминаний"
reminders:
at_desktop: "При следующем посещении форума"
later_today: "Сегодня, но позже"

2
config/locales/server.sv.yml
View File

@ -335,7 +335,7 @@ sv:
already_bookmarked_post: "Du kan inte bokmärka samma inlägg två gånger."
cannot_set_past_reminder: "Du kan inte sätta en påminnelse för bokmärke i det förflutna."
cannot_set_reminder_in_distant_future: "Du kan inte sätta en påminnelse för bokmärke mer än 10 år framåt i tiden."
time_must_be_provided: "tid måste tillhandahållas för alla påminnelser utom '%{reminder_type}'"
time_must_be_provided: "tid måste tillhandahållas för alla påminnelser"
reminders:
at_desktop: "Nästa gång är jag vid datorn"
later_today: "Senare idag"

2
config/locales/server.tr_TR.yml
View File

@ -324,7 +324,7 @@ tr_TR:
bookmarks:
errors:
already_bookmarked_post: "Aynı gönderiyi iki kez işaretleyemezsiniz."
time_must_be_provided: "&#39;%{reminder_type}&#39; dışındaki tüm hatırlatıcılara zaman verilmelidir"
time_must_be_provided: "Tüm hatırlatıcılara zaman verilmelidir"
reminders:
at_desktop: "Bir dahaki sefere masaüstümdeyim"
later_today: "Bugün ilerleyen saatlerde"

2
config/locales/server.uk.yml
View File

@ -354,7 +354,7 @@ uk:
bookmarks:
errors:
already_bookmarked_post: "Ви не можете зробити закладку на одну публікацію двічі."
time_must_be_provided: "Для всіх нагадувань повинен бути встановлений час, крім '%{reminder_type}'"
time_must_be_provided: "Для всіх нагадувань повинен бути встановлений час"
reminders:
at_desktop: "Наступного разу на мій робочий стіл"
later_today: "Пізніше сьогодні"

2
config/locales/server.zh_CN.yml
View File

@ -318,7 +318,7 @@ zh_CN:
already_bookmarked_post: "你不能将同一帖子收藏两次。"
cannot_set_past_reminder: "你不能在此主题设置收藏提醒。"
cannot_set_reminder_in_distant_future: "你不能够设置一个大于十年的收藏提醒。"
time_must_be_provided: "必须为除“ %{reminder_type}”以外的所有提醒提供时间"
time_must_be_provided: "必须为除提醒提供时间"
reminders:
at_desktop: "下次我使用桌面设备时"
later_today: "今天的某个时候"

4
config/site_settings.yml
View File

@ -294,10 +294,6 @@ basic:
client: true
default: true
hidden: true
enable_bookmark_at_desktop_reminders:
client: true
default: false
hidden: true
push_notifications_prompt:
default: true
client: true

17
db/migrate/20200506044956_migrate_at_desktop_bookmark_reminders.rb Normal file
View File

@ -0,0 +1,17 @@
# frozen_string_literal: true
class MigrateAtDesktopBookmarkReminders < ActiveRecord::Migration[6.0]
def up
# reminder_type 0 is at_desktop, which is no longer valid
DB.exec(
<<~SQL, now: Time.zone.now
UPDATE bookmarks SET reminder_type = NULL, reminder_at = NULL, updated_at = :now
WHERE reminder_type = 0
SQL
)
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

4
lib/auth/default_current_user_provider.rb
View File

@ -124,10 +124,6 @@ class Auth::DefaultCurrentUserProvider
u.update_last_seen!
u.update_ip_address!(ip)
end
BookmarkReminderNotificationHandler.defer_at_desktop_reminder(
user: u, request_user_agent: @request.user_agent
)
end
@env[CURRENT_USER_KEY] = current_user

13
lib/bookmark_manager.rb
View File

@ -29,7 +29,6 @@ class BookmarkManager
update_topic_user_bookmarked(topic: post.topic, bookmarked: true)
BookmarkReminderNotificationHandler.cache_pending_at_desktop_reminder(@user)
bookmark
end
@ -40,7 +39,6 @@ class BookmarkManager
raise Discourse::InvalidAccess.new if !Guardian.new(@user).can_delete?(bookmark)
bookmark.destroy
clear_at_desktop_cache_if_required
bookmarks_remaining_in_topic = Bookmark.exists?(topic_id: bookmark.topic_id, user: @user)
if !bookmarks_remaining_in_topic
@ -61,8 +59,6 @@ class BookmarkManager
update_topic_user_bookmarked(topic: topic, bookmarked: false)
end
clear_at_desktop_cache_if_required
end
def self.send_reminder_notification(id)
@ -94,15 +90,6 @@ class BookmarkManager
private
def clear_at_desktop_cache_if_required
return if user_has_any_pending_at_desktop_reminders?
Discourse.redis.del(BookmarkReminderNotificationHandler::PENDING_AT_DESKTOP_KEY_PREFIX + @user.id.to_s)
end
def user_has_any_pending_at_desktop_reminders?
Bookmark.at_desktop_reminders_for_user(@user).any?
end
def update_topic_user_bookmarked(topic:, bookmarked:)
TopicUser.change(@user.id, topic, bookmarked: bookmarked)
end

30
lib/bookmark_reminder_notification_handler.rb
View File

@ -1,9 +1,6 @@
# frozen_string_literal: true
class BookmarkReminderNotificationHandler
PENDING_AT_DESKTOP_KEY_PREFIX ||= 'pending_at_desktop_bookmark_reminder_user_'
PENDING_AT_DESKTOP_EXPIRY_DAYS ||= 20
def self.send_notification(bookmark)
return if bookmark.blank?
Bookmark.transaction do
@ -42,31 +39,4 @@ class BookmarkReminderNotificationHandler
}.to_json
)
end
def self.user_has_pending_at_desktop_reminders?(user)
Discourse.redis.exists("#{PENDING_AT_DESKTOP_KEY_PREFIX}#{user.id}")
end
def self.cache_pending_at_desktop_reminder(user)
Discourse.redis.setex("#{PENDING_AT_DESKTOP_KEY_PREFIX}#{user.id}", PENDING_AT_DESKTOP_EXPIRY_DAYS.days, true)
end
def self.send_at_desktop_reminder(user:, request_user_agent:)
return if MobileDetection.mobile_device?(request_user_agent)
return if !user_has_pending_at_desktop_reminders?(user)
DistributedMutex.synchronize("sending_at_desktop_bookmark_reminders_user_#{user.id}") do
Bookmark.at_desktop_reminders_for_user(user).each do |bookmark|
BookmarkReminderNotificationHandler.send_notification(bookmark)
end
Discourse.redis.del("#{PENDING_AT_DESKTOP_KEY_PREFIX}#{user.id}")
end
end
def self.defer_at_desktop_reminder(user:, request_user_agent:)
Scheduler::Defer.later "Sending Desktop Bookmark Reminders" do
send_at_desktop_reminder(user: user, request_user_agent: request_user_agent)
end
end
end

8
spec/components/auth/default_current_user_provider_spec.rb
View File

@ -502,14 +502,6 @@ describe Auth::DefaultCurrentUserProvider do
expect(u.last_seen_at).to eq(nil)
end
end
it "defers any at_desktop bookmark reminders" do
BookmarkReminderNotificationHandler.expects(:defer_at_desktop_reminder).with(
user: user, request_user_agent: 'test'
)
provider2 = provider("/", "HTTP_COOKIE" => "_t=#{unhashed_token}", "HTTP_USER_AGENT" => 'test')
provider2.current_user
end
end
it "should update last seen for non ajax" do

68
spec/jobs/bookmark_reminder_notifications_spec.rb
View File

@ -48,28 +48,6 @@ RSpec.describe Jobs::BookmarkReminderNotifications do
expect(bookmark4.reload.reminder_at).not_to eq(nil)
end
it "increments the job run number correctly and resets to 1 when it reaches 6" do
expect(Discourse.redis.get(described_class::JOB_RUN_NUMBER_KEY)).to eq(nil)
subject.execute
expect(Discourse.redis.get(described_class::JOB_RUN_NUMBER_KEY)).to eq('1')
subject.execute
subject.execute
subject.execute
subject.execute
subject.execute
expect(Discourse.redis.get(described_class::JOB_RUN_NUMBER_KEY)).to eq('6')
subject.execute
expect(Discourse.redis.get(described_class::JOB_RUN_NUMBER_KEY)).to eq('1')
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
BookmarkManager.any_instance.expects(:send_reminder_notification).with(bookmark1.id).never
subject.execute
end
end
context "when the number of notifications exceed max_reminder_notifications_per_run" do
it "does not send them in the current run, but will send them in the next" do
begin
@ -81,50 +59,4 @@ RSpec.describe Jobs::BookmarkReminderNotifications do
end
end
end
context "when this is the 6th run (so every half hour) of this job we need to ensure consistency of at_desktop reminders" do
let(:set_at) { Time.zone.now }
let!(:bookmark) do
Fabricate(
:bookmark,
reminder_type: Bookmark.reminder_types[:at_desktop],
reminder_at: nil,
reminder_set_at: set_at
)
end
before do
Discourse.redis.set(Jobs::BookmarkReminderNotifications::JOB_RUN_NUMBER_KEY, 6)
bookmark.user.update(last_seen_at: Time.zone.now - 1.minute)
end
context "when an at_desktop reminder is not pending in redis for a user who should have one" do
it "puts the pending reminder into redis" do
expect(BookmarkReminderNotificationHandler.user_has_pending_at_desktop_reminders?(bookmark.user)).to eq(false)
subject.execute
expect(BookmarkReminderNotificationHandler.user_has_pending_at_desktop_reminders?(bookmark.user)).to eq(true)
end
context "if the user has not been seen in the past 24 hours" do
before do
bookmark.user.update(last_seen_at: Time.zone.now - 25.hours)
end
it "does not put the pending reminder into redis" do
subject.execute
expect(BookmarkReminderNotificationHandler.user_has_pending_at_desktop_reminders?(bookmark.user)).to eq(false)
end
end
context "if the at_desktop reminder is expired (set over PENDING_AT_DESKTOP_EXPIRY_DAYS days ago)" do
let(:set_at) { Time.zone.now - (BookmarkReminderNotificationHandler::PENDING_AT_DESKTOP_EXPIRY_DAYS + 1).days }
it "does not put the pending reminder into redis, and clears the reminder type/time" do
expect(BookmarkReminderNotificationHandler.user_has_pending_at_desktop_reminders?(bookmark.user)).to eq(false)
subject.execute
expect(BookmarkReminderNotificationHandler.user_has_pending_at_desktop_reminders?(bookmark.user)).to eq(false)
bookmark.reload
expect(bookmark.reminder_set_at).to eq(nil)
expect(bookmark.reminder_last_sent_at).to eq(nil)
expect(bookmark.reminder_type).to eq(nil)
end
end
end
end
end

62
spec/lib/bookmark_manager_spec.rb
View File

@ -42,28 +42,6 @@ RSpec.describe BookmarkManager do
end
end
context "when the reminder type is at_desktop" do
let(:reminder_type) { 'at_desktop' }
let(:reminder_at) { nil }
def create_bookmark
subject.create(post_id: post.id, name: name, reminder_type: reminder_type, reminder_at: reminder_at)
end
it "this is a special case which needs client-side logic and has no reminder_at datetime" do
create_bookmark
bookmark = Bookmark.find_by(user: user)
expect(bookmark.reminder_at).to eq(nil)
expect(bookmark.reminder_type).to eq(Bookmark.reminder_types[:at_desktop])
end
it "sets a redis key for the user so we know they have a pending at_desktop reminder" do
create_bookmark
expect(Discourse.redis.get("pending_at_desktop_bookmark_reminder_user_#{user.id}")).not_to eq(nil)
end
end
context "when the bookmark already exists for the user & post" do
before do
Bookmark.create(post: post, user: user, topic: post.topic)
@ -80,7 +58,7 @@ RSpec.describe BookmarkManager do
it "adds an error to the manager" do
subject.create(post_id: post.id, name: name, reminder_type: reminder_type, reminder_at: reminder_at)
expect(subject.errors.full_messages).to include(
"Reminder at " + I18n.t("bookmarks.errors.time_must_be_provided", reminder_type: I18n.t("bookmarks.reminders.at_desktop"))
"Reminder at " + I18n.t("bookmarks.errors.time_must_be_provided")
)
end
end
@ -157,25 +135,6 @@ RSpec.describe BookmarkManager do
expect { subject.destroy(9999) }.to raise_error(Discourse::NotFound)
end
end
context "if the user has pending at desktop reminders for another bookmark" do
before do
Fabricate(:bookmark, user: user, post: Fabricate(:post), reminder_type: Bookmark.reminder_types[:at_desktop])
BookmarkReminderNotificationHandler.cache_pending_at_desktop_reminder(user)
end
it "does not clear the at bookmark redis key" do
subject.destroy(bookmark.id)
expect(BookmarkReminderNotificationHandler.user_has_pending_at_desktop_reminders?(user)).to eq(true)
end
end
context "if the user has pending at desktop reminders for another bookmark" do
it "does clear the at bookmark redis key" do
BookmarkReminderNotificationHandler.cache_pending_at_desktop_reminder(user)
subject.destroy(bookmark.id)
expect(BookmarkReminderNotificationHandler.user_has_pending_at_desktop_reminders?(user)).to eq(false)
end
end
end
describe ".update" do
@ -241,25 +200,6 @@ RSpec.describe BookmarkManager do
expect(Bookmark.where(user: user2, topic: topic).length).to eq(1)
end
context "if the user has pending at desktop reminders for another bookmark" do
before do
Fabricate(:bookmark, user: user, post: Fabricate(:post), reminder_type: Bookmark.reminder_types[:at_desktop])
BookmarkReminderNotificationHandler.cache_pending_at_desktop_reminder(user)
end
it "does not clear the at bookmark redis key" do
subject.destroy_for_topic(topic)
expect(BookmarkReminderNotificationHandler.user_has_pending_at_desktop_reminders?(user)).to eq(true)
end
end
context "if the user has pending at desktop reminders for another bookmark" do
it "does clear the at bookmark redis key" do
BookmarkReminderNotificationHandler.cache_pending_at_desktop_reminder(user)
subject.destroy_for_topic(topic)
expect(BookmarkReminderNotificationHandler.user_has_pending_at_desktop_reminders?(user)).to eq(false)
end
end
it "updates the topic user bookmarked column to false" do
TopicUser.create(user: user, topic: topic, bookmarked: true)
subject.destroy_for_topic(topic)

56
spec/lib/bookmark_reminder_notification_handler_spec.rb
View File

@ -37,60 +37,4 @@ RSpec.describe BookmarkReminderNotificationHandler do
end
end
end
describe "#send_at_desktop_reminder" do
fab!(:reminder) do
Fabricate(
:bookmark,
user: user,
reminder_type: Bookmark.reminder_types[:at_desktop],
reminder_at: nil,
reminder_set_at: Time.zone.now
)
end
let(:user_agent) { "Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 Safari/537.36" }
context "when there are pending bookmark at desktop reminders" do
before do
described_class.cache_pending_at_desktop_reminder(user)
end
context "when the user agent is for mobile" do
let(:user_agent) { "Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13B143 Safari/601.1" }
it "does not attempt to send any reminders" do
DistributedMutex.expects(:synchronize).never
send_reminder
end
end
context "when the user agent is for desktop" do
let(:user_agent) { "Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 Safari/537.36" }
it "deletes the key in redis" do
send_reminder
expect(described_class.user_has_pending_at_desktop_reminders?(user)).to eq(false)
end
it "sends a notification to the user and clears the reminder_at" do
send_reminder
expect(Notification.where(user: user, notification_type: Notification.types[:bookmark_reminder]).count).to eq(1)
expect(reminder.reload.reminder_type).to eq(nil)
expect(reminder.reload.reminder_last_sent_at).not_to eq(nil)
expect(reminder.reload.reminder_set_at).to eq(nil)
end
end
end
context "when there are no pending bookmark at desktop reminders" do
it "does nothing" do
DistributedMutex.expects(:synchronize).never
send_reminder
end
end
def send_reminder
subject.send_at_desktop_reminder(user: user, request_user_agent: user_agent)
end
end
end

2
spec/requests/bookmarks_controller_spec.rb
View File

@ -40,7 +40,7 @@ describe BookmarksController do
expect(response.status).to eq(400)
expect(JSON.parse(response.body)['errors'].first).to include(
I18n.t("bookmarks.errors.time_must_be_provided", reminder_type: I18n.t("bookmarks.reminders.at_desktop"))
I18n.t("bookmarks.errors.time_must_be_provided")
)
end
end