FEATURE: Remind users of assigned tasks. (#28)
* FEATURE: Remind users of assigned tasks. * User an user custom field instead of adding a new column * Improve tests. fix assigns count and display oldest assigned topics correctly. Do not remind about recently assigned tasks
This commit is contained in:
parent
4625108888
commit
7b7432990a
|
@ -0,0 +1,30 @@
|
|||
require_dependency 'enum_site_setting'
|
||||
|
||||
class RemindAssignsFrequencySiteSettings < EnumSiteSetting
|
||||
|
||||
def self.valid_value?(val)
|
||||
val.to_i.to_s == val.to_s &&
|
||||
values.any? { |v| v[:value] == val.to_i }
|
||||
end
|
||||
|
||||
def self.values
|
||||
@values ||= [
|
||||
{ name: 'discourse_assign.reminders_frequency.never', value: 0 },
|
||||
{ name: 'discourse_assign.reminders_frequency.daily', value: 1440 },
|
||||
{ name: 'discourse_assign.reminders_frequency.monthly', value: 43200 },
|
||||
{ name: 'discourse_assign.reminders_frequency.quarterly', value: 131400 }
|
||||
]
|
||||
end
|
||||
|
||||
def self.translate_names?
|
||||
true
|
||||
end
|
||||
|
||||
def self.frequency_for(minutes)
|
||||
value = values.detect { |v| v[:value] == minutes }
|
||||
|
||||
raise Discourse::InvalidParameters(:task_reminders_frequency) if value.nil?
|
||||
|
||||
I18n.t(value[:name])
|
||||
end
|
||||
end
|
|
@ -12,7 +12,6 @@ en:
|
|||
unassign_all:
|
||||
title: "Unassign All"
|
||||
confirm: "Are you sure you want to unassign all topics from {{username}}?"
|
||||
|
||||
unassign:
|
||||
title: "Unassign"
|
||||
help: "Unassign Topic"
|
||||
|
@ -26,6 +25,11 @@ en:
|
|||
claim:
|
||||
title: "claim"
|
||||
help: "Assign topic to yourself"
|
||||
reminders_frequency:
|
||||
never: 'Never'
|
||||
daily: 'Daily'
|
||||
monthly: 'Monthly'
|
||||
quarterly: 'Quarterly'
|
||||
user:
|
||||
messages:
|
||||
assigned_title: "Assigned (%{count})"
|
||||
|
|
|
@ -12,6 +12,8 @@ en:
|
|||
assign_locks_flags: "When a topic is assigned to a staff member, its flags can only be handled by that person"
|
||||
assign_mailer_enabled: "When enabled, the assigned user will receive a notification email on each assignment"
|
||||
flags_require_assign: "When enabled, flags cannot be handled unless assigned to a user."
|
||||
remind_assigns: "Remind users about pending assigns."
|
||||
remind_assigns_frequency: "Frequency for reminding users about assigned topics."
|
||||
discourse_assign:
|
||||
assigned_to: "Topic assigned to @%{username}"
|
||||
unassigned: "Topic was unassigned"
|
||||
|
@ -20,6 +22,11 @@ en:
|
|||
flag_assigned: "Sorry, that flag's topic is assigned to another user"
|
||||
flag_unclaimed: "You must claim that topic before acting on the flag"
|
||||
topic_assigned_excerpt: "assigned you the topic '%{title}'"
|
||||
reminders_frequency:
|
||||
never: 'never'
|
||||
daily: 'daily'
|
||||
monthly: 'monthly'
|
||||
quarterly: 'quarterly'
|
||||
assign_mailer:
|
||||
title: "Assign Mailer"
|
||||
subject_template: "[%{email_prefix}] %{assignee_name} assigned you to '%{topic_title}'!"
|
||||
|
@ -31,3 +38,22 @@ en:
|
|||
|
||||
If you're interested, click the link below:
|
||||
[%{topic_link}](%{topic_link})
|
||||
pending_assigns_reminder:
|
||||
title: "You have %{pending_assignments} pending assignments"
|
||||
body: |
|
||||
You currently have [%{pending_assignments} pending assignments](%{assignments_link}).
|
||||
|
||||
%{newest_assignments}
|
||||
%{oldest_assignments}
|
||||
|
||||
This reminder will be sent %{frequency} if you have more than one assigned topic.
|
||||
newest: |
|
||||
### Newest
|
||||
%{topic_0}
|
||||
%{topic_1}
|
||||
%{topic_2}
|
||||
oldest: |
|
||||
### Oldest
|
||||
%{topic_0}
|
||||
%{topic_1}
|
||||
%{topic_2}
|
||||
|
|
|
@ -19,3 +19,7 @@ plugins:
|
|||
default: false
|
||||
client: true
|
||||
assign_mailer_enabled: false
|
||||
remind_assigns_frequency:
|
||||
client: true
|
||||
enum: "RemindAssignsFrequencySiteSettings"
|
||||
default: 43200
|
|
@ -0,0 +1,6 @@
|
|||
class AddLastRemindedAtIndex < ActiveRecord::Migration[5.2]
|
||||
def change
|
||||
add_index :user_custom_fields, %i[name user_id], name: :idx_user_custom_fields_last_reminded_at,
|
||||
unique: true, where: "name = 'last_reminded_at'"
|
||||
end
|
||||
end
|
|
@ -0,0 +1,12 @@
|
|||
module Jobs
|
||||
class RemindUser < Jobs::Base
|
||||
sidekiq_options queue: 'low'
|
||||
|
||||
def execute(args)
|
||||
user = User.find_by(args[:user_id])
|
||||
raise Discourse::InvalidParameters.new(:user_id) if user.nil?
|
||||
|
||||
PendingAssignsReminder.new.remind(user)
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,34 @@
|
|||
module Jobs
|
||||
class EnqueueReminders < Jobs::Scheduled
|
||||
every 2.hours
|
||||
|
||||
def execute(_args)
|
||||
return if skip_enqueue?
|
||||
user_ids.each { |id| Jobs.enqueue(:remind_user, user_id: id) }
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def skip_enqueue?
|
||||
SiteSetting.remind_assigns_frequency.nil? || SiteSetting.remind_assigns_frequency.zero?
|
||||
end
|
||||
|
||||
def user_ids
|
||||
interval = SiteSetting.remind_assigns_frequency
|
||||
|
||||
TopicCustomField
|
||||
.joins(<<~SQL
|
||||
LEFT OUTER JOIN user_custom_fields ON topic_custom_fields.value::INT = user_custom_fields.user_id
|
||||
AND user_custom_fields.name = '#{PendingAssignsReminder::REMINDED_AT}'
|
||||
SQL
|
||||
).where(<<~SQL
|
||||
user_custom_fields.value IS NULL OR
|
||||
user_custom_fields.value::TIMESTAMP <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * #{interval})
|
||||
SQL
|
||||
).where("topic_custom_fields.updated_at::TIMESTAMP <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * ?)", interval)
|
||||
.where(name: TopicAssigner::ASSIGNED_TO_ID)
|
||||
.group('topic_custom_fields.value').having('COUNT(topic_custom_fields.value) > 1')
|
||||
.pluck('topic_custom_fields.value')
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,85 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class PendingAssignsReminder
|
||||
REMINDED_AT = 'last_reminded_at'
|
||||
REMINDER_THRESHOLD = 2
|
||||
|
||||
def remind(user)
|
||||
newest_topics = assigned_topics(user, order: :desc)
|
||||
return if newest_topics.size < REMINDER_THRESHOLD
|
||||
oldest_topics = assigned_topics(user, order: :asc).where.not(id: newest_topics.map(&:id))
|
||||
assigned_topics_count = assigned_count_for(user)
|
||||
|
||||
title = I18n.t('pending_assigns_reminder.title', pending_assignments: assigned_topics_count)
|
||||
|
||||
PostCreator.create!(
|
||||
Discourse.system_user,
|
||||
title: title,
|
||||
raw: reminder_body(user, assigned_topics_count, newest_topics, oldest_topics),
|
||||
archetype: Archetype.private_message,
|
||||
target_usernames: user.username,
|
||||
validate: false
|
||||
)
|
||||
|
||||
update_last_reminded(user)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def assigned_count_for(user)
|
||||
TopicCustomField.where(name: TopicAssigner::ASSIGNED_TO_ID, value: user.id).count
|
||||
end
|
||||
|
||||
def assigned_topics(user, order:)
|
||||
Topic.joins(:_custom_fields).select(:slug, :id, :fancy_title, 'topic_custom_fields.created_at AS assigned_at')
|
||||
.where('topic_custom_fields.name = ? AND topic_custom_fields.value = ?', TopicAssigner::ASSIGNED_TO_ID, user.id.to_s)
|
||||
.order("topic_custom_fields.created_at #{order}")
|
||||
.limit(3)
|
||||
end
|
||||
|
||||
def reminder_body(user, assigned_topics_count, first_three_topics, last_three_topics)
|
||||
newest_list = build_list_for(:newest, first_three_topics)
|
||||
oldest_list = build_list_for(:oldest, last_three_topics)
|
||||
|
||||
I18n.t(
|
||||
'pending_assigns_reminder.body',
|
||||
pending_assignments: assigned_topics_count,
|
||||
assignments_link: "#{Discourse.base_url}/u/#{user.username_lower}/assigned",
|
||||
newest_assignments: newest_list,
|
||||
oldest_assignments: oldest_list,
|
||||
frequency: frequency_in_words
|
||||
)
|
||||
end
|
||||
|
||||
def build_list_for(key, topics)
|
||||
return '' if topics.empty?
|
||||
initial_list = { 'topic_0' => '', 'topic_1' => '', 'topic_2' => '' }
|
||||
items = topics.each_with_index.reduce(initial_list) do |memo, (t, index)|
|
||||
memo["topic_#{index}"] = "- [#{Emoji.gsub_emoji_to_unicode(t.fancy_title)}](#{t.relative_url}) - assigned #{time_in_words_for(t)}"
|
||||
memo
|
||||
end
|
||||
|
||||
I18n.t("pending_assigns_reminder.#{key}", items.symbolize_keys!)
|
||||
end
|
||||
|
||||
def time_in_words_for(topic)
|
||||
FreedomPatches::Rails4.distance_of_time_in_words(
|
||||
Time.zone.now, topic.assigned_at.to_time, false, scope: 'datetime.distance_in_words_verbose'
|
||||
)
|
||||
end
|
||||
|
||||
def frequency_in_words
|
||||
::RemindAssignsFrequencySiteSettings.frequency_for(SiteSetting.remind_assigns_frequency)
|
||||
end
|
||||
|
||||
def update_last_reminded(user)
|
||||
update_last_reminded = { REMINDED_AT => DateTime.now }
|
||||
# New API in Discouse that's better under concurrency
|
||||
if user.respond_to?(:upsert_custom_fields)
|
||||
user.upsert_custom_fields(update_last_reminded)
|
||||
else
|
||||
user.custom_fields.merge!(update_last_reminded)
|
||||
user.save_custom_fields
|
||||
end
|
||||
end
|
||||
end
|
|
@ -21,8 +21,11 @@ Discourse::Application.routes.append do
|
|||
end
|
||||
|
||||
after_initialize do
|
||||
require File.expand_path('../jobs/unassign_bulk', __FILE__)
|
||||
require File.expand_path('../jobs/unassign_bulk.rb', __FILE__)
|
||||
require File.expand_path('../jobs/scheduled/enqueue_reminders.rb', __FILE__)
|
||||
require File.expand_path('../jobs/regular/remind_user.rb', __FILE__)
|
||||
require 'topic_assigner'
|
||||
require 'pending_assigns_reminder'
|
||||
|
||||
# Raise an invalid access error if a user tries to act on something
|
||||
# not assigned to them
|
||||
|
|
|
@ -0,0 +1,66 @@
|
|||
require 'rails_helper'
|
||||
|
||||
RSpec.describe Jobs::EnqueueReminders do
|
||||
let(:user) { Fabricate(:user) }
|
||||
|
||||
describe '#execute' do
|
||||
it 'does not enqueue reminders when there are no assigned tasks' do
|
||||
assert_reminders_enqueued(0)
|
||||
end
|
||||
|
||||
it 'enqueues a reminder when the user has more than one task' do
|
||||
assign_multiple_tasks_to(user)
|
||||
|
||||
assert_reminders_enqueued(1)
|
||||
end
|
||||
|
||||
it 'does not enqueue a reminder when the user only has one task' do
|
||||
assign_one_task_to(user)
|
||||
|
||||
assert_reminders_enqueued(0)
|
||||
end
|
||||
|
||||
it "does not enqueue a reminder if it's too soon" do
|
||||
user.upsert_custom_fields(PendingAssignsReminder::REMINDED_AT => 2.days.ago)
|
||||
assign_multiple_tasks_to(user)
|
||||
|
||||
assert_reminders_enqueued(0)
|
||||
end
|
||||
|
||||
it 'enqueues a reminder if the user was reminded more than a month ago' do
|
||||
user.upsert_custom_fields(PendingAssignsReminder::REMINDED_AT => 31.days.ago)
|
||||
assign_multiple_tasks_to(user)
|
||||
|
||||
assert_reminders_enqueued(1)
|
||||
end
|
||||
|
||||
it 'does not enqueue reminders if the remind frequency is set to never' do
|
||||
SiteSetting.remind_assigns_frequency = 0
|
||||
assign_multiple_tasks_to(user)
|
||||
|
||||
assert_reminders_enqueued(0)
|
||||
end
|
||||
|
||||
it 'does not enqueue reminders if the topic was just assigned to the user' do
|
||||
just_assigned = DateTime.now
|
||||
assign_multiple_tasks_to(user, just_assigned)
|
||||
|
||||
assert_reminders_enqueued(0)
|
||||
end
|
||||
|
||||
def assert_reminders_enqueued(expected_amount)
|
||||
expect { subject.execute({}) }.to change(Jobs::RemindUser.jobs, :size).by(expected_amount)
|
||||
end
|
||||
|
||||
def assign_one_task_to(user, assigned_on = 3.months.ago)
|
||||
freeze_time(assigned_on) do
|
||||
post = Fabricate(:post)
|
||||
TopicAssigner.new(post.topic, user).assign(user)
|
||||
end
|
||||
end
|
||||
|
||||
def assign_multiple_tasks_to(user, assigned_on = 3.months.ago)
|
||||
PendingAssignsReminder::REMINDER_THRESHOLD.times { assign_one_task_to(user, assigned_on) }
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,56 @@
|
|||
require 'rails_helper'
|
||||
|
||||
RSpec.describe PendingAssignsReminder do
|
||||
let(:user) { Fabricate(:user) }
|
||||
|
||||
it 'does not create a reminder if the user has 0 assigned topics' do
|
||||
assert_reminder_not_created
|
||||
end
|
||||
|
||||
it 'does not create a reminder if the user only has one task' do
|
||||
post = Fabricate(:post)
|
||||
TopicAssigner.new(post.topic, user).assign(user)
|
||||
|
||||
assert_reminder_not_created
|
||||
end
|
||||
|
||||
def assert_reminder_not_created
|
||||
expect { subject.remind(user) }.to change { Post.count }.by(0)
|
||||
end
|
||||
|
||||
describe 'when the user has multiple tasks' do
|
||||
let(:system) { Discourse.system_user }
|
||||
|
||||
before do
|
||||
@post = Fabricate(:post)
|
||||
@another_post = Fabricate(:post)
|
||||
TopicAssigner.new(@post.topic, user).assign(user)
|
||||
TopicAssigner.new(@another_post.topic, user).assign(user)
|
||||
@assigned_posts = 2
|
||||
end
|
||||
|
||||
it 'creates a reminder for a particular user and sets the timestamp of the last reminder' do
|
||||
expected_last_reminder = DateTime.now
|
||||
|
||||
freeze_time(expected_last_reminder) do
|
||||
subject.remind(user)
|
||||
|
||||
created_post = Post.includes(topic: %i[topic_allowed_users]).last
|
||||
reminded_at = user.reload.custom_fields[described_class::REMINDED_AT].to_datetime
|
||||
|
||||
assert_remind_was_created_correctly(created_post)
|
||||
expect(reminded_at).to eq_time(expected_last_reminder)
|
||||
end
|
||||
end
|
||||
|
||||
def assert_remind_was_created_correctly(post)
|
||||
topic = post.topic
|
||||
expect(topic.user).to eq(system)
|
||||
expect(topic.archetype).to eq(Archetype.private_message)
|
||||
expect(topic.topic_allowed_users.pluck(:user_id)).to contain_exactly(system.id, user.id)
|
||||
expect(topic.title).to eq(I18n.t('pending_assigns_reminder.title', pending_assignments: @assigned_posts))
|
||||
expect(post.raw).to include(@post.topic.fancy_title)
|
||||
expect(post.raw).to include(@another_post.topic.fancy_title)
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue