FEATURE: staff can set a timer to remind them about a topic

This commit is contained in:
Neil Lalonde 2017-05-16 14:49:42 -04:00
parent 2e152f4d39
commit 7821400141
20 changed files with 255 additions and 42 deletions

View File

@ -7,6 +7,7 @@ export const CLOSE_STATUS_TYPE = 'close';
const OPEN_STATUS_TYPE = 'open';
const PUBLISH_TO_CATEGORY_STATUS_TYPE = 'publish_to_category';
const DELETE_STATUS_TYPE = 'delete';
const REMINDER_TYPE = 'reminder';
export default Ember.Controller.extend(ModalFunctionality, {
loading: false,
@ -17,8 +18,9 @@ export default Ember.Controller.extend(ModalFunctionality, {
autoClose: Ember.computed.equal('selection', CLOSE_STATUS_TYPE),
autoDelete: Ember.computed.equal('selection', DELETE_STATUS_TYPE),
publishToCategory: Ember.computed.equal('selection', PUBLISH_TO_CATEGORY_STATUS_TYPE),
reminder: Ember.computed.equal('selection', REMINDER_TYPE),
showTimeOnly: Ember.computed.or('autoOpen', 'autoDelete'),
showTimeOnly: Ember.computed.or('autoOpen', 'autoDelete', 'reminder'),
@computed("model.closed")
timerTypes(closed) {
@ -26,7 +28,8 @@ export default Ember.Controller.extend(ModalFunctionality, {
{ id: CLOSE_STATUS_TYPE, name: I18n.t(closed ? 'topic.temp_open.title' : 'topic.auto_close.title'), },
{ id: OPEN_STATUS_TYPE, name: I18n.t(closed ? 'topic.auto_reopen.title' : 'topic.temp_close.title') },
{ id: PUBLISH_TO_CATEGORY_STATUS_TYPE, name: I18n.t('topic.publish_to_category.title') },
{ id: DELETE_STATUS_TYPE, name: I18n.t('topic.auto_delete.title') }
{ id: DELETE_STATUS_TYPE, name: I18n.t('topic.auto_delete.title') },
{ id: REMINDER_TYPE, name: I18n.t('topic.reminder.title') }
];
},

View File

@ -0,0 +1,26 @@
module Jobs
class TopicReminder < Jobs::Base
def execute(args)
topic_timer = TopicTimer.find_by(id: args[:topic_timer_id])
topic = topic_timer&.topic
user = topic_timer&.user
if topic_timer.blank? || topic.blank? || user.blank? ||
topic_timer.execute_at > Time.zone.now
return
end
user.notifications.create!(
notification_type: Notification.types[:topic_reminder],
topic_id: topic.id,
post_number: 1,
data: { topic_title: topic.title, display_username: user.username }.to_json
)
topic_timer.trash!(Discourse.system_user)
end
end
end

View File

@ -52,7 +52,8 @@ class Notification < ActiveRecord::Base
custom: 14,
group_mentioned: 15,
group_message_summary: 16,
watching_first_post: 17
watching_first_post: 17,
topic_reminder: 18
)
end

View File

@ -215,7 +215,7 @@ class Topic < ActiveRecord::Base
if !@ignore_category_auto_close &&
self.category &&
self.category.auto_close_hours &&
!topic_timer&.execute_at
!public_topic_timer&.execute_at
self.set_or_create_timer(
TopicTimer.types[:close],
@ -953,12 +953,8 @@ SQL
Topic.where("pinned_until < now()").update_all(pinned_at: nil, pinned_globally: false, pinned_until: nil)
end
def topic_timer
@topic_timer ||= topic_timers.first
end
def topic_status_update
topic_timer # will be used to filter timers unrelated to topic status
def public_topic_timer
topic_timers.where(deleted_at: nil, public_type: true).first
end
# Valid arguments for the time:
@ -974,10 +970,11 @@ SQL
# * based_on_last_post: True if time should be based on timestamp of the last post.
# * category_id: Category that the update will apply to.
def set_or_create_timer(status_type, time, by_user: nil, timezone_offset: 0, based_on_last_post: false, category_id: SiteSetting.uncategorized_category_id)
topic_timer = TopicTimer.find_or_initialize_by(
status_type: status_type,
topic: self
)
topic_timer = if TopicTimer.public_types[status_type]
TopicTimer.find_or_initialize_by( status_type: status_type, topic: self )
else
TopicTimer.find_or_initialize_by( status_type: status_type, topic: self, user: by_user )
end
if time.blank?
topic_timer.trash!(trashed_by: by_user || Discourse.system_user)

View File

@ -9,13 +9,15 @@ class TopicTimer < ActiveRecord::Base
validates :topic_id, presence: true
validates :execute_at, presence: true
validates :status_type, presence: true
validates :status_type, uniqueness: { scope: [:topic_id, :deleted_at] }
validates :status_type, uniqueness: { scope: [:topic_id, :deleted_at] }, if: :public_type?
validates :status_type, uniqueness: { scope: [:topic_id, :deleted_at, :user_id] }, if: :private_type?
validates :category_id, presence: true, if: :publishing_to_category?
validate :ensure_update_will_happen
before_save do
self.created_at ||= Time.zone.now if execute_at
self.public_type = self.public_type?
if (execute_at_changed? && !execute_at_was.nil?) || user_id_changed?
self.send("cancel_auto_#{self.class.types[status_type]}_job")
@ -36,10 +38,19 @@ class TopicTimer < ActiveRecord::Base
close: 1,
open: 2,
publish_to_category: 3,
delete: 4
delete: 4,
reminder: 5
)
end
def self.public_types
@_public_types ||= types.except(:reminder)
end
def self.private_types
@_private_types ||= types.only(:reminder)
end
def self.ensure_consistency!
TopicTimer.where("topic_timers.execute_at < ?", Time.zone.now)
.find_each do |topic_timer|
@ -59,6 +70,14 @@ class TopicTimer < ActiveRecord::Base
end
end
def public_type?
!!self.class.public_types[self.status_type]
end
def private_type?
!!self.class.private_types[self.status_type]
end
private
def ensure_update_will_happen
@ -82,6 +101,10 @@ class TopicTimer < ActiveRecord::Base
Jobs.cancel_scheduled_job(:delete_topic, topic_timer_id: id)
end
def cancel_auto_reminder_job
Jobs.cancel_scheduled_job(:topic_reminder, topic_timer_id: id)
end
def schedule_auto_open_job(time)
return unless topic
topic.update_status('closed', true, user) if !topic.closed
@ -113,6 +136,10 @@ class TopicTimer < ActiveRecord::Base
def schedule_auto_delete_job(time)
Jobs.enqueue_at(time, :delete_topic, topic_timer_id: id)
end
def schedule_auto_reminder_job(time)
Jobs.enqueue_at(time, :topic_reminder, topic_timer_id: id)
end
end
# == Schema Information

View File

@ -251,7 +251,7 @@ class TopicViewSerializer < ApplicationSerializer
def topic_timer
TopicTimerSerializer.new(
object.topic.topic_timer, root: false
object.topic.public_topic_timer, root: false
)
end

View File

@ -2,7 +2,7 @@ TopicStatusUpdater = Struct.new(:topic, :user) do
def update!(status, enabled, opts={})
status = Status.new(status, enabled)
@topic_status_update = topic.topic_status_update
@topic_status_update = topic.public_topic_timer
updated = nil
Topic.transaction do

View File

@ -1240,6 +1240,7 @@ en:
moved_post: "<i title='moved post' class='fa fa-sign-out'></i><p><span>{{username}}</span> moved {{description}}</p>"
linked: "<i title='linked post' class='fa fa-link'></i><p><span>{{username}}</span> {{description}}</p>"
granted_badge: "<i title='badge granted' class='fa fa-certificate'></i><p>Earned '{{description}}'</p>"
topic_reminder: "<i title='topic reminder' class='fa fa-hand-o-right'></i><p><span>{{username}}</span> {{description}}</p>"
watching_first_post: "<i title='new topic' class='fa fa-dot-circle-o'></i><p><span>New Topic</span> {{description}}</p>"
@ -1527,6 +1528,8 @@ en:
based_on_last_post: "Don't close until the last post in the topic is at least this old."
auto_delete:
title: "Auto-Delete Topic"
reminder:
title: "Remind Me"
status_update_notice:
auto_open: "This topic will automatically open %{timeLeft}."
@ -1534,6 +1537,7 @@ en:
auto_publish_to_category: "This topic will be published to <a href=%{categoryUrl}>#%{categoryName}</a> %{timeLeft}."
auto_close_based_on_last_post: "This topic will close %{duration} after the last reply."
auto_delete: "This topic will be automatically deleted %{timeLeft}."
auto_reminder: "You will be reminded about this topic %{timeLeft}."
auto_close_title: 'Auto-Close Settings'
auto_close_immediate:
one: "The last post in the topic is already 1 hour old, so the topic will be closed immediately."

View File

@ -0,0 +1,27 @@
class AddPublicTypeToTopicTimers < ActiveRecord::Migration
def up
add_column :topic_timers, :public_type, :boolean, default: true
execute("drop index idx_topic_id_status_type_deleted_at") rescue nil
# Only one public timer per topic (close, open, delete):
execute <<~SQL
CREATE UNIQUE INDEX idx_topic_id_public_type_deleted_at
ON topic_timers (topic_id)
WHERE public_type = TRUE
AND deleted_at IS NULL
SQL
end
def down
execute "DROP INDEX idx_topic_id_public_type_deleted_at"
execute <<~SQL
CREATE UNIQUE INDEX idx_topic_id_status_type_deleted_at
ON topic_timers (topic_id, status_type)
WHERE deleted_at IS NULL
SQL
remove_column :topic_timers, :public_type
end
end

View File

@ -390,7 +390,7 @@ class PostCreator
end
def update_topic_auto_close
topic_timer = @topic.topic_timer
topic_timer = @topic.public_topic_timer
if topic_timer &&
topic_timer.based_on_last_post &&

View File

@ -39,7 +39,7 @@ describe TopicCreator do
it "ignores auto_close_time without raising an error" do
topic = TopicCreator.create(user, Guardian.new(user), valid_attrs.merge(auto_close_time: '24'))
expect(topic).to be_valid
expect(topic.topic_status_update).to eq(nil)
expect(topic.public_topic_timer).to eq(nil)
end
it "category name is case insensitive" do

View File

@ -67,7 +67,7 @@ RSpec.describe "Managing a topic's status update", type: :request do
status_type: TopicTimer.types[1]
expect(response).to be_success
expect(topic.reload.topic_status_update).to eq(nil)
expect(topic.reload.public_topic_timer).to eq(nil)
json = JSON.parse(response.body)

View File

@ -16,7 +16,7 @@ describe Topic do
let(:category) { nil }
it 'should not schedule the topic to auto-close' do
expect(topic.topic_status_update).to eq(nil)
expect(topic.public_topic_timer).to eq(nil)
expect(job_klass.jobs).to eq([])
end
end
@ -25,7 +25,7 @@ describe Topic do
let(:category) { Fabricate(:category, auto_close_hours: nil) }
it 'should not schedule the topic to auto-close' do
expect(topic.topic_status_update).to eq(nil)
expect(topic.public_topic_timer).to eq(nil)
expect(job_klass.jobs).to eq([])
end
end
@ -49,11 +49,11 @@ describe Topic do
topic_status_update = TopicTimer.last
expect(topic_status_update.topic).to eq(topic)
expect(topic.topic_status_update.execute_at).to be_within_one_second_of(2.hours.from_now)
expect(topic.public_topic_timer.execute_at).to be_within_one_second_of(2.hours.from_now)
args = job_klass.jobs.last['args'].first
expect(args["topic_timer_id"]).to eq(topic.topic_status_update.id)
expect(args["topic_timer_id"]).to eq(topic.public_topic_timer.id)
expect(args["state"]).to eq(true)
end
@ -79,7 +79,7 @@ describe Topic do
context 'topic is closed manually' do
it 'should remove the schedule to auto-close the topic' do
Timecop.freeze do
topic_timer_id = staff_topic.topic_timer.id
topic_timer_id = staff_topic.public_topic_timer.id
staff_topic.update_status('closed', true, admin)

View File

@ -19,10 +19,10 @@ describe Jobs::DeleteTopic do
first_post
Timecop.freeze(2.hours.from_now) do
described_class.new.execute(topic_timer_id: topic.topic_timer.id)
described_class.new.execute(topic_timer_id: topic.public_topic_timer.id)
expect(topic.reload).to be_trashed
expect(first_post.reload).to be_trashed
expect(topic.reload.topic_timer).to eq(nil)
expect(topic.reload.public_topic_timer).to eq(nil)
end
end
@ -31,7 +31,7 @@ describe Jobs::DeleteTopic do
topic.trash!
Timecop.freeze(2.hours.from_now) do
Topic.any_instance.expects(:trash!).never
described_class.new.execute(topic_timer_id: topic.topic_timer.id)
described_class.new.execute(topic_timer_id: topic.public_topic_timer.id)
end
end
@ -41,7 +41,7 @@ describe Jobs::DeleteTopic do
)
create_post(topic: t)
Timecop.freeze(4.hours.from_now) do
described_class.new.execute(topic_timer_id: t.topic_timer.id)
described_class.new.execute(topic_timer_id: t.public_topic_timer.id)
expect(t.reload).to_not be_trashed
end
end
@ -56,7 +56,7 @@ describe Jobs::DeleteTopic do
it "shouldn't delete the topic" do
create_post(topic: topic)
Timecop.freeze(2.hours.from_now) do
described_class.new.execute(topic_timer_id: topic.topic_timer.id)
described_class.new.execute(topic_timer_id: topic.public_topic_timer.id)
expect(topic.reload).to_not be_trashed
end
end

View File

@ -29,7 +29,7 @@ RSpec.describe Jobs::PublishTopicToCategory do
Timecop.travel(1.hour.ago) { topic }
topic.trash!
described_class.new.execute(topic_timer_id: topic.topic_timer.id)
described_class.new.execute(topic_timer_id: topic.public_topic_timer.id)
topic.reload
expect(topic.category).to eq(category)
@ -41,13 +41,13 @@ RSpec.describe Jobs::PublishTopicToCategory do
Timecop.travel(1.hour.ago) { topic.update!(visible: false) }
message = MessageBus.track_publish do
described_class.new.execute(topic_timer_id: topic.topic_timer.id)
described_class.new.execute(topic_timer_id: topic.public_topic_timer.id)
end.first
topic.reload
expect(topic.category).to eq(another_category)
expect(topic.visible).to eq(true)
expect(topic.topic_timer).to eq(nil)
expect(topic.public_topic_timer).to eq(nil)
%w{created_at bumped_at updated_at last_posted_at}.each do |attribute|
expect(topic.public_send(attribute)).to be_within(1.second).of(Time.zone.now)
@ -68,7 +68,7 @@ RSpec.describe Jobs::PublishTopicToCategory do
it 'should publish the topic to the new category' do
message = MessageBus.track_publish do
described_class.new.execute(topic_timer_id: topic.topic_timer.id)
described_class.new.execute(topic_timer_id: topic.public_topic_timer.id)
end.last
topic.reload

View File

@ -18,7 +18,7 @@ describe Jobs::ToggleTopicClosed do
Timecop.travel(1.hour.from_now) do
described_class.new.execute(
topic_timer_id: topic.topic_timer.id,
topic_timer_id: topic.public_topic_timer.id,
state: true
)
@ -35,7 +35,7 @@ describe Jobs::ToggleTopicClosed do
Timecop.travel(1.hour.from_now) do
described_class.new.execute(
topic_timer_id: topic.topic_timer.id,
topic_timer_id: topic.public_topic_timer.id,
state: false
)
@ -54,7 +54,7 @@ describe Jobs::ToggleTopicClosed do
Topic.any_instance.expects(:update_status).never
described_class.new.execute(
topic_timer_id: topic.topic_timer.id,
topic_timer_id: topic.public_topic_timer.id,
state: true
)
end
@ -69,7 +69,7 @@ describe Jobs::ToggleTopicClosed do
it 'should not do anything' do
described_class.new.execute(
topic_timer_id: topic.topic_timer.id,
topic_timer_id: topic.public_topic_timer.id,
state: false
)

View File

@ -0,0 +1,54 @@
require 'rails_helper'
describe Jobs::TopicReminder do
let(:admin) { Fabricate(:admin) }
let(:topic) { Fabricate(:topic, topic_timers: [
Fabricate(:topic_timer, user: admin, status_type: TopicTimer.types[:reminder])
]) }
before do
SiteSetting.queue_jobs = true
end
it "should be able to create a reminder" do
topic_timer = topic.topic_timers.first
Timecop.freeze(1.day.from_now) do
expect {
described_class.new.execute(topic_timer_id: topic_timer.id)
}.to change { Notification.count }.by(1)
expect( admin.notifications.where(notification_type: Notification.types[:topic_reminder]).first&.topic_id ).to eq(topic.id)
expect( TopicTimer.where(id: topic_timer.id).first ).to be_nil
end
end
it "does nothing if it was trashed before the scheduled time" do
topic_timer = topic.topic_timers.first
topic_timer.trash!(Discourse.system_user)
Timecop.freeze(1.day.from_now) do
expect {
described_class.new.execute(topic_timer_id: topic_timer.id)
}.to_not change { Notification.count }
end
end
it "does nothing if job runs too early" do
topic_timer = topic.topic_timers.first
topic_timer.update_attribute(:execute_at, 8.hours.from_now)
Timecop.freeze(6.hours.from_now) do
expect {
described_class.new.execute(topic_timer_id: topic_timer.id)
}.to_not change { Notification.count }
end
end
it "does nothing if topic was deleted" do
topic_timer = topic.topic_timers.first
topic.trash!
Timecop.freeze(1.day.from_now) do
expect {
described_class.new.execute(topic_timer_id: topic_timer.id)
}.to_not change { Notification.count }
end
end
end

View File

@ -343,7 +343,7 @@ describe Category do
it "should not set its description topic to auto-close" do
category = Fabricate(:category, name: 'Closing Topics', auto_close_hours: 1)
expect(category.topic.topic_status_update).to eq(nil)
expect(category.topic.public_topic_timer).to eq(nil)
end
describe "creating a new category with the same slug" do

View File

@ -1253,7 +1253,7 @@ describe Topic do
it 'updates topic status update execute_at if it was already set to close' do
Timecop.freeze(now) do
closing_topic.set_or_create_timer(TopicTimer.types[:close], 48)
expect(closing_topic.reload.topic_status_update.execute_at).to eq(2.days.from_now)
expect(closing_topic.reload.public_topic_timer.execute_at).to eq(2.days.from_now)
end
end
@ -1280,6 +1280,30 @@ describe Topic do
end
end
end
describe "private status type" do
let(:topic) { Fabricate(:topic) }
let(:reminder) { Fabricate(:topic_timer, user: admin, topic: topic, status_type: TopicTimer.types[:reminder]) }
let(:other_admin) { Fabricate(:admin) }
it "lets two users have their own record" do
reminder
expect {
topic.set_or_create_timer(TopicTimer.types[:reminder], 2, by_user: other_admin)
}.to change { TopicTimer.count }.by(1)
end
it "can update a user's existing record" do
Timecop.freeze(now) do
reminder
expect {
topic.set_or_create_timer(TopicTimer.types[:reminder], 11, by_user: admin)
}.to_not change { TopicTimer.count }
reminder.reload
expect(reminder.execute_at).to eq(11.hours.from_now)
end
end
end
end
describe 'for_digest' do

View File

@ -3,6 +3,7 @@ require 'rails_helper'
RSpec.describe TopicTimer, type: :model do
let(:topic_timer) { Fabricate(:topic_timer) }
let(:topic) { Fabricate(:topic) }
let(:admin) { Fabricate(:admin) }
before do
Jobs::ToggleTopicClosed.jobs.clear
@ -10,13 +11,27 @@ RSpec.describe TopicTimer, type: :model do
context "validations" do
describe '#status_type' do
it 'should ensure that only one active topic status update exists' do
it 'should ensure that only one active public topic status update exists' do
topic_timer.update!(topic: topic)
Fabricate(:topic_timer, deleted_at: Time.zone.now, topic: topic)
expect { Fabricate(:topic_timer, topic: topic) }
.to raise_error(ActiveRecord::RecordInvalid)
end
it 'should ensure that only one active private topic timer exists per user' do
Fabricate(:topic_timer, topic: topic, user: admin, status_type: TopicTimer.types[:reminder])
expect { Fabricate(:topic_timer, topic: topic, user: admin, status_type: TopicTimer.types[:reminder]) }
.to raise_error(ActiveRecord::RecordInvalid)
end
it 'should allow users to have their own private topic timer' do
Fabricate(:topic_timer, topic: topic, user: admin, status_type: TopicTimer.types[:reminder])
expect {
Fabricate(:topic_timer, topic: topic, user: Fabricate(:admin), status_type: TopicTimer.types[:reminder])
}.to_not raise_error
end
end
describe '#execute_at' do
@ -204,6 +219,27 @@ RSpec.describe TopicTimer, type: :model do
end
end
end
describe 'public_type' do
[:close, :open, :delete].each do |public_type|
it "is true for #{public_type}" do
timer = Fabricate(:topic_timer, status_type: described_class.types[public_type])
expect(timer.public_type).to eq(true)
end
end
it "is true for publish_to_category" do
timer = Fabricate(:topic_timer, status_type: described_class.types[:publish_to_category], category: Fabricate(:category))
expect(timer.public_type).to eq(true)
end
described_class.private_types.keys.each do |private_type|
it "is false for #{private_type}" do
timer = Fabricate(:topic_timer, status_type: described_class.types[private_type])
expect(timer.public_type).to be_falsey
end
end
end
end
describe '.ensure_consistency!' do
@ -244,5 +280,19 @@ RSpec.describe TopicTimer, type: :model do
expect(job_args["topic_timer_id"]).to eq(open_topic_timer.id)
expect(job_args["state"]).to eq(false)
end
it "should enqueue remind me jobs that have been missed" do
reminder = Fabricate(:topic_timer,
status_type: described_class.types[:reminder],
execute_at: Time.zone.now - 1.hour,
created_at: Time.zone.now - 2.hour
)
expect { described_class.ensure_consistency! }
.to change { Jobs::TopicReminder.jobs.count }.by(1)
job_args = Jobs::TopicReminder.jobs.first["args"].first
expect(job_args["topic_timer_id"]).to eq(reminder.id)
end
end
end