FEATURE: Users can override reminders frequency (#30)

FEATURE: Users can override reminders frequency (#30)

  • FEATURE: Users can override reminders frequency

  • Changes:

  • Avoid creating a user custom field when the used didn’t override the frequency
  • Sanitize frequency value using coercion
  • Minor fixes
  • Sanitize query and user query single
diff --git a/app/models/remind_assigns_frequency_site_settings.rb b/app/models/remind_assigns_frequency_site_settings.rb
index 0ad6e55..305d43a 100644
--- a/app/models/remind_assigns_frequency_site_settings.rb
+++ b/app/models/remind_assigns_frequency_site_settings.rb
@@ -10,6 +10,7 @@ class RemindAssignsFrequencySiteSettings < EnumSiteSetting
   end
 
   DAILY_MINUTES = 24 * 60 * 1
+  WEEKLY_MINUTES = DAILY_MINUTES * 7
   MONTHLY_MINUTES = DAILY_MINUTES * 30
   QUARTERLY_MINUTES = DAILY_MINUTES * 90
 
@@ -23,6 +24,10 @@ class RemindAssignsFrequencySiteSettings < EnumSiteSetting
         value: DAILY_MINUTES
       },
       {
+        name: 'discourse_assign.reminders_frequency.weekly',
+        value: WEEKLY_MINUTES
+      },
+      {
         name: 'discourse_assign.reminders_frequency.monthly',
         value: MONTHLY_MINUTES
       },
diff --git a/assets/javascripts/discourse-assign/connectors/user-preferences-notifications/remind-assigns-frequency.hbs b/assets/javascripts/discourse-assign/connectors/user-preferences-notifications/remind-assigns-frequency.hbs
new file mode 100644
index 0000000..195e9fd
--- /dev/null
+++ b/assets/javascripts/discourse-assign/connectors/user-preferences-notifications/remind-assigns-frequency.hbs
@@ -0,0 +1 @@
+{{remind-assigns-frequency user=model}}
diff --git a/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6 b/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6
index 7ad9b70..638031c 100644
--- a/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6
+++ b/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6
@@ -226,6 +226,15 @@ function initialize(api) {
     "notification.discourse_assign.assign_notification",
     "user-plus"
   );
+
+  api.modifyClass("controller:preferences/notifications", {
+    actions: {
+      save() {
+        this.get("saveAttrNames").push("custom_fields");
+        this._super(...arguments);
+      }
+    }
+  });
 }
 
 export default {
diff --git a/assets/javascripts/discourse/components/remind-assigns-frequency.js.es6 b/assets/javascripts/discourse/components/remind-assigns-frequency.js.es6
new file mode 100644
index 0000000..29e9831
--- /dev/null
+++ b/assets/javascripts/discourse/components/remind-assigns-frequency.js.es6
@@ -0,0 +1,34 @@
+import computed from "ember-addons/ember-computed-decorators";
+
+export default Ember.Component.extend({
+  selectedFrequency: null,
+
+  @computed("user.reminders_frequency")
+  availableFrequencies() {
+    return this.get("user.reminders_frequency").map(freq => {
+      return {
+        name: I18n.t(freq.name),
+        value: freq.value,
+        selected: false
+      };
+    });
+  },
+
+  didInsertElement() {
+    let current_frequency = this.get(
+      "user.custom_fields.remind_assigns_frequency"
+    );
+
+    if (current_frequency === undefined) {
+      current_frequency = this.get("siteSettings.remind_assigns_frequency");
+    }
+
+    this.set("selectedFrequency", current_frequency);
+  },
+
+  actions: {
+    setFrequency(newFrequency) {
+      this.set("user.custom_fields.remind_assigns_frequency", newFrequency);
+    }
+  }
+});
diff --git a/assets/javascripts/discourse/templates/components/remind-assigns-frequency.hbs b/assets/javascripts/discourse/templates/components/remind-assigns-frequency.hbs
new file mode 100644
index 0000000..e7b3e7b
--- /dev/null
+++ b/assets/javascripts/discourse/templates/components/remind-assigns-frequency.hbs
@@ -0,0 +1,10 @@
+{{#if siteSettings.assign_enabled}}
+  <div class="controls controls-dropdown">
+    <label>{{i18n "discourse_assign.reminders_frequency.description"}}</label>
+    {{combo-box valueAttribute="value" 
+                content=availableFrequencies 
+                value=selectedFrequency
+                onSelect=(action "setFrequency")
+    }}
+  </div>
+{{/if}}
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 1d1cfaf..c09e9be 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -26,8 +26,10 @@ en:
         title: "claim"
         help: "Assign topic to yourself"
       reminders_frequency:
+        description: "Frequency for receiving assigned topics reminders"
         never: 'Never'
         daily: 'Daily'
+        weekly: 'Weekly'
         monthly: 'Monthly'
         quarterly: 'Quarterly'
     user:
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index a33fa3f..184c4a1 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -27,6 +27,7 @@ en:
     reminders_frequency:
         never: 'never'
         daily: 'daily'
+        weekly: 'weekly'
         monthly: 'monthly'
         quarterly: 'quarterly'
   assign_mailer:
diff --git a/db/migrate/20190503145428_add_reminds_assign_frequency_index.rb b/db/migrate/20190503145428_add_reminds_assign_frequency_index.rb
new file mode 100644
index 0000000..1cdf2a5
--- /dev/null
+++ b/db/migrate/20190503145428_add_reminds_assign_frequency_index.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class AddRemindsAssignFrequencyIndex < ActiveRecord::Migration[5.2]
+  def change
+    add_index :user_custom_fields, %i[name user_id], name: :idx_user_custom_fields_remind_assigns_frequency,
+                                                     unique: true, where: "name = 'remind_assigns_frequency'"
+  end
+end
diff --git a/jobs/scheduled/enqueue_reminders.rb b/jobs/scheduled/enqueue_reminders.rb
index 0f7cfa2..dcfee89 100644
--- a/jobs/scheduled/enqueue_reminders.rb
+++ b/jobs/scheduled/enqueue_reminders.rb
@@ -12,27 +12,41 @@ module Jobs
     private
 
     def skip_enqueue?
-      SiteSetting.remind_assigns_frequency.nil? || SiteSetting.remind_assigns_frequency.zero?
+      SiteSetting.remind_assigns_frequency.nil? || !SiteSetting.assign_enabled?
     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
-        ).joins("INNER JOIN users ON topic_custom_fields.value::INT = users.id")
-        .where("users.moderator OR users.admin")
-        .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')
+      global_frequency = SiteSetting.remind_assigns_frequency
+      frequency = ActiveRecord::Base.sanitize_sql("COALESCE(user_frequency.value, '#{global_frequency}')::INT")
+
+      DB.query_single(<<~SQL
+        SELECT topic_custom_fields.value
+        FROM topic_custom_fields
+
+        LEFT OUTER JOIN user_custom_fields AS last_reminder
+        ON topic_custom_fields.value::INT = last_reminder.user_id
+        AND last_reminder.name = '#{PendingAssignsReminder::REMINDED_AT}'
+
+        LEFT OUTER JOIN user_custom_fields AS user_frequency
+        ON topic_custom_fields.value::INT = user_frequency.user_id
+        AND user_frequency.name = '#{PendingAssignsReminder::REMINDERS_FREQUENCY}'
+
+        INNER JOIN users ON topic_custom_fields.value::INT = users.id
+        INNER JOIN topics ON topics.id = topic_custom_fields.topic_id AND (topics.deleted_at IS NULL)
+
+        WHERE (users.moderator OR users.admin)
+        AND #{frequency} > 0
+        AND (
+          last_reminder.value IS NULL OR
+          last_reminder.value::TIMESTAMP <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * #{frequency})
+        )

[... diff too long, it was truncated ...]

GitHub sha: abe81420

1 Like