FEATURE: Allow durations < 1 hour and < 1 day for topic timers where duration is specified (auto delete replies, close based on last post) (#11961)

This PR allows entering a float value for topic timers e.g. 0.5 for 30 minutes when entering hours, 0.5 for 12 hours when entering days. This is achieved by adding a new column to store the duration of a topic timer in minutes instead of the ambiguous both hours and days that it could be before.

This PR has ommitted the post migration to delete the duration column in topic timers; it will be done in a subsequent PR to ensure that no data is lost if the UPDATE query to set duration_mintues fails.

I have to keep the old keyword of duration in set_or_create_topic_timer for backwards compat, will remove at a later date after plugins are updated.

app/assets/javascripts/discourse/app/components/edit-topic-timer-form.js

@@ -7,7 +7,7 @@ import {
   PUBLISH_TO_CATEGORY_STATUS_TYPE,
 } from "discourse/controllers/edit-topic-timer";
 import { FORMAT } from "select-kit/components/future-date-input-selector";
-import discourseComputed from "discourse-common/utils/decorators";
+import discourseComputed, { on } from "discourse-common/utils/decorators";
 import { equal, or, readOnly } from "@ember/object/computed";
 import I18n from "I18n";
 import { action } from "@ember/object";
@@ -26,7 +26,19 @@ export default Component.extend({
   showTimeOnly: or("autoOpen", "autoDelete", "autoBump"),
   showFutureDateInput: or("showTimeOnly", "publishToCategory", "autoClose"),
   useDuration: or("isBasedOnLastPost", "autoDeleteReplies"),
-  originalTopicTimerTime: null,
+  duration: null,
+
+  @on("init")
+  preloadDuration() {
+    if (!this.useDuration || !this.topicTimer.duration_minutes) {
+      return;
+    }
+    if (this.durationType === "days") {
+      this.set("duration", this.topicTimer.duration_minutes / 60 / 24);
+    } else {
+      this.set("duration", this.topicTimer.duration_minutes / 60);
+    }
+  },
 
   @discourseComputed("autoDeleteReplies")
   durationType(autoDeleteReplies) {
@@ -93,13 +105,12 @@ export default Component.extend({
 
   @discourseComputed(
     "topicTimer.updateTime",
-    "topicTimer.duration",
+    "topicTimer.duration_minutes",
-    "useDuration",
+    "useDuration"
-    "durationType"
   )
-  executeAt(updateTime, duration, useDuration, durationType) {
+  executeAt(updateTime, duration, useDuration) {
     if (useDuration) {
-      return moment().add(parseFloat(duration), durationType).format(FORMAT);
+      return moment().add(parseFloat(duration), "minutes").format(FORMAT);
     } else {
       return updateTime;
     }
@@ -107,13 +118,13 @@ export default Component.extend({
 
   @discourseComputed(
     "isBasedOnLastPost",
-    "topicTimer.duration",
+    "topicTimer.duration_minutes",
     "topic.last_posted_at"
   )
   willCloseImmediately(isBasedOnLastPost, duration, lastPostedAt) {
     if (isBasedOnLastPost && duration) {
       let closeDate = moment(lastPostedAt);
-      closeDate = closeDate.add(duration, "hours");
+      closeDate = closeDate.add(duration, "minutes");
       return closeDate < moment();
     }
   },
@@ -140,7 +151,7 @@ export default Component.extend({
     "willCloseImmediately",
     "topicTimer.category_id",
     "useDuration",
-    "topicTimer.duration"
+    "topicTimer.duration_minutes"
   )
   showTopicStatusInfo(
     statusType,
@@ -178,4 +189,13 @@ export default Component.extend({
     });
     this.onChangeInput(type, time);
   },
+
+  @action
+  durationChanged(newDuration) {
+    if (this.durationType === "days") {
+      this.set("topicTimer.duration_minutes", newDuration * 60 * 24);
+    } else {
+      this.set("topicTimer.duration_minutes", newDuration * 60);
+    }
+  },
 });

app/assets/javascripts/discourse/app/components/topic-timer-info.js

@@ -63,15 +63,11 @@ export default Component.extend({
       } else if (minutesLeft > 2) {
         rerenderDelay = 60000;
       }
-      let durationHours = parseInt(this.duration, 0) || 0;
+      let durationMinutes = parseInt(this.durationMinutes, 0) || 0;
-
-      if (isDeleteRepliesType) {
-        durationHours *= 24;
-      }
 
       let options = {
         timeLeft: duration.humanize(true),
-        duration: moment.duration(durationHours, "hours").humanize(),
+        duration: moment.duration(durationMinutes, "minutes").humanize(),
       };
 
       const categoryId = this.categoryId;

app/assets/javascripts/discourse/app/controllers/edit-topic-timer.js

@@ -60,24 +60,24 @@ export default Controller.extend(ModalFunctionality, {
 
   topicTimer: alias("model.topic_timer"),
 
-  _setTimer(time, duration, statusType, basedOnLastPost, categoryId) {
+  _setTimer(time, durationMinutes, statusType, basedOnLastPost, categoryId) {
     this.set("loading", true);
 
-    TopicTimer.updateStatus(
+    TopicTimer.update(
       this.get("model.id"),
       time,
       basedOnLastPost,
       statusType,
       categoryId,
-      duration
+      durationMinutes
     )
       .then((result) => {
-        if (time || duration) {
+        if (time || durationMinutes) {
           this.send("closeModal");
 
           setProperties(this.topicTimer, {
             execute_at: result.execute_at,
-            duration: result.duration,
+            duration_minutes: result.duration_minutes,
             category_id: result.category_id,
           });
 
@@ -131,14 +131,10 @@ export default Controller.extend(ModalFunctionality, {
       this.set("topicTimer.updateTime", time);
     },
 
-    onChangeDuration(value) {
-      this.set("topicTimer.duration", value);
-    },
-
     saveTimer() {
       if (
         !this.get("topicTimer.updateTime") &&
-        !this.get("topicTimer.duration")
+        !this.get("topicTimer.duration_minutes")
       ) {
         this.flash(
           I18n.t("topic.topic_status_update.time_frame_required"),
@@ -148,9 +144,9 @@ export default Controller.extend(ModalFunctionality, {
       }
 
       if (
-        this.get("topicTimer.duration") &&
+        this.get("topicTimer.duration_minutes") &&
         !this.get("topicTimer.updateTime") &&
-        this.get("topicTimer.duration") < 1
+        this.get("topicTimer.duration_minutes") <= 0
       ) {
         this.flash(
           I18n.t("topic.topic_status_update.min_duration"),
@@ -161,7 +157,7 @@ export default Controller.extend(ModalFunctionality, {
 
       this._setTimer(
         this.get("topicTimer.updateTime"),
-        this.get("topicTimer.duration"),
+        this.get("topicTimer.duration_minutes"),
         this.get("topicTimer.status_type"),
         this.get("topicTimer.based_on_last_post"),
         this.get("topicTimer.category_id")

app/assets/javascripts/discourse/app/controllers/topic.js

@@ -1099,13 +1099,7 @@ export default Controller.extend(bufferedProperty("model"), {
     },
 
     removeTopicTimer(statusType, topicTimer) {
-      TopicTimer.updateStatus(
+      TopicTimer.update(this.get("model.id"), null, null, statusType, null)
-        this.get("model.id"),
-        null,
-        null,
-        statusType,
-        null
-      )
         .then(() => this.set(`model.${topicTimer}`, EmberObject.create({})))
         .catch((error) => popupAjaxError(error));
     },

app/assets/javascripts/discourse/app/models/topic-timer.js

@@ -4,13 +4,13 @@ import { ajax } from "discourse/lib/ajax";
 const TopicTimer = RestModel.extend({});
 
 TopicTimer.reopenClass({
-  updateStatus(
+  update(
     topicId,
     time,
     basedOnLastPost,
     statusType,
     categoryId,
-    duration
+    durationMinutes
   ) {
     let data = {
       time,
@@ -23,8 +23,8 @@ TopicTimer.reopenClass({
     if (categoryId) {
       data.category_id = categoryId;
     }
-    if (duration) {
+    if (durationMinutes) {
-      data.duration = duration;
+      data.duration_minutes = durationMinutes;
     }
 
     return ajax({

app/assets/javascripts/discourse/app/templates/components/edit-topic-timer-form.hbs

@@ -24,7 +24,7 @@
   {{#if useDuration}}
     <div class="controls">
       <label class="control-label">{{durationLabel}}</label>
-      {{text-field id="topic_timer_duration" class="topic-timer-duration" type="number" value=topicTimer.duration min="1"}}
+      {{text-field id="topic_timer_duration" class="topic-timer-duration" type="number" value=duration min="0.1" step="0.1" onChange=durationChanged}}
     </div>
   {{/if}}
   {{#if willCloseImmediately}}
@@ -39,7 +39,7 @@
         statusType=statusType
         executeAt=executeAt
         basedOnLastPost=topicTimer.based_on_last_post
-        duration=topicTimer.duration
+        durationMinutes=topicTimer.duration_minutes
         categoryId=topicTimer.category_id
       }}
     </div>

app/assets/javascripts/discourse/app/templates/modal/edit-topic-timer.hbs

@@ -5,7 +5,6 @@
     timerTypes=publicTimerTypes
     onChangeStatusType=(action "onChangeStatusType")
     onChangeInput=(action "onChangeInput")
-    onChangeDuration=(action "onChangeDuration")
   }}
 
   <div class="modal-footer control-group edit-topic-timer-buttons">

app/assets/javascripts/discourse/app/templates/topic.hbs

@@ -298,7 +298,7 @@
                   statusType=model.topic_timer.status_type
                   executeAt=model.topic_timer.execute_at
                   basedOnLastPost=model.topic_timer.based_on_last_post
-                  duration=model.topic_timer.duration
+                  durationMinutes=model.topic_timer.duration_minutes
                   categoryId=model.topic_timer.category_id
                   removeTopicTimer=(action "removeTopicTimer" model.topic_timer.status_type "topic_timer")}}
 

app/assets/javascripts/discourse/tests/acceptance/topic-edit-timer-test.js

@@ -16,7 +16,7 @@ acceptance("Topic - Edit timer", function (needs) {
         execute_at: new Date(
           new Date().getTime() + 1 * 60 * 60 * 1000
         ).toISOString(),
-        duration: 1,
+        duration_minutes: 1440,
         based_on_last_post: false,
         closed: false,
         category_id: null,

app/controllers/topics_controller.rb

@@ -461,7 +461,7 @@ class TopicsController < ApplicationController
         invalid_param(:status_type)
       end
     based_on_last_post = params[:based_on_last_post]
-    params.require(:duration) if based_on_last_post
+    params.require(:duration_minutes) if based_on_last_post
 
     topic = Topic.find_by(id: params[:topic_id])
     guardian.ensure_can_moderate!(topic)
@@ -472,6 +472,7 @@ class TopicsController < ApplicationController
     }
 
     options.merge!(category_id: params[:category_id]) if !params[:category_id].blank?
+    options.merge!(duration_minutes: params[:duration_minutes].to_i) if params[:duration_minutes].present?
     options.merge!(duration: params[:duration].to_i) if params[:duration].present?
 
     topic_status_update = topic.set_or_create_timer(
@@ -483,7 +484,7 @@ class TopicsController < ApplicationController
     if topic.save
       render json: success_json.merge!(
         execute_at: topic_status_update&.execute_at,
-        duration: topic_status_update&.duration,
+        duration_minutes: topic_status_update&.duration_minutes,
         based_on_last_post: topic_status_update&.based_on_last_post,
         closed: topic.closed,
         category_id: topic_status_update&.category_id

app/jobs/regular/delete_replies.rb

@@ -11,11 +11,11 @@ module Jobs
       replies = topic.posts.where("posts.post_number > 1")
       replies = replies.where("like_count < ?", SiteSetting.skip_auto_delete_reply_likes) if SiteSetting.skip_auto_delete_reply_likes > 0
 
-      replies.where('posts.created_at < ?', topic_timer.duration.days.ago).each do |post|
+      replies.where('posts.created_at < ?', topic_timer.duration_minutes.minutes.ago).each do |post|
         PostDestroyer.new(topic_timer.user, post, context: I18n.t("topic_statuses.auto_deleted_by_timer")).destroy
       end
 
-      topic_timer.execute_at = (replies.minimum(:created_at) || Time.zone.now) + topic_timer.duration.days
+      topic_timer.execute_at = (replies.minimum(:created_at) || Time.zone.now) + topic_timer.duration_minutes.minutes
       topic_timer.save
     end
 

app/models/topic.rb

@@ -378,14 +378,14 @@ class Topic < ActiveRecord::Base
        !public_topic_timer&.execute_at
 
       based_on_last_post = self.category.auto_close_based_on_last_post
-      duration = based_on_last_post ? self.category.auto_close_hours : nil
+      duration_minutes = based_on_last_post ? self.category.auto_close_hours * 60 : nil
 
       self.set_or_create_timer(
         TopicTimer.types[timer_type],
         self.category.auto_close_hours,
         by_user: Discourse.system_user,
         based_on_last_post: based_on_last_post,
-        duration: duration
+        duration_minutes: duration_minutes
       )
     end
   end
@@ -1307,8 +1307,18 @@ class Topic < ActiveRecord::Base
   #  * by_user: User who is setting the topic's status update.
   #  * 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, based_on_last_post: false, category_id: SiteSetting.uncategorized_category_id, duration: nil, silent: nil)
+  #  * duration: TODO(2021-06-01): DEPRECATED - do not use
-    return delete_topic_timer(status_type, by_user: by_user) if time.blank? && duration.blank?
+  #  * duration_minutes: The duration of the timer in minutes, which is used if the timer is based
+  #                      on the last post or if the timer type is delete_replies.
+  #  * silent: Affects whether the close topic timer status change will be silent or not.
+  def set_or_create_timer(status_type, time, by_user: nil, based_on_last_post: false, category_id: SiteSetting.uncategorized_category_id, duration: nil, duration_minutes: nil, silent: nil)
+    return delete_topic_timer(status_type, by_user: by_user) if time.blank? && duration_minutes.blank? && duration.blank?
+
+    duration_minutes = duration_minutes ? duration_minutes.to_i : 0
+
+    # TODO(2021-06-01): deprecated - remove this when plugins calling set_or_create_timer
+    # have been fixed to use duration_minutes
+    duration = duration ? duration.to_i : 0
 
     public_topic_timer = !!TopicTimer.public_types[status_type]
     topic_timer_options = { topic: self, public_type: public_topic_timer }
@@ -1319,22 +1329,37 @@ class Topic < ActiveRecord::Base
 
     time_now = Time.zone.now
     topic_timer.based_on_last_post = !based_on_last_post.blank?
-    topic_timer.duration = duration
 
     if status_type == TopicTimer.types[:publish_to_category]
       topic_timer.category = Category.find_by(id: category_id)
     end
 
     if topic_timer.based_on_last_post
-      if duration > 0
+      if duration > 0 || duration_minutes > 0
         last_post_created_at = self.ordered_posts.last.present? ? self.ordered_posts.last.created_at : time_now
-        topic_timer.execute_at = last_post_created_at + duration.hours
+
+        # TODO(2021-06-01): deprecated - remove this when plugins calling set_or_create_timer
+        # have been fixed to use duration_minutes
+        if duration > 0
+          duration_minutes = duration * 60
+        end
+
+        topic_timer.duration_minutes = duration_minutes
+        topic_timer.execute_at = last_post_created_at + duration_minutes.minutes
         topic_timer.created_at = last_post_created_at
       end
     elsif topic_timer.status_type == TopicTimer.types[:delete_replies]
-      if duration > 0
+      if duration > 0 || duration_minutes > 0
         first_reply_created_at = (self.ordered_posts.where("post_number > 1").minimum(:created_at) || time_now)
-        topic_timer.execute_at = first_reply_created_at + duration.days
+
+        # TODO(2021-06-01): deprecated - remove this when plugins calling set_or_create_timer
+        # have been fixed to use duration_minutes
+        if duration > 0
+          duration_minutes = duration * 60 * 24
+        end
+
+        topic_timer.duration_minutes = duration_minutes
+        topic_timer.execute_at = first_reply_created_at + duration_minutes.minutes
         topic_timer.created_at = first_reply_created_at
       end
     else

app/models/topic_timer.rb

@@ -1,6 +1,10 @@
 # frozen_string_literal: true
 
 class TopicTimer < ActiveRecord::Base
+  self.ignored_columns = [
+    "duration" # TODO(2021-06-01): remove
+  ]
+
   include Trashable
 
   belongs_to :user
@@ -237,7 +241,7 @@ end
 #  updated_at         :datetime         not null
 #  category_id        :integer
 #  public_type        :boolean          default(TRUE)
-#  duration           :integer
+#  duration_minutes   :integer
 #
 # Indexes
 #

app/serializers/topic_timer_serializer.rb

@@ -3,7 +3,7 @@
 class TopicTimerSerializer < ApplicationSerializer
   attributes :id,
              :execute_at,
-             :duration,
+             :duration_minutes,
              :based_on_last_post,
              :status_type,
              :category_id

app/services/topic_status_updater.rb

@@ -91,13 +91,15 @@ TopicStatusUpdater = Struct.new(:topic, :user) do
   def message_for_autoclosed(locale_key)
     num_minutes =
       if @topic_timer&.based_on_last_post
-        (@topic_timer.duration || 0).hours
+        (@topic_timer.duration_minutes || 0).minutes.to_i
       elsif @topic_timer&.created_at
         Time.zone.now - @topic_timer.created_at
       else
         Time.zone.now - topic.created_at
       end
 
+    # all of the results above are in seconds, this brings them
+    # back to the actual minutes integer
     num_minutes = (num_minutes / 1.minute).round
 
     if num_minutes.minutes >= 2.days

db/migrate/20210203031628_add_duration_minutes_to_topic_timer.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class AddDurationMinutesToTopicTimer < ActiveRecord::Migration[6.0]
+  def up
+    add_column :topic_timers, :duration_minutes, :integer
+
+    # 7 is delete_replies type, this duration is measured in days, the other
+    # duration is measured in hours
+    DB.exec("UPDATE topic_timers SET duration_minutes = (duration * 60 * 24) WHERE duration_minutes != duration AND status_type = 7 AND duration IS NOT NULL")
+    DB.exec("UPDATE topic_timers SET duration_minutes = (duration * 60) WHERE duration_minutes != duration AND status_type != 7 AND duration IS NOT NULL")
+  end
+
+  def down
+    remove_column :topic_timers, :duration_minutes
+  end
+end

lib/post_creator.rb

@@ -527,12 +527,12 @@ class PostCreator
 
       if topic_timer &&
          topic_timer.based_on_last_post &&
-         topic_timer.duration.to_i > 0
+         topic_timer.duration_minutes.to_i > 0
 
         @topic.set_or_create_timer(TopicTimer.types[:close],
           nil,
           based_on_last_post: topic_timer.based_on_last_post,
-          duration: topic_timer.duration
+          duration_minutes: topic_timer.duration_minutes
         )
       end
     end

spec/components/post_creator_spec.rb

@@ -345,7 +345,7 @@ describe PostCreator do
               based_on_last_post: true,
               execute_at: Time.zone.now - 12.hours,
               created_at: Time.zone.now - 24.hours,
-              duration: 12
+              duration_minutes: 12 * 60
             )
           end
 

spec/jobs/delete_replies_spec.rb

@@ -7,7 +7,7 @@ describe Jobs::DeleteReplies do
 
   fab!(:topic) { Fabricate(:topic) }
   fab!(:topic_timer) do
-    Fabricate(:topic_timer, status_type: TopicTimer.types[:delete_replies], duration: 2, user: admin, topic: topic, execute_at: 2.days.from_now)
+    Fabricate(:topic_timer, status_type: TopicTimer.types[:delete_replies], duration_minutes: 2880, user: admin, topic: topic, execute_at: 2.days.from_now)
   end
 
   before do

spec/models/topic_spec.rb

@@ -1752,7 +1752,7 @@ describe Topic do
 
     it 'can take a number of hours as a string and can handle based on last post' do
       freeze_time now
-      topic.set_or_create_timer(TopicTimer.types[:close], nil, by_user: admin, based_on_last_post: true, duration: 18)
+      topic.set_or_create_timer(TopicTimer.types[:close], nil, by_user: admin, based_on_last_post: true, duration_minutes: '1080')
       expect(topic.topic_timers.first.execute_at).to eq_time(18.hours.from_now)
     end
 

spec/requests/topics_controller_spec.rb

@@ -3081,7 +3081,7 @@ RSpec.describe TopicsController do
         expect(DateTime.parse(json['execute_at']))
           .to eq_time(DateTime.parse(topic_status_update.execute_at.to_s))
 
-        expect(json['duration']).to eq(topic_status_update.duration)
+        expect(json['duration_minutes']).to eq(topic_status_update.duration_minutes)
         expect(json['closed']).to eq(topic.reload.closed)
       end
 
@@ -3099,13 +3099,13 @@ RSpec.describe TopicsController do
         json = response.parsed_body
 
         expect(json['execute_at']).to eq(nil)
-        expect(json['duration']).to eq(nil)
+        expect(json['duration_minutes']).to eq(nil)
         expect(json['closed']).to eq(topic.closed)
       end
 
       it 'should be able to create a topic status update with duration' do
         post "/t/#{topic.id}/timer.json", params: {
-          duration: 5,
+          duration_minutes: 7200,
           status_type: TopicTimer.types[7]
         }
 
@@ -3115,14 +3115,14 @@ RSpec.describe TopicsController do
 
         expect(topic_status_update.topic).to eq(topic)
         expect(topic_status_update.execute_at).to eq_time(5.days.from_now)
-        expect(topic_status_update.duration).to eq(5)
+        expect(topic_status_update.duration_minutes).to eq(7200)
 
         json = response.parsed_body
 
         expect(DateTime.parse(json['execute_at']))
           .to eq_time(DateTime.parse(topic_status_update.execute_at.to_s))
 
-        expect(json['duration']).to eq(topic_status_update.duration)
+        expect(json['duration_minutes']).to eq(topic_status_update.duration_minutes)
       end
 
       it 'should be able to delete a topic status update for delete_replies type' do

spec/services/topic_status_updater_spec.rb

@@ -59,7 +59,7 @@ describe TopicStatusUpdater do
     Fabricate(:post, topic: topic)
 
     topic.set_or_create_timer(
-      TopicTimer.types[:close], nil, based_on_last_post: true, duration: 10
+      TopicTimer.types[:close], nil, based_on_last_post: true, duration_minutes: 600
     )
 
     TopicStatusUpdater.new(topic, admin).update!("autoclosed", true)