Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Martin Brennan 2021-02-05 10:12:56 +10:00 committed by GitHub
parent 6f263653c6
commit 4af77f1e38
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 125 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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",
"useDuration",
"durationType"
"topicTimer.duration_minutes",
"useDuration"
)
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);
}
},
});

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

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

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

@ -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")

8
app/assets/javascripts/discourse/app/controllers/topic.js
View File

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

8
app/assets/javascripts/discourse/app/models/topic-timer.js
View File

@ -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) {
data.duration = duration;
if (durationMinutes) {
data.duration_minutes = durationMinutes;
}
return ajax({

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

@ -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>

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

@ -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">

2
app/assets/javascripts/discourse/app/templates/topic.hbs
View File

@ -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")}}

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

@ -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,

5
app/controllers/topics_controller.rb
View File

@ -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

4
app/jobs/regular/delete_replies.rb
View File

@ -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

43
app/models/topic.rb
View File

@ -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)
return delete_topic_timer(status_type, by_user: by_user) if time.blank? && duration.blank?
# * duration: TODO(2021-06-01): DEPRECATED - do not use
# * 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

6
app/models/topic_timer.rb
View File

@ -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
#

2
app/serializers/topic_timer_serializer.rb
View File

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

4
app/services/topic_status_updater.rb
View File

@ -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

16
db/migrate/20210203031628_add_duration_minutes_to_topic_timer.rb Normal file
View File

@ -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

4
lib/post_creator.rb
View File

@ -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

2
spec/components/post_creator_spec.rb
View File

@ -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

2
spec/jobs/delete_replies_spec.rb
View File

@ -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

2
spec/models/topic_spec.rb
View File

@ -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

10
spec/requests/topics_controller_spec.rb
View File

@ -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

2
spec/services/topic_status_updater_spec.rb
View File

@ -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)