diff --git a/app/models/remind_assigns_frequency_site_settings.rb b/app/models/remind_assigns_frequency_site_settings.rb new file mode 100644 index 0000000..38a40bd --- /dev/null +++ b/app/models/remind_assigns_frequency_site_settings.rb @@ -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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index eba1a21..633c349 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -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})" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index d9a0f48..6e41032 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -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} diff --git a/config/settings.yml b/config/settings.yml index 44c98f6..f722435 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -19,3 +19,7 @@ plugins: default: false client: true assign_mailer_enabled: false + remind_assigns_frequency: + client: true + enum: "RemindAssignsFrequencySiteSettings" + default: 43200 \ No newline at end of file diff --git a/db/migrate/20190422200243_add_last_reminded_at_index.rb b/db/migrate/20190422200243_add_last_reminded_at_index.rb new file mode 100644 index 0000000..9113382 --- /dev/null +++ b/db/migrate/20190422200243_add_last_reminded_at_index.rb @@ -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 diff --git a/jobs/regular/remind_user.rb b/jobs/regular/remind_user.rb new file mode 100644 index 0000000..9a8cd78 --- /dev/null +++ b/jobs/regular/remind_user.rb @@ -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 diff --git a/jobs/scheduled/enqueue_reminders.rb b/jobs/scheduled/enqueue_reminders.rb new file mode 100644 index 0000000..dada73a --- /dev/null +++ b/jobs/scheduled/enqueue_reminders.rb @@ -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 diff --git a/lib/pending_assigns_reminder.rb b/lib/pending_assigns_reminder.rb new file mode 100644 index 0000000..9996db2 --- /dev/null +++ b/lib/pending_assigns_reminder.rb @@ -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 diff --git a/plugin.rb b/plugin.rb index 935a306..fbcf71a 100644 --- a/plugin.rb +++ b/plugin.rb @@ -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 diff --git a/spec/jobs/scheduled/enqueue_reminders_spec.rb b/spec/jobs/scheduled/enqueue_reminders_spec.rb new file mode 100644 index 0000000..f5e50d0 --- /dev/null +++ b/spec/jobs/scheduled/enqueue_reminders_spec.rb @@ -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 diff --git a/spec/lib/pending_assigns_reminder_spec.rb b/spec/lib/pending_assigns_reminder_spec.rb new file mode 100644 index 0000000..534b63d --- /dev/null +++ b/spec/lib/pending_assigns_reminder_spec.rb @@ -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