diff --git a/app/assets/javascripts/discourse/app/controllers/bookmark.js b/app/assets/javascripts/discourse/app/controllers/bookmark.js index 38ce4c76ad7..992c3228a02 100644 --- a/app/assets/javascripts/discourse/app/controllers/bookmark.js +++ b/app/assets/javascripts/discourse/app/controllers/bookmark.js @@ -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: diff --git a/app/assets/javascripts/discourse/app/lib/bookmark.js b/app/assets/javascripts/discourse/app/lib/bookmark.js index 8874034e9a2..ebad3713138 100644 --- a/app/assets/javascripts/discourse/app/lib/bookmark.js +++ b/app/assets/javascripts/discourse/app/lib/bookmark.js @@ -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", diff --git a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs index 821ffe19765..2cac81e9b65 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/bookmark.hbs @@ -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")}} -
{{i18n "bookmarks.reminders.at_desktop"}}
- {{/tap-tile}} - {{/if}} - {{#if showLaterToday}} {{#tap-tile icon="far-moon" tileId=reminderTypes.LATER_TODAY activeTile=grid.activeTile onChange=(action "selectReminderType")}}
{{i18n "bookmarks.reminders.later_today"}}
diff --git a/app/assets/javascripts/discourse/app/widgets/post-menu.js b/app/assets/javascripts/discourse/app/widgets/post-menu.js index b02253a8e1c..376772c237c 100644 --- a/app/assets/javascripts/discourse/app/widgets/post-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/post-menu.js @@ -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"; } diff --git a/app/jobs/scheduled/bookmark_reminder_notifications.rb b/app/jobs/scheduled/bookmark_reminder_notifications.rb index 4dca47c1f6c..cb7b016074a 100644 --- a/app/jobs/scheduled/bookmark_reminder_notifications.rb +++ b/app/jobs/scheduled/bookmark_reminder_notifications.rb @@ -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 diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb index 2dc9733e6cb..3cfae4fe174 100644 --- a/app/models/bookmark.rb +++ b/app/models/bookmark.rb @@ -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, diff --git a/config/locales/server.de.yml b/config/locales/server.de.yml index 9efbe743196..1d9b104de18 100644 --- a/config/locales/server.de.yml +++ b/config/locales/server.de.yml @@ -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" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index bb7863aa189..f49bb379796 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -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" diff --git a/config/locales/server.es.yml b/config/locales/server.es.yml index 1c4e406b111..cfb25c56942 100644 --- a/config/locales/server.es.yml +++ b/config/locales/server.es.yml @@ -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" diff --git a/config/locales/server.fi.yml b/config/locales/server.fi.yml index 73a9acd1dce..7fbbb3a1a8a 100644 --- a/config/locales/server.fi.yml +++ b/config/locales/server.fi.yml @@ -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" diff --git a/config/locales/server.he.yml b/config/locales/server.he.yml index f8caa6b2bb0..314d2d907c0 100644 --- a/config/locales/server.he.yml +++ b/config/locales/server.he.yml @@ -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: "בהמשך היום" diff --git a/config/locales/server.it.yml b/config/locales/server.it.yml index fb079d67886..e1de9665192 100644 --- a/config/locales/server.it.yml +++ b/config/locales/server.it.yml @@ -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" diff --git a/config/locales/server.ko.yml b/config/locales/server.ko.yml index ae56e149629..dc8ec6212a4 100644 --- a/config/locales/server.ko.yml +++ b/config/locales/server.ko.yml @@ -302,7 +302,7 @@ ko: 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: "오늘 늦게" diff --git a/config/locales/server.nl.yml b/config/locales/server.nl.yml index 647ec1fcc02..34ffb7558dd 100644 --- a/config/locales/server.nl.yml +++ b/config/locales/server.nl.yml @@ -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" diff --git a/config/locales/server.pl_PL.yml b/config/locales/server.pl_PL.yml index 02bb335598f..356901790ad 100644 --- a/config/locales/server.pl_PL.yml +++ b/config/locales/server.pl_PL.yml @@ -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" diff --git a/config/locales/server.ru.yml b/config/locales/server.ru.yml index 1cfe19be31c..92897216731 100644 --- a/config/locales/server.ru.yml +++ b/config/locales/server.ru.yml @@ -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: "Сегодня, но позже" diff --git a/config/locales/server.sv.yml b/config/locales/server.sv.yml index 8cb45527a3d..66860108623 100644 --- a/config/locales/server.sv.yml +++ b/config/locales/server.sv.yml @@ -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" diff --git a/config/locales/server.tr_TR.yml b/config/locales/server.tr_TR.yml index 063fbd4e009..a9c04b6ca18 100644 --- a/config/locales/server.tr_TR.yml +++ b/config/locales/server.tr_TR.yml @@ -324,7 +324,7 @@ tr_TR: bookmarks: errors: already_bookmarked_post: "Aynı gönderiyi iki kez işaretleyemezsiniz." - time_must_be_provided: "'%{reminder_type}' 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" diff --git a/config/locales/server.uk.yml b/config/locales/server.uk.yml index f71315c01eb..92e1287ab54 100644 --- a/config/locales/server.uk.yml +++ b/config/locales/server.uk.yml @@ -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: "Пізніше сьогодні" diff --git a/config/locales/server.zh_CN.yml b/config/locales/server.zh_CN.yml index 845f67887ff..d1f5b7c61fb 100644 --- a/config/locales/server.zh_CN.yml +++ b/config/locales/server.zh_CN.yml @@ -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: "今天的某个时候" diff --git a/config/site_settings.yml b/config/site_settings.yml index edda4aa263e..ea4bbd4b665 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -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 diff --git a/db/migrate/20200506044956_migrate_at_desktop_bookmark_reminders.rb b/db/migrate/20200506044956_migrate_at_desktop_bookmark_reminders.rb new file mode 100644 index 00000000000..add607b31d9 --- /dev/null +++ b/db/migrate/20200506044956_migrate_at_desktop_bookmark_reminders.rb @@ -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 diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index acd9381771c..81217248a5d 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -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 diff --git a/lib/bookmark_manager.rb b/lib/bookmark_manager.rb index 1670a976613..23328021f6a 100644 --- a/lib/bookmark_manager.rb +++ b/lib/bookmark_manager.rb @@ -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 diff --git a/lib/bookmark_reminder_notification_handler.rb b/lib/bookmark_reminder_notification_handler.rb index e129b4ac195..223c55b2ac5 100644 --- a/lib/bookmark_reminder_notification_handler.rb +++ b/lib/bookmark_reminder_notification_handler.rb @@ -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 diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb index e5ce0ced7bf..141382da8cd 100644 --- a/spec/components/auth/default_current_user_provider_spec.rb +++ b/spec/components/auth/default_current_user_provider_spec.rb @@ -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 diff --git a/spec/jobs/bookmark_reminder_notifications_spec.rb b/spec/jobs/bookmark_reminder_notifications_spec.rb index 58f72625036..0bcfa0b3d21 100644 --- a/spec/jobs/bookmark_reminder_notifications_spec.rb +++ b/spec/jobs/bookmark_reminder_notifications_spec.rb @@ -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 diff --git a/spec/lib/bookmark_manager_spec.rb b/spec/lib/bookmark_manager_spec.rb index 34b029b5153..d821e994465 100644 --- a/spec/lib/bookmark_manager_spec.rb +++ b/spec/lib/bookmark_manager_spec.rb @@ -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) diff --git a/spec/lib/bookmark_reminder_notification_handler_spec.rb b/spec/lib/bookmark_reminder_notification_handler_spec.rb index 318a52429aa..d86ce73a5ed 100644 --- a/spec/lib/bookmark_reminder_notification_handler_spec.rb +++ b/spec/lib/bookmark_reminder_notification_handler_spec.rb @@ -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 diff --git a/spec/requests/bookmarks_controller_spec.rb b/spec/requests/bookmarks_controller_spec.rb index 8c29af87073..7c4292c78d0 100644 --- a/spec/requests/bookmarks_controller_spec.rb +++ b/spec/requests/bookmarks_controller_spec.rb @@ -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