diff --git a/app/assets/javascripts/discourse/components/auto-close-form.js.es6 b/app/assets/javascripts/discourse/components/auto-update-input.js.es6 similarity index 52% rename from app/assets/javascripts/discourse/components/auto-close-form.js.es6 rename to app/assets/javascripts/discourse/components/auto-update-input.js.es6 index f6ab041c7c9..7541d4988e4 100644 --- a/app/assets/javascripts/discourse/components/auto-close-form.js.es6 +++ b/app/assets/javascripts/discourse/components/auto-update-input.js.es6 @@ -3,33 +3,31 @@ import { observes } from "ember-addons/ember-computed-decorators"; export default Ember.Component.extend({ limited: false, - autoCloseValid: false, + inputValid: false, @computed("limited") - autoCloseUnits(limited) { - const key = limited ? "composer.auto_close.limited.units" : "composer.auto_close.all.units"; - return I18n.t(key); + inputUnitsKey(limited) { + return limited ? "topic.auto_update_input.limited.units" : "topic.auto_update_input.all.units"; }, @computed("limited") - autoCloseExamples(limited) { - const key = limited ? "composer.auto_close.limited.examples" : "composer.auto_close.all.examples"; - return I18n.t(key); + inputExamplesKey(limited) { + return limited ? "topic.auto_update_input.limited.examples" : "topic.auto_update_input.all.examples"; }, - @observes("autoCloseTime", "limited") - _updateAutoCloseValid() { - const limited = this.get("limited"), - autoCloseTime = this.get("autoCloseTime"), - isValid = this._isAutoCloseValid(autoCloseTime, limited); - this.set("autoCloseValid", isValid); + @observes("input", "limited") + _updateInputValid() { + this.set( + "inputValid", this._isInputValid(this.get("input"), this.get("limited")) + ); }, - _isAutoCloseValid(autoCloseTime, limited) { - const t = (autoCloseTime || "").toString().trim(); + _isInputValid(input, limited) { + const t = (input || "").toString().trim(); + if (t.length === 0) { - // "empty" is always valid return true; + // "empty" is always valid } else if (limited) { // only # of hours in limited mode return t.match(/^(\d+\.)?\d+$/); diff --git a/app/assets/javascripts/discourse/components/topic-closing.js.es6 b/app/assets/javascripts/discourse/components/topic-closing.js.es6 deleted file mode 100644 index f4ab5748ce4..00000000000 --- a/app/assets/javascripts/discourse/components/topic-closing.js.es6 +++ /dev/null @@ -1,51 +0,0 @@ -import { bufferedRender } from 'discourse-common/lib/buffered-render'; - -export default Ember.Component.extend(bufferedRender({ - elementId: 'topic-closing-info', - delayedRerender: null, - - rerenderTriggers: ['topic.closed', - 'topic.details.auto_close_at', - 'topic.details.auto_close_based_on_last_post', - 'topic.details.auto_close_hours'], - - buildBuffer(buffer) { - if (!!Ember.isEmpty(this.get('topic.details.auto_close_at'))) return; - if (this.get("topic.closed")) return; - - var autoCloseAt = moment(this.get('topic.details.auto_close_at')); - if (autoCloseAt < new Date()) return; - - var duration = moment.duration(autoCloseAt - moment()); - var minutesLeft = duration.asMinutes(); - var timeLeftString = duration.humanize(true); - var rerenderDelay = 1000; - - if (minutesLeft > 2160) { - rerenderDelay = 12 * 60 * 60000; - } else if (minutesLeft > 1410) { - rerenderDelay = 60 * 60000; - } else if (minutesLeft > 90) { - rerenderDelay = 30 * 60000; - } else if (minutesLeft > 2) { - rerenderDelay = 60000; - } - - var basedOnLastPost = this.get("topic.details.auto_close_based_on_last_post"); - var key = basedOnLastPost ? 'topic.auto_close_notice_based_on_last_post' : 'topic.auto_close_notice'; - var autoCloseHours = this.get("topic.details.auto_close_hours") || 0; - - buffer.push('

'); - buffer.push( I18n.t(key, { timeLeft: timeLeftString, duration: moment.duration(autoCloseHours, "hours").humanize() }) ); - buffer.push('

'); - - // TODO Sam: concerned this can cause a heavy rerender loop - this.set('delayedRerender', Em.run.later(this, this.rerender, rerenderDelay)); - }, - - willDestroyElement() { - if( this.delayedRerender ) { - Em.run.cancel(this.get('delayedRerender')); - } - } -})); diff --git a/app/assets/javascripts/discourse/components/topic-status-info.js.es6 b/app/assets/javascripts/discourse/components/topic-status-info.js.es6 new file mode 100644 index 00000000000..9545a022a0c --- /dev/null +++ b/app/assets/javascripts/discourse/components/topic-status-info.js.es6 @@ -0,0 +1,63 @@ +import { bufferedRender } from 'discourse-common/lib/buffered-render'; + +export default Ember.Component.extend(bufferedRender({ + elementId: 'topic-status-info', + delayedRerender: null, + + rerenderTriggers: ['topic.closed', + 'topic.topic_status_update.execute_at', + 'topic.topic_status_update.based_on_last_post', + 'topic.topic_status_update.duration'], + + buildBuffer(buffer) { + if (Ember.isEmpty(this.get('topic.topic_status_update.execute_at'))) return; + if (!this.get('topic.topic_status_update')) return; + + let statusUpdateAt = moment(this.get('topic.topic_status_update.execute_at')); + if (statusUpdateAt < new Date()) return; + + let duration = moment.duration(statusUpdateAt - moment()); + let minutesLeft = duration.asMinutes(); + let rerenderDelay = 1000; + + if (minutesLeft > 2160) { + rerenderDelay = 12 * 60 * 60000; + } else if (minutesLeft > 1410) { + rerenderDelay = 60 * 60000; + } else if (minutesLeft > 90) { + rerenderDelay = 30 * 60000; + } else if (minutesLeft > 2) { + rerenderDelay = 60000; + } + + let autoCloseHours = this.get("topic.topic_status_update.duration") || 0; + + buffer.push('

'); + + buffer.push(I18n.t(this._noticeKey(), { + timeLeft: duration.humanize(true), + duration: moment.duration(autoCloseHours, "hours").humanize() + })); + + buffer.push('

'); + + // TODO Sam: concerned this can cause a heavy rerender loop + this.set('delayedRerender', Em.run.later(this, this.rerender, rerenderDelay)); + }, + + willDestroyElement() { + if( this.delayedRerender ) { + Em.run.cancel(this.get('delayedRerender')); + } + }, + + _noticeKey() { + const statusType = this.get('topic.topic_status_update.status_type'); + + if (this.get("topic.topic_status_update.based_on_last_post")) { + return `topic.status_update_notice.auto_${statusType}_based_on_last_post`; + } else { + return `topic.status_update_notice.auto_${statusType}`; + } + } +})); diff --git a/app/assets/javascripts/discourse/controllers/edit-topic-auto-close.js.es6 b/app/assets/javascripts/discourse/controllers/edit-topic-auto-close.js.es6 deleted file mode 100644 index 81aa4f5b06a..00000000000 --- a/app/assets/javascripts/discourse/controllers/edit-topic-auto-close.js.es6 +++ /dev/null @@ -1,78 +0,0 @@ -import { ajax } from 'discourse/lib/ajax'; -import { observes } from "ember-addons/ember-computed-decorators"; -import ModalFunctionality from 'discourse/mixins/modal-functionality'; - -// Modal related to auto closing of topics -export default Ember.Controller.extend(ModalFunctionality, { - auto_close_valid: true, - auto_close_invalid: Em.computed.not('auto_close_valid'), - disable_submit: Em.computed.or('auto_close_invalid', 'loading'), - loading: false, - - @observes("model.details.auto_close_at", "model.details.auto_close_hours") - setAutoCloseTime() { - let autoCloseTime = null; - - if (this.get("model.details.auto_close_based_on_last_post")) { - autoCloseTime = this.get("model.details.auto_close_hours"); - } else if (this.get("model.details.auto_close_at")) { - const closeTime = new Date(this.get("model.details.auto_close_at")); - if (closeTime > new Date()) { - autoCloseTime = moment(closeTime).format("YYYY-MM-DD HH:mm"); - } - } - - this.set("model.auto_close_time", autoCloseTime); - }, - - actions: { - saveAutoClose() { this.setAutoClose(this.get("model.auto_close_time")); }, - removeAutoClose() { this.setAutoClose(null); } - }, - - setAutoClose(time) { - const self = this; - this.set('loading', true); - ajax({ - url: `/t/${this.get('model.id')}/autoclose`, - type: 'PUT', - dataType: 'json', - data: { - auto_close_time: time, - auto_close_based_on_last_post: this.get("model.details.auto_close_based_on_last_post"), - timezone_offset: (new Date().getTimezoneOffset()) - } - }).then(result => { - self.set('loading', false); - if (result.success) { - this.send('closeModal'); - this.set('model.details.auto_close_at', result.auto_close_at); - this.set('model.details.auto_close_hours', result.auto_close_hours); - } else { - bootbox.alert(I18n.t('composer.auto_close.error')); - } - }).catch(() => { - // TODO - incorrectly responds to network errors as bad input - bootbox.alert(I18n.t('composer.auto_close.error')); - self.set('loading', false); - }); - }, - - willCloseImmediately: function() { - if (!this.get('model.details.auto_close_based_on_last_post')) { - return false; - } - let closeDate = new Date(this.get('model.last_posted_at')); - closeDate.setHours(closeDate.getHours() + this.get('model.auto_close_time')); - return closeDate < new Date(); - }.property('model.details.auto_close_based_on_last_post', 'model.auto_close_time', 'model.last_posted_at'), - - willCloseI18n: function() { - if (this.get('model.details.auto_close_based_on_last_post')) { - let closeDate = new Date(this.get('model.last_posted_at')); - let diff = Math.round((new Date() - closeDate)/(1000*60*60)); - return I18n.t('topic.auto_close_immediate', {count: diff}); - } - }.property('model.details.auto_close_based_on_last_post', 'model.last_posted_at') - -}); diff --git a/app/assets/javascripts/discourse/controllers/edit-topic-status-update.js.es6 b/app/assets/javascripts/discourse/controllers/edit-topic-status-update.js.es6 new file mode 100644 index 00000000000..ad4c1d87510 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/edit-topic-status-update.js.es6 @@ -0,0 +1,101 @@ +import { default as computed, observes } from "ember-addons/ember-computed-decorators"; +import ModalFunctionality from 'discourse/mixins/modal-functionality'; +import TopicStatusUpdate from 'discourse/models/topic-status-update'; +import { popupAjaxError } from 'discourse/lib/ajax-error'; + +const CLOSE_STATUS_TYPE = 'close'; +const OPEN_STATUS_TYPE = 'open'; + +export default Ember.Controller.extend(ModalFunctionality, { + updateTimeValid: null, + updateTimeInvalid: Em.computed.not('updateTimeValid'), + loading: false, + updateTime: null, + topicStatusUpdate: Ember.computed.alias("model.topic_status_update"), + selection: Ember.computed.alias('model.topic_status_update.status_type'), + autoReopen: Ember.computed.equal('selection', OPEN_STATUS_TYPE), + autoClose: Ember.computed.equal('selection', CLOSE_STATUS_TYPE), + disableAutoReopen: Ember.computed.and('autoClose', 'updateTime'), + disableAutoClose: Ember.computed.and('autoReopen', 'updateTime'), + + @computed('topicStatusUpdate.based_on_last_post', 'updateTime', 'model.last_posted_at') + willCloseImmediately(basedOnLastPost, updateTime, lastPostedAt) { + if (!basedOnLastPost) { + return false; + } + const closeDate = new Date(lastPostedAt); + closeDate.setHours(closeDate.getHours() + updateTime); + return closeDate < new Date(); + }, + + @computed('topicStatusUpdate.based_on_last_post', 'model.last_posted_at') + willCloseI18n(basedOnLastPost, lastPostedAt) { + if (basedOnLastPost) { + const diff = Math.round((new Date() - new Date(lastPostedAt)) / (1000*60*60)); + return I18n.t('topic.auto_close_immediate', { count: diff }); + } + }, + + @computed('updateTime', 'updateTimeInvalid', 'loading') + saveDisabled(updateTime, updateTimeInvalid, loading) { + return Ember.isEmpty(updateTime) || updateTimeInvalid || loading; + }, + + @computed('autoReopen', 'autoClose') + removeStatusUpdateLabel(autoReopen, autoClose) { + if (autoReopen) { + return 'topic.auto_reopen.remove'; + } else if (autoClose) { + return 'topic.auto_close.remove'; + } + }, + + @observes("topicStatusUpdate.execute_at", "topicStatusUpdate.duration") + setAutoCloseTime() { + let time = null; + + if (this.get("topicStatusUpdate.based_on_last_post")) { + time = this.get("topicStatusUpdate.duration"); + } else if (this.get("topicStatusUpdate.execute_at")) { + const closeTime = new Date(this.get("topicStatusUpdate.execute_at")); + + if (closeTime > new Date()) { + time = moment(closeTime).format("YYYY-MM-DD HH:mm"); + } + } + + this.set("updateTime", time); + }, + + _setStatusUpdate(time, status_type) { + this.set('loading', true); + + TopicStatusUpdate.updateStatus( + this.get('model.id'), + time, + this.get('topicStatusUpdate.based_on_last_post'), + status_type + ).then(result => { + if (time) { + this.send('closeModal'); + this.set('topicStatusUpdate.execute_at', result.execute_at); + this.set('topicStatusUpdate.duration', result.duration); + } else { + this.set('topicStatusUpdate', Ember.Object.create({})); + this.set('selection', null); + } + }).catch(error => { + popupAjaxError(error); + }).finally(() => this.set('loading', false)); + }, + + actions: { + saveStatusUpdate() { + this._setStatusUpdate(this.get("updateTime"), this.get('selection')); + }, + + removeStatusUpdate() { + this._setStatusUpdate(null, this.get('selection')); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index 7d1c98142bf..a774cb8ad08 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/app/assets/javascripts/discourse/controllers/topic.js.es6 @@ -161,10 +161,6 @@ export default Ember.Controller.extend(SelectedPostsCount, BufferedContent, { return this.send(name, model); }, - openAutoClose() { - this.send('showAutoClose'); - }, - openFeatureTopic() { this.send('showFeatureTopic'); }, diff --git a/app/assets/javascripts/discourse/models/topic-status-update.js.es6 b/app/assets/javascripts/discourse/models/topic-status-update.js.es6 new file mode 100644 index 00000000000..cec7eb91a02 --- /dev/null +++ b/app/assets/javascripts/discourse/models/topic-status-update.js.es6 @@ -0,0 +1,24 @@ +import { ajax } from 'discourse/lib/ajax'; +import RestModel from 'discourse/models/rest'; + +const TopicStatusUpdate = RestModel.extend({}); + +TopicStatusUpdate.reopenClass({ + updateStatus(topicId, time, basedOnLastPost, statusType) { + let data = { + time: time, + timezone_offset: (new Date().getTimezoneOffset()), + status_type: statusType + }; + + if (basedOnLastPost) data.based_on_last_post = basedOnLastPost; + + return ajax({ + url: `/t/${topicId}/status_update`, + type: 'POST', + data + }); + } +}); + +export default TopicStatusUpdate; diff --git a/app/assets/javascripts/discourse/models/topic.js.es6 b/app/assets/javascripts/discourse/models/topic.js.es6 index 84545aa9ea9..a40aecc3f72 100644 --- a/app/assets/javascripts/discourse/models/topic.js.es6 +++ b/app/assets/javascripts/discourse/models/topic.js.es6 @@ -229,10 +229,6 @@ const Topic = RestModel.extend({ saveStatus(property, value, until) { if (property === 'closed') { this.incrementProperty('posts_count'); - - if (value === true) { - this.set('details.auto_close_at', null); - } } return ajax(this.get('url') + "/status", { type: 'PUT', diff --git a/app/assets/javascripts/discourse/routes/topic.js.es6 b/app/assets/javascripts/discourse/routes/topic.js.es6 index 105d0e6ba0e..edcbaba332c 100644 --- a/app/assets/javascripts/discourse/routes/topic.js.es6 +++ b/app/assets/javascripts/discourse/routes/topic.js.es6 @@ -50,9 +50,11 @@ const TopicRoute = Discourse.Route.extend({ this.controllerFor('flag').setProperties({ selected: null, flagTopic: true }); }, - showAutoClose() { - showModal('edit-topic-auto-close', { model: this.modelFor('topic') }); - this.controllerFor('modal').set('modalClass', 'edit-auto-close-modal'); + showTopicStatusUpdate() { + const model = this.modelFor('topic'); + if (!model.get('topic_status_update')) model.set('topic_status_update', Ember.Object.create()); + showModal('edit-topic-status-update', { model }); + this.controllerFor('modal').set('modalClass', 'topic-close-modal'); }, showChangeTimestamp() { diff --git a/app/assets/javascripts/discourse/templates/components/auto-close-form.hbs b/app/assets/javascripts/discourse/templates/components/auto-close-form.hbs deleted file mode 100644 index 34d53e2f70b..00000000000 --- a/app/assets/javascripts/discourse/templates/components/auto-close-form.hbs +++ /dev/null @@ -1,19 +0,0 @@ -
-
- -
-
- {{autoCloseExamples}} -
-
- -
-
diff --git a/app/assets/javascripts/discourse/templates/components/auto-update-input.hbs b/app/assets/javascripts/discourse/templates/components/auto-update-input.hbs new file mode 100644 index 00000000000..d9b9cec0e91 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/auto-update-input.hbs @@ -0,0 +1,25 @@ +
+
+ +
+ + {{#if inputExamplesKey}} +
+ {{i18n inputExamplesKey}} +
+ {{/if}} + + + {{#unless hideBasedOnLastPost}} +
+ +
+ {{/unless}} +
diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs index aae72706cd7..e1fec090805 100644 --- a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs +++ b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs @@ -1,8 +1,10 @@
- {{auto-close-form autoCloseTime=category.auto_close_hours - autoCloseBasedOnLastPost=category.auto_close_based_on_last_post - autoCloseExamples="" - limited="true" }} + {{auto-update-input + inputLabelKey='topic.auto_close.label' + input=category.auto_close_hours + basedOnLastPost=category.auto_close_based_on_last_post + inputExamplesKey='' + limited=true}}
diff --git a/app/assets/javascripts/discourse/templates/components/topic-footer-buttons.hbs b/app/assets/javascripts/discourse/templates/components/topic-footer-buttons.hbs index ec1e9eac88f..7e28cc0f9a6 100644 --- a/app/assets/javascripts/discourse/templates/components/topic-footer-buttons.hbs +++ b/app/assets/javascripts/discourse/templates/components/topic-footer-buttons.hbs @@ -8,7 +8,7 @@ toggleClosed=toggleClosed toggleArchived=toggleArchived toggleVisibility=toggleVisibility - showAutoClose=showAutoClose + showTopicStatusUpdate=showTopicStatusUpdate showFeatureTopic=showFeatureTopic showChangeTimestamp=showChangeTimestamp convertToPublicTopic=convertToPublicTopic diff --git a/app/assets/javascripts/discourse/templates/modal/edit-topic-auto-close.hbs b/app/assets/javascripts/discourse/templates/modal/edit-topic-auto-close.hbs deleted file mode 100644 index d5dcb92fa69..00000000000 --- a/app/assets/javascripts/discourse/templates/modal/edit-topic-auto-close.hbs +++ /dev/null @@ -1,20 +0,0 @@ -
- {{#d-modal-body title="topic.auto_close_title" autoFocus="false"}} - {{auto-close-form autoCloseTime=model.auto_close_time - autoCloseValid=auto_close_valid - autoCloseBasedOnLastPost=model.details.auto_close_based_on_last_post - limited=model.details.auto_close_based_on_last_post }} - {{#if willCloseImmediately}} -
- {{fa-icon "warning"}} - {{willCloseI18n}} -
- {{/if}} - {{/d-modal-body}} - -
diff --git a/app/assets/javascripts/discourse/templates/modal/edit-topic-status-update.hbs b/app/assets/javascripts/discourse/templates/modal/edit-topic-status-update.hbs new file mode 100644 index 00000000000..5c906045ce3 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/edit-topic-status-update.hbs @@ -0,0 +1,64 @@ +
+ {{#d-modal-body title="topic.topic_status_update.title" autoFocus="false"}} +
+ {{radio-button + disabled=disableAutoClose + name="auto-close" + id="auto-close" + value="close" + selection=selection}} + + + + {{radio-button + disabled=disableAutoReopen + name="auto-reopen" + id="auto-reopen" + value="open" + selection=selection}} + + +
+ + {{#if autoReopen}} + {{auto-update-input + inputLabelKey='topic.auto_reopen.label' + input=updateTime + inputValid=updateTimeValid + hideBasedOnLastPost=true + basedOnLastPost=false}} + {{else if autoClose}} + {{auto-update-input + inputLabelKey='topic.auto_close.label' + input=updateTime + inputValid=updateTimeValid + limited=topicStatusUpdate.based_on_last_post + basedOnLastPost=topicStatusUpdate.based_on_last_post}} + + {{#if willCloseImmediately}} +
+ {{fa-icon "warning"}} + {{willCloseI18n}} +
+ {{/if}} + {{/if}} + {{/d-modal-body}} + + +
diff --git a/app/assets/javascripts/discourse/templates/topic.hbs b/app/assets/javascripts/discourse/templates/topic.hbs index ff77c3ae0ed..c22ed994bb2 100644 --- a/app/assets/javascripts/discourse/templates/topic.hbs +++ b/app/assets/javascripts/discourse/templates/topic.hbs @@ -81,7 +81,7 @@ toggleClosed=(action "toggleClosed") toggleArchived=(action "toggleArchived") toggleVisibility=(action "toggleVisibility") - showAutoClose=(action "topicRouteAction" "showAutoClose") + showTopicStatusUpdate=(action "topicRouteAction" "showTopicStatusUpdate") showFeatureTopic=(action "topicRouteAction" "showFeatureTopic") showChangeTimestamp=(action "topicRouteAction" "showChangeTimestamp") convertToPublicTopic=(action "convertToPublicTopic") @@ -107,7 +107,7 @@ toggleClosed=(action "toggleClosed") toggleArchived=(action "toggleArchived") toggleVisibility=(action "toggleVisibility") - showAutoClose=(action "topicRouteAction" "showAutoClose") + showTopicStatusUpdate=(action "topicRouteAction" "showTopicStatusUpdate") showFeatureTopic=(action "topicRouteAction" "showFeatureTopic") showChangeTimestamp=(action "topicRouteAction" "showChangeTimestamp") convertToPublicTopic=(action "convertToPublicTopic") @@ -174,7 +174,7 @@ {{#conditional-loading-spinner condition=model.postStream.loadingFilter}} {{#if loadedAllPosts}} - {{topic-closing topic=model}} + {{topic-status-info topic=model}} {{#if session.showSignupCta}} {{! replace "Log In to Reply" with the infobox }} {{signup-cta}} @@ -188,7 +188,7 @@ toggleClosed=(action "toggleClosed") toggleArchived=(action "toggleArchived") toggleVisibility=(action "toggleVisibility") - showAutoClose=(action "topicRouteAction" "showAutoClose") + showTopicStatusUpdate=(action "topicRouteAction" "showTopicStatusUpdate") showFeatureTopic=(action "topicRouteAction" "showFeatureTopic") showChangeTimestamp=(action "topicRouteAction" "showChangeTimestamp") convertToPublicTopic=(action "convertToPublicTopic") diff --git a/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 b/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 index 699dd78142c..ce00e979de2 100644 --- a/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 +++ b/app/assets/javascripts/discourse/widgets/topic-admin-menu.js.es6 @@ -122,12 +122,13 @@ export default createWidget('topic-admin-menu', { action: 'toggleClosed', icon: 'lock', label: 'actions.close' }); - buttons.push({ className: 'topic-admin-autoclose', - action: 'showAutoClose', - icon: 'clock-o', - label: 'actions.auto_close' }); } + buttons.push({ className: 'topic-admin-status-update', + action: 'showTopicStatusUpdate', + icon: 'clock-o', + label: 'actions.timed_update' }); + const isPrivateMessage = topic.get('isPrivateMessage'); if (!isPrivateMessage && topic.get('visible')) { diff --git a/app/assets/stylesheets/common/base/compose.scss b/app/assets/stylesheets/common/base/compose.scss index bf1c8ea5c58..b0076abdb0b 100644 --- a/app/assets/stylesheets/common/base/compose.scss +++ b/app/assets/stylesheets/common/base/compose.scss @@ -133,7 +133,7 @@ div.ac-wrap { } } -.auto-close-fields { +.auto-update-input { div:not(:first-child) { margin-top: 10px; } diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss index 3eb8868bfa8..5aaca0e35ed 100644 --- a/app/assets/stylesheets/common/base/modal.scss +++ b/app/assets/stylesheets/common/base/modal.scss @@ -295,14 +295,29 @@ } } -.edit-auto-close-modal { +.topic-close-modal { + .radios { + padding-bottom: 20px; + display: inline-block; + + input[type='radio'] { + vertical-align: middle; + margin: 0px; + } + + label { + padding: 0 10px 0px 5px; + display: inline-block; + } + } + .btn.pull-right { margin-right: 10px; } form { margin: 0; } - .auto-close-fields { + .auto-update-input { i.fa-clock-o { font-size: 1.143em; } @@ -313,13 +328,13 @@ } .edit-category-modal { - .auto-close-fields, .num-featured-topics-fields, .position-fields { + .auto-update-input, .num-featured-topics-fields, .position-fields { input[type=text] { width: 50px; } } - .auto-close-fields label { + .auto-update-input label { font-size: .929em; } @@ -397,4 +412,3 @@ } } } - diff --git a/app/assets/stylesheets/common/printer-friendly.scss b/app/assets/stylesheets/common/printer-friendly.scss index 5f2b7f81ab5..6d62eb40d8d 100644 --- a/app/assets/stylesheets/common/printer-friendly.scss +++ b/app/assets/stylesheets/common/printer-friendly.scss @@ -16,7 +16,7 @@ .show-topic-admin, #topic-progress, .quote-controls, - #topic-closing-info, + #topic-status-info, div.lazyYT, .post-info.edits, .post-action, diff --git a/app/assets/stylesheets/desktop/compose.scss b/app/assets/stylesheets/desktop/compose.scss index 0f9bfb589b7..c8aa5306ea0 100644 --- a/app/assets/stylesheets/desktop/compose.scss +++ b/app/assets/stylesheets/desktop/compose.scss @@ -342,7 +342,7 @@ display: block; bottom: 8px; } - .auto-close-fields .examples { + .auto-update-input .examples { margin-top: 0; padding-bottom: 8px; } diff --git a/app/assets/stylesheets/desktop/topic.scss b/app/assets/stylesheets/desktop/topic.scss index ff8f81747a8..59d297ac479 100644 --- a/app/assets/stylesheets/desktop/topic.scss +++ b/app/assets/stylesheets/desktop/topic.scss @@ -79,7 +79,7 @@ } } -#topic-closing-info { +#topic-status-info { border-top: 1px solid dark-light-diff($primary, $secondary, 90%, -75%); padding-top: 10px; height: 20px; @@ -242,4 +242,3 @@ and (max-width : 485px) { max-width: 100%; } } - diff --git a/app/assets/stylesheets/mobile/topic.scss b/app/assets/stylesheets/mobile/topic.scss index 00210c6be54..4365009a6a3 100644 --- a/app/assets/stylesheets/mobile/topic.scss +++ b/app/assets/stylesheets/mobile/topic.scss @@ -43,7 +43,7 @@ clear: both; } -#topic-closing-info { +#topic-status-info { margin-left: 10px; } @@ -190,7 +190,7 @@ sup sup, sub sup, sup sub, sub sub { top: 0; } } // make mobile timeline top and bottom dates easier to select -.topic-timeline { +.topic-timeline { .start-date { font-size: 110%; padding: 5px; } .now-date { font-size: 110%; padding: 5px; } } diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index e3f70dc40b9..257744a359a 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -21,7 +21,7 @@ class TopicsController < ApplicationController :merge_topic, :clear_pin, :re_pin, - :autoclose, + :status_update, :bulk, :reset_new, :change_post_owners, @@ -284,20 +284,33 @@ class TopicsController < ApplicationController toggle_mute end - def autoclose - params.permit(:auto_close_time, :timezone_offset) - params.require(:auto_close_based_on_last_post) + def status_update + params.permit(:time, :timezone_offset, :based_on_last_post) + params.require(:status_type) - topic = Topic.find_by(id: params[:topic_id].to_i) + status_type = + begin + TopicStatusUpdate.types.fetch(params[:status_type].to_sym) + rescue + invalid_param(:status_type) + end + + topic = Topic.find_by(id: params[:topic_id]) guardian.ensure_can_moderate!(topic) - topic.auto_close_based_on_last_post = params[:auto_close_based_on_last_post] - topic.set_auto_close(params[:auto_close_time], {by_user: current_user, timezone_offset: params[:timezone_offset] ? params[:timezone_offset].to_i : nil}) + topic_status_update = topic.set_or_create_status_update( + status_type, + params[:time], + by_user: current_user, + timezone_offset: params[:timezone_offset]&.to_i, + based_on_last_post: params[:based_on_last_post] + ) if topic.save render json: success_json.merge!({ - auto_close_at: topic.auto_close_at, - auto_close_hours: topic.auto_close_hours + execute_at: topic_status_update&.execute_at, + duration: topic_status_update&.duration, + based_on_last_post: topic_status_update&.based_on_last_post }) else render_json_error(topic) diff --git a/app/jobs/regular/close_topic.rb b/app/jobs/regular/close_topic.rb deleted file mode 100644 index 0703c971f55..00000000000 --- a/app/jobs/regular/close_topic.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Jobs - class CloseTopic < Jobs::Base - - def execute(args) - if topic = Topic.find_by(id: args[:topic_id]) - closer = User.find_by(id: args[:user_id]) - guardian = Guardian.new(closer) - unless guardian.can_close?(topic) - closer = Discourse.system_user - end - topic.auto_close(closer) - end - end - - end -end diff --git a/app/jobs/regular/toggle_topic_closed.rb b/app/jobs/regular/toggle_topic_closed.rb new file mode 100644 index 00000000000..84b946fd8ad --- /dev/null +++ b/app/jobs/regular/toggle_topic_closed.rb @@ -0,0 +1,22 @@ +module Jobs + class ToggleTopicClosed < Jobs::Base + def execute(args) + topic_status_update = TopicStatusUpdate.find_by(id: args[:topic_status_update_id]) + raise Discourse::InvalidParameters.new(:topic_status_update_id) if topic_status_update.blank? + + return if topic_status_update.execute_at > Time.zone.now + + topic = topic_status_update.topic + return if topic.blank? + + state = !!args[:state] + return if topic.closed == state + + user = topic_status_update.user + + if Guardian.new(user).can_close?(topic) + topic.update_status('autoclosed', state, user) + end + end + end +end diff --git a/app/jobs/scheduled/periodical_updates.rb b/app/jobs/scheduled/periodical_updates.rb index 70c634e738c..fbf6f80f06d 100644 --- a/app/jobs/scheduled/periodical_updates.rb +++ b/app/jobs/scheduled/periodical_updates.rb @@ -24,8 +24,8 @@ module Jobs args[:max_topic_length] = 500 unless self.class.should_update_long_topics? ScoreCalculator.new.calculate(args) - # Automatically close stuff that we missed - Topic.auto_close + # Re-run stuff that we missed + TopicStatusUpdate.ensure_consistency! # Forces rebake of old posts where needed, as long as no system avatars need updating unless UserAvatar.where("last_gravatar_download_attempt IS NULL").limit(1).first diff --git a/app/models/category.rb b/app/models/category.rb index fb5c3309210..2a48c6a53af 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -203,7 +203,7 @@ SQL t = Topic.new(title: I18n.t("category.topic_prefix", category: name), user: user, pinned_at: Time.now, category_id: id) t.skip_callbacks = true t.ignore_category_auto_close = true - t.set_auto_close(nil) + t.set_or_create_status_update(TopicStatusUpdate.types[:close], nil) t.save!(validate: false) update_column(:topic_id, t.id) t.posts.create(raw: post_template, user: user) diff --git a/app/models/topic.rb b/app/models/topic.rb index 437ddaf5341..e5a95d70ee7 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -110,15 +110,14 @@ class Topic < ActiveRecord::Base belongs_to :featured_user2, class_name: 'User', foreign_key: :featured_user2_id belongs_to :featured_user3, class_name: 'User', foreign_key: :featured_user3_id belongs_to :featured_user4, class_name: 'User', foreign_key: :featured_user4_id - belongs_to :auto_close_user, class_name: 'User', foreign_key: :auto_close_user_id has_many :topic_users has_many :topic_links has_many :topic_invites has_many :invites, through: :topic_invites, source: :invite + has_many :topic_status_updates, dependent: :destroy has_one :warning - has_one :first_post, -> {where post_number: 1}, class_name: Post # When we want to temporarily attach some data to a forum topic (usually before serialization) @@ -175,7 +174,6 @@ class Topic < ActiveRecord::Base before_save do unless skip_callbacks - cancel_auto_close_job ensure_topic_has_a_category end if title_changed? @@ -184,10 +182,6 @@ class Topic < ActiveRecord::Base end after_save do - unless skip_callbacks - schedule_auto_close_job - end - banner = "banner".freeze if archetype_was == banner || archetype == banner @@ -210,9 +204,16 @@ class Topic < ActiveRecord::Base end def inherit_auto_close_from_category - if !@ignore_category_auto_close && self.category && self.category.auto_close_hours && self.auto_close_at.nil? - self.auto_close_based_on_last_post = self.category.auto_close_based_on_last_post - set_auto_close(self.category.auto_close_hours) + if !@ignore_category_auto_close && + self.category && + self.category.auto_close_hours && + !topic_status_update&.execute_at + + self.set_or_create_status_update( + TopicStatusUpdate.types[:close], + self.category.auto_close_hours, + based_on_last_post: self.category.auto_close_based_on_last_post + ) end end @@ -224,20 +225,6 @@ class Topic < ActiveRecord::Base end end - def cancel_auto_close_job - if (auto_close_at_changed? && !auto_close_at_was.nil?) || (auto_close_user_id_changed? && auto_close_at) - self.auto_close_started_at ||= Time.zone.now if auto_close_at - Jobs.cancel_scheduled_job(:close_topic, topic_id: id) - end - end - - def schedule_auto_close_job - if auto_close_at && (auto_close_at_changed? || auto_close_user_id_changed?) - options = { topic_id: id, user_id: auto_close_user_id || user_id } - Jobs.enqueue_at(auto_close_at, :close_topic, options) - end - end - def ensure_topic_has_a_category if category_id.nil? && (archetype.nil? || archetype == Archetype.default) self.category_id = SiteSetting.uncategorized_category_id @@ -470,7 +457,7 @@ class Topic < ActiveRecord::Base end def update_status(status, enabled, user, opts={}) - TopicStatusUpdate.new(self, user).update!(status, enabled, opts) + TopicStatusUpdater.new(self, user).update!(status, enabled, opts) DiscourseEvent.trigger(:topic_status_updated, self.id, status, enabled) end @@ -951,91 +938,81 @@ SQL Topic.where("pinned_until < now()").update_all(pinned_at: nil, pinned_globally: false, pinned_until: nil) end - def self.auto_close - Topic.where("NOT closed AND auto_close_at < ? AND auto_close_user_id IS NOT NULL", 1.minute.ago).each do |t| - t.auto_close - end + def topic_status_update + @topic_status_update ||= topic_status_updates.where('deleted_at IS NULL').first end - def auto_close(closer = nil) - if auto_close_at && !closed? && !deleted_at && auto_close_at < 5.minutes.from_now - closer ||= auto_close_user - if Guardian.new(closer).can_moderate?(self) - update_status('autoclosed', true, closer) - end - end - end - - # Valid arguments for the auto close time: - # * An integer, which is the number of hours from now to close the topic. - # * A time, like "12:00", which is the time at which the topic will close in the current day + # Valid arguments for the time: + # * An integer, which is the number of hours from now to update the topic's status. + # * A time, like "12:00", which is the time at which the topic's status will update in the current day # or the next day if that time has already passed today. - # * A timestamp, like "2013-11-25 13:00", when the topic should close. + # * A timestamp, like "2013-11-25 13:00", when the topic's status should update. # * A timestamp with timezone in JSON format. (e.g., "2013-11-26T21:00:00.000Z") - # * nil, to prevent the topic from automatically closing. + # * `nil` to delete the topic's status update. # Options: - # * by_user: User who is setting the auto close time + # * by_user: User who is setting the topic's status update. # * timezone_offset: (Integer) offset from UTC in minutes of the given argument. Default 0. - def set_auto_close(arg, opts={}) - self.auto_close_hours = nil - by_user = opts[:by_user] - offset_minutes = opts[:timezone_offset] + def set_or_create_status_update(status_type, time, by_user: nil, timezone_offset: 0, based_on_last_post: false) + topic_status_update = TopicStatusUpdate.find_or_initialize_by( + status_type: status_type, + topic: self + ) + + if time.blank? + topic_status_update.trash!(trashed_by: by_user || Discourse.system_user) + return + end + + time_now = Time.zone.now + topic_status_update.based_on_last_post = !based_on_last_post.blank? + + if topic_status_update.based_on_last_post + num_hours = time.to_f - if self.auto_close_based_on_last_post - num_hours = arg.to_f if num_hours > 0 - last_post_created_at = self.ordered_posts.last.try(:created_at) || Time.zone.now - self.auto_close_at = last_post_created_at + num_hours.hours - self.auto_close_hours = num_hours - else - self.auto_close_at = nil + last_post_created_at = self.ordered_posts.last.created_at || time_now + topic_status_update.execute_at = last_post_created_at + num_hours.hours + topic_status_update.created_at = last_post_created_at end else utc = Time.find_zone("UTC") - if arg.is_a?(String) && m = /^(\d{1,2}):(\d{2})(?:\s*[AP]M)?$/i.match(arg.strip) + is_timestamp = time.is_a?(String) + now = utc.now + + if is_timestamp && m = /^(\d{1,2}):(\d{2})(?:\s*[AP]M)?$/i.match(time.strip) # a time of day in client's time zone, like "15:00" - now = utc.now - self.auto_close_at = utc.local(now.year, now.month, now.day, m[1].to_i, m[2].to_i) - self.auto_close_at += offset_minutes * 60 if offset_minutes - self.auto_close_at += 1.day if self.auto_close_at < now - self.auto_close_hours = -1 - elsif arg.is_a?(String) && arg.include?("-") && timestamp = utc.parse(arg) + topic_status_update.execute_at = utc.local(now.year, now.month, now.day, m[1].to_i, m[2].to_i) + topic_status_update.execute_at += timezone_offset * 60 if timezone_offset + topic_status_update.execute_at += 1.day if topic_status_update.execute_at < now + elsif is_timestamp && time.include?("-") && timestamp = utc.parse(time) # a timestamp in client's time zone, like "2015-5-27 12:00" - self.auto_close_at = timestamp - self.auto_close_at += offset_minutes * 60 if offset_minutes - self.auto_close_hours = -1 - self.errors.add(:auto_close_at, :invalid) if timestamp < Time.zone.now + topic_status_update.execute_at = timestamp + topic_status_update.execute_at += timezone_offset * 60 if timezone_offset + topic_status_update.errors.add(:execute_at, :invalid) if timestamp < now else - num_hours = arg.to_f + num_hours = time.to_f + if num_hours > 0 - self.auto_close_at = num_hours.hours.from_now - self.auto_close_hours = num_hours - else - self.auto_close_at = nil + topic_status_update.execute_at = num_hours.hours.from_now end end end - if self.auto_close_at.nil? - self.auto_close_started_at = nil - else - if self.auto_close_based_on_last_post - self.auto_close_started_at = Time.zone.now + if topic_status_update.execute_at + if by_user&.staff? || by_user&.trust_level == TrustLevel[4] + topic_status_update.user = by_user else - self.auto_close_started_at ||= Time.zone.now - end - if by_user.try(:staff?) || by_user.try(:trust_level) == TrustLevel[4] - self.auto_close_user = by_user - else - self.auto_close_user ||= (self.user.staff? || self.user.trust_level == TrustLevel[4] ? self.user : Discourse.system_user) + topic_status_update.user ||= (self.user.staff? || self.user.trust_level == TrustLevel[4] ? self.user : Discourse.system_user) end - if self.auto_close_at.try(:<, Time.zone.now) - auto_close(auto_close_user) + if self.persisted? + topic_status_update.save! + else + self.topic_status_updates << topic_status_update end + + topic_status_update end - - self end def read_restricted_category? @@ -1214,56 +1191,51 @@ end # # Table name: topics # -# id :integer not null, primary key -# title :string not null -# last_posted_at :datetime -# created_at :datetime not null -# updated_at :datetime not null -# views :integer default(0), not null -# posts_count :integer default(0), not null -# user_id :integer -# last_post_user_id :integer not null -# reply_count :integer default(0), not null -# featured_user1_id :integer -# featured_user2_id :integer -# featured_user3_id :integer -# avg_time :integer -# deleted_at :datetime -# highest_post_number :integer default(0), not null -# image_url :string -# like_count :integer default(0), not null -# incoming_link_count :integer default(0), not null -# category_id :integer -# visible :boolean default(TRUE), not null -# moderator_posts_count :integer default(0), not null -# closed :boolean default(FALSE), not null -# archived :boolean default(FALSE), not null -# bumped_at :datetime not null -# has_summary :boolean default(FALSE), not null -# vote_count :integer default(0), not null -# archetype :string default("regular"), not null -# featured_user4_id :integer -# notify_moderators_count :integer default(0), not null -# spam_count :integer default(0), not null -# pinned_at :datetime -# score :float -# percent_rank :float default(1.0), not null -# subtype :string -# slug :string -# auto_close_at :datetime -# auto_close_user_id :integer -# auto_close_started_at :datetime -# deleted_by_id :integer -# participant_count :integer default(1) -# word_count :integer -# excerpt :string(1000) -# pinned_globally :boolean default(FALSE), not null -# auto_close_based_on_last_post :boolean default(FALSE) -# auto_close_hours :float -# pinned_until :datetime -# fancy_title :string(400) -# highest_staff_post_number :integer default(0), not null -# featured_link :string +# id :integer not null, primary key +# title :string not null +# last_posted_at :datetime +# created_at :datetime not null +# updated_at :datetime not null +# views :integer default(0), not null +# posts_count :integer default(0), not null +# user_id :integer +# last_post_user_id :integer not null +# reply_count :integer default(0), not null +# featured_user1_id :integer +# featured_user2_id :integer +# featured_user3_id :integer +# avg_time :integer +# deleted_at :datetime +# highest_post_number :integer default(0), not null +# image_url :string +# like_count :integer default(0), not null +# incoming_link_count :integer default(0), not null +# category_id :integer +# visible :boolean default(TRUE), not null +# moderator_posts_count :integer default(0), not null +# closed :boolean default(FALSE), not null +# archived :boolean default(FALSE), not null +# bumped_at :datetime not null +# has_summary :boolean default(FALSE), not null +# vote_count :integer default(0), not null +# archetype :string default("regular"), not null +# featured_user4_id :integer +# notify_moderators_count :integer default(0), not null +# spam_count :integer default(0), not null +# pinned_at :datetime +# score :float +# percent_rank :float default(1.0), not null +# subtype :string +# slug :string +# deleted_by_id :integer +# participant_count :integer default(1) +# word_count :integer +# excerpt :string(1000) +# pinned_globally :boolean default(FALSE), not null +# pinned_until :datetime +# fancy_title :string(400) +# highest_staff_post_number :integer default(0), not null +# featured_link :string # # Indexes # diff --git a/app/models/topic_status_update.rb b/app/models/topic_status_update.rb index bfd438bbae9..7c8417423b8 100644 --- a/app/models/topic_status_update.rb +++ b/app/models/topic_status_update.rb @@ -1,116 +1,110 @@ -TopicStatusUpdate = Struct.new(:topic, :user) do - def update!(status, enabled, opts={}) - status = Status.new(status, enabled) +class TopicStatusUpdate < ActiveRecord::Base + include Trashable - Topic.transaction do - change(status, opts) - highest_post_number = topic.highest_post_number - create_moderator_post_for(status, opts[:message]) - update_read_state_for(status, highest_post_number) + belongs_to :user + belongs_to :topic + + validates :user_id, presence: true + validates :topic_id, presence: true + validates :execute_at, presence: true + validates :status_type, presence: true + + validates :status_type, uniqueness: { scope: [:topic_id, :deleted_at] } + + validate :ensure_update_will_happen + + before_save do + self.created_at ||= Time.zone.now if execute_at + + if (execute_at_changed? && !execute_at_was.nil?) || user_id_changed? + self.send("cancel_auto_#{self.class.types[status_type]}_job") + end + end + + after_save do + if execute_at_changed? || user_id_changed? + now = Time.zone.now + time = execute_at < now ? now : execute_at + + self.send("schedule_auto_#{self.class.types[status_type]}_job", time) + end + end + + def self.types + @types ||= Enum.new( + close: 1, + open: 2 + ) + end + + def self.ensure_consistency! + TopicStatusUpdate.where("execute_at < ?", Time.zone.now).find_each do |topic_status_update| + topic_status_update.send( + "schedule_auto_#{self.types[topic_status_update.status_type]}_job", + topic_status_update.execute_at + ) + end + end + + def duration + if (self.execute_at && self.created_at) + ((self.execute_at - self.created_at) / 1.hour).round(2) + else + 0 end end private - def change(status, opts={}) - if status.pinned? || status.pinned_globally? - topic.update_pinned(status.enabled?, status.pinned_globally?, opts[:until]) - elsif status.autoclosed? - topic.update_column('closed', status.enabled?) - else - topic.update_column(status.name, status.enabled?) - end - - if topic.auto_close_at && (status.reopening_topic? || status.manually_closing_topic?) - topic.reload.set_auto_close(nil).save - end - - # remove featured topics if we close/archive/make them invisible. Previously we used - # to run the whole featuring logic but that could be very slow and have concurrency - # errors on large sites with many autocloses and topics being created. - if ((status.enabled? && (status.autoclosed? || status.closed? || status.archived?)) || - (status.disabled? && status.visible?)) - CategoryFeaturedTopic.where(topic_id: topic.id).delete_all - end - end - - def create_moderator_post_for(status, message=nil) - topic.add_moderator_post(user, message || message_for(status), options_for(status)) - topic.reload - end - - def update_read_state_for(status, old_highest_read) - if status.autoclosed? - # let's pretend all the people that read up to the autoclose message - # actually read the topic - PostTiming.pretend_read(topic.id, old_highest_read, topic.highest_post_number) - end - end - - def message_for(status) - if status.autoclosed? - locale_key = status.locale_key - locale_key << "_lastpost" if topic.auto_close_based_on_last_post - message_for_autoclosed(locale_key) - end - end - - def message_for_autoclosed(locale_key) - num_minutes = (( - if topic.auto_close_based_on_last_post - topic.auto_close_hours.hours - elsif topic.auto_close_started_at - Time.zone.now - topic.auto_close_started_at - else - Time.zone.now - topic.created_at - end - ) / 1.minute).round - - if num_minutes.minutes >= 2.days - I18n.t("#{locale_key}_days", count: (num_minutes.minutes / 1.day).round) - else - num_hours = (num_minutes.minutes / 1.hour).round - if num_hours >= 2 - I18n.t("#{locale_key}_hours", count: num_hours) - else - I18n.t("#{locale_key}_minutes", count: num_minutes) + def ensure_update_will_happen + if created_at && (execute_at < created_at) + errors.add(:execute_at, I18n.t( + 'activerecord.errors.models.topic_status_update.attributes.execute_at.in_the_past' + )) end end - end - def options_for(status) - { bump: status.reopening_topic?, - post_type: Post.types[:small_action], - action_code: status.action_code } - end + def cancel_auto_close_job + Jobs.cancel_scheduled_job(:toggle_topic_closed, topic_status_update_id: id) + end + alias_method :cancel_auto_open_job, :cancel_auto_close_job - Status = Struct.new(:name, :enabled) do - %w(pinned_globally pinned autoclosed closed visible archived).each do |status| - define_method("#{status}?") { name == status } + def schedule_auto_open_job(time) + topic.update_status('closed', true, user) if !topic.closed + + Jobs.enqueue_at(time, :toggle_topic_closed, + topic_status_update_id: id, + state: false + ) end - def enabled? - enabled - end + def schedule_auto_close_job(time) + topic.update_status('closed', false, user) if topic.closed - def disabled? - !enabled? + Jobs.enqueue_at(time, :toggle_topic_closed, + topic_status_update_id: id, + state: true + ) end - - def action_code - "#{name}.#{enabled? ? 'enabled' : 'disabled'}" - end - - def locale_key - "topic_statuses.#{action_code.tr('.', '_')}" - end - - def reopening_topic? - (closed? || autoclosed?) && disabled? - end - - def manually_closing_topic? - closed? && enabled? - end - end end + +# == Schema Information +# +# Table name: topic_status_updates +# +# id :integer not null, primary key +# execute_at :datetime not null +# status_type :integer not null +# user_id :integer not null +# topic_id :integer not null +# based_on_last_post :boolean default(FALSE), not null +# deleted_at :datetime +# deleted_by_id :integer +# created_at :datetime +# updated_at :datetime +# +# Indexes +# +# idx_topic_id_status_type_deleted_at (topic_id,status_type) UNIQUE +# index_topic_status_updates_on_user_id (user_id) +# diff --git a/app/serializers/topic_status_update_serializer.rb b/app/serializers/topic_status_update_serializer.rb new file mode 100644 index 00000000000..4b6335d2a63 --- /dev/null +++ b/app/serializers/topic_status_update_serializer.rb @@ -0,0 +1,11 @@ +class TopicStatusUpdateSerializer < ApplicationSerializer + attributes :id, + :execute_at, + :duration, + :based_on_last_post, + :status_type + + def status_type + TopicStatusUpdate.types[object.status_type] + end +end diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 586eb35baad..848c3e3f02e 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -57,16 +57,16 @@ class TopicViewSerializer < ApplicationSerializer :bookmarked, :message_archived, :tags, - :featured_link + :featured_link, + :topic_status_update # TODO: Split off into proper object / serializer def details + topic = object.topic + result = { - auto_close_at: object.topic.auto_close_at, - auto_close_hours: object.topic.auto_close_hours, - auto_close_based_on_last_post: object.topic.auto_close_based_on_last_post, - created_by: BasicUserSerializer.new(object.topic.user, scope: scope, root: false), - last_poster: BasicUserSerializer.new(object.topic.last_poster, scope: scope, root: false) + created_by: BasicUserSerializer.new(topic.user, scope: scope, root: false), + last_poster: BasicUserSerializer.new(topic.last_poster, scope: scope, root: false) } if object.topic.private_message? @@ -246,6 +246,12 @@ class TopicViewSerializer < ApplicationSerializer SiteSetting.tagging_enabled end + def topic_status_update + TopicStatusUpdateSerializer.new( + object.topic.topic_status_update, root: false + ) + end + def tags object.topic.tags.map(&:name) end diff --git a/app/services/topic_status_updater.rb b/app/services/topic_status_updater.rb new file mode 100644 index 00000000000..5bd72a7b4bb --- /dev/null +++ b/app/services/topic_status_updater.rb @@ -0,0 +1,131 @@ +TopicStatusUpdater = Struct.new(:topic, :user) do + def update!(status, enabled, opts={}) + status = Status.new(status, enabled) + + @topic_status_update = topic.topic_status_update + + Topic.transaction do + change(status, opts) + highest_post_number = topic.highest_post_number + create_moderator_post_for(status, opts[:message]) + update_read_state_for(status, highest_post_number) + end + end + + private + + def change(status, opts={}) + if status.pinned? || status.pinned_globally? + topic.update_pinned(status.enabled?, status.pinned_globally?, opts[:until]) + elsif status.autoclosed? + topic.update_column('closed', status.enabled?) + else + topic.update_column(status.name, status.enabled?) + end + + if @topic_status_update + if status.manually_closing_topic? || status.closing_topic? + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], nil) + elsif status.manually_opening_topic? || status.opening_topic? + topic.set_or_create_status_update(TopicStatusUpdate.types[:open], nil) + end + end + + # remove featured topics if we close/archive/make them invisible. Previously we used + # to run the whole featuring logic but that could be very slow and have concurrency + # errors on large sites with many autocloses and topics being created. + if ((status.enabled? && (status.autoclosed? || status.closed? || status.archived?)) || + (status.disabled? && status.visible?)) + CategoryFeaturedTopic.where(topic_id: topic.id).delete_all + end + end + + def create_moderator_post_for(status, message=nil) + topic.add_moderator_post(user, message || message_for(status), options_for(status)) + topic.reload + end + + def update_read_state_for(status, old_highest_read) + if status.autoclosed? && status.enabled? + # let's pretend all the people that read up to the autoclose message + # actually read the topic + PostTiming.pretend_read(topic.id, old_highest_read, topic.highest_post_number) + end + end + + def message_for(status) + if status.autoclosed? + locale_key = status.locale_key + locale_key << "_lastpost" if @topic_status_update&.based_on_last_post + message_for_autoclosed(locale_key) + end + end + + def message_for_autoclosed(locale_key) + num_minutes = + if @topic_status_update&.based_on_last_post + @topic_status_update.duration.hours + elsif @topic_status_update&.created_at + Time.zone.now - @topic_status_update.created_at + else + Time.zone.now - topic.created_at + end + + num_minutes = (num_minutes / 1.minute).round + + if num_minutes.minutes >= 2.days + I18n.t("#{locale_key}_days", count: (num_minutes.minutes / 1.day).round) + else + num_hours = (num_minutes.minutes / 1.hour).round + if num_hours >= 2 + I18n.t("#{locale_key}_hours", count: num_hours) + else + I18n.t("#{locale_key}_minutes", count: num_minutes) + end + end + end + + def options_for(status) + { bump: status.opening_topic?, + post_type: Post.types[:small_action], + action_code: status.action_code } + end + + Status = Struct.new(:name, :enabled) do + %w(pinned_globally pinned autoclosed closed visible archived).each do |status| + define_method("#{status}?") { name == status } + end + + def enabled? + enabled + end + + def disabled? + !enabled? + end + + def action_code + "#{name}.#{enabled? ? 'enabled' : 'disabled'}" + end + + def locale_key + "topic_statuses.#{action_code.tr('.', '_')}" + end + + def opening_topic? + (closed? || autoclosed?) && disabled? + end + + def closing_topic? + (closed? || autoclosed?) && enabled? + end + + def manually_closing_topic? + closed? && enabled? + end + + def manually_opening_topic? + closed? && disabled? + end + end +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d57c7ffe36b..c6cdfaebf36 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -6,7 +6,7 @@ # To work with us on translations, see: https://www.transifex.com/projects/p/discourse-org/ # # This is a "source" file, which is used by Transifex to get translations for other languages. -# After this file is changed, it needs to be pushed by a maintainer to Transifex: +# After this file is changed, ift needs to be pushed by a maintainer to Transifex: # # tx push -s # @@ -1190,16 +1190,6 @@ en: body: "Right now this message is only being sent to yourself!" admin_options_title: "Optional staff settings for this topic" - auto_close: - label: "Auto-close topic time:" - error: "Please enter a valid value." - based_on_last_post: "Don't close until the last post in the topic is at least this old." - all: - units: "" - examples: 'Enter number of hours (24), absolute time (17:30) or timestamp (2013-11-22 14:00).' - limited: - units: "(# of hours)" - examples: 'Enter number of hours (24).' notifications: title: "notifications of @name mentions, replies to your posts and topics, messages, etc" @@ -1479,11 +1469,36 @@ en: jump_reply_down: jump to later reply deleted: "The topic has been deleted" - auto_close_notice: "This topic will automatically close %{timeLeft}." - auto_close_notice_based_on_last_post: "This topic will close %{duration} after the last reply." + topic_status_update: + title: "Defer Topic Status Update" + save: "Set Topic Status Update" + close: + title: "Close Topic" + save: "Close topic" + auto_update_input: + limited: + units: "(# of hours)" + examples: 'Enter number of hours (24).' + all: + units: "" + examples: 'Enter number of hours (24), absolute time (17:30) or timestamp (2013-11-22 14:00).' + auto_reopen: + title: "Auto-Reopen Topic" + label: "Auto-reopen topic time:" + remove: "Don't Auto-Reopen This Topic" + auto_close: + title: "Auto-Close Topic" + label: "Auto-close topic time:" + error: "Please enter a valid value." + based_on_last_post: "Don't close until the last post in the topic is at least this old." + remove: "Don't Auto-Close This Topic" + + status_update_notice: + auto_open: "This topic will automatically open %{timeLeft}." + auto_close: "This topic will automatically close %{timeLeft}." + auto_open_based_on_last_post: "This topic will open %{duration} after the last reply." + auto_close_based_on_last_post: "This topic will close %{duration} after the last reply." auto_close_title: 'Auto-Close Settings' - auto_close_save: "Save" - auto_close_remove: "Don't Auto-Close This Topic" auto_close_immediate: one: "The last post in the topic is already 1 hour old, so the topic will be closed immediately." other: "The last post in the topic is already %{count} hours old, so the topic will be closed immediately." @@ -1556,7 +1571,7 @@ en: open: "Open Topic" close: "Close Topic" multi_select: "Select Posts…" - auto_close: "Auto Close…" + timed_update: "Defer Status Update..." pin: "Pin Topic…" unpin: "Un-Pin Topic…" unarchive: "Unarchive Topic" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e042e5742b2..12ee021f749 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -387,7 +387,7 @@ en: invalid: "is not a valid color" post_reply: base: - different_topic: "Post and reply must belong to the same topic." + different_topic: "Post and reply must be.long to the same topic." web_hook: attributes: payload_url: @@ -396,6 +396,10 @@ en: attributes: name: taken: is already in use by another emoji + topic_status_update: + attributes: + execute_at: + in_the_past: "must be in the future." user_profile: no_info_me: "
the About Me field of your profile is currently blank, would you like to fill it out?
" @@ -1610,6 +1614,26 @@ en: autoclosed_enabled_lastpost_minutes: one: "This topic was automatically closed 1 minute after the last reply. New replies are no longer allowed." other: "This topic was automatically closed %{count} minutes after the last reply. New replies are no longer allowed." + + autoclosed_disabled_days: + one: "This topic was automatically opened after 1 day." + other: "This topic was automatically opened after %{count} days." + autoclosed_disabled_hours: + one: "This topic was automatically opened after 1 hour." + other: "This topic was automatically opened after %{count} hours." + autoclosed_disabled_minutes: + one: "This topic was automatically opened after 1 minute." + other: "This topic was automatically opened after %{count} minutes." + autoclosed_disabled_lastpost_days: + one: "This topic was automatically opened 1 day after the last reply." + other: "This topic was automatically opened %{count} days after the last reply." + autoclosed_disabled_lastpost_hours: + one: "This topic was automatically opened 1 hour after the last reply." + other: "This topic was automatically opened %{count} hours after the last reply." + autoclosed_disabled_lastpost_minutes: + one: "This topic was automatically opened 1 minute after the last reply." + other: "This topic was automatically opened %{count} minutes after the last reply." + autoclosed_disabled: "This topic is now opened. New replies are allowed." autoclosed_disabled_lastpost: "This topic is now opened. New replies are allowed." pinned_enabled: "This topic is now pinned. It will appear at the top of its category until it is unpinned by staff for everyone, or by individual users for themselves." diff --git a/config/routes.rb b/config/routes.rb index ed26fd02dc6..b33b21830cb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -578,7 +578,7 @@ Discourse::Application.routes.draw do put "t/:topic_id/re-pin" => "topics#re_pin", constraints: {topic_id: /\d+/} put "t/:topic_id/mute" => "topics#mute", constraints: {topic_id: /\d+/} put "t/:topic_id/unmute" => "topics#unmute", constraints: {topic_id: /\d+/} - put "t/:topic_id/autoclose" => "topics#autoclose", constraints: {topic_id: /\d+/} + post "t/:topic_id/status_update" => "topics#status_update", constraints: {topic_id: /\d+/} put "t/:topic_id/make-banner" => "topics#make_banner", constraints: {topic_id: /\d+/} put "t/:topic_id/remove-banner" => "topics#remove_banner", constraints: {topic_id: /\d+/} put "t/:topic_id/remove-allowed-user" => "topics#remove_allowed_user", constraints: {topic_id: /\d+/} diff --git a/db/migrate/20170322065911_create_topic_status_updates.rb b/db/migrate/20170322065911_create_topic_status_updates.rb new file mode 100644 index 00000000000..36f81a6fa8e --- /dev/null +++ b/db/migrate/20170322065911_create_topic_status_updates.rb @@ -0,0 +1,16 @@ +class CreateTopicStatusUpdates < ActiveRecord::Migration + def change + create_table :topic_status_updates do |t| + t.datetime :execute_at, null: false + t.integer :status_type, null: false + t.integer :user_id, null: false + t.integer :topic_id, null: false + t.boolean :based_on_last_post, null: false, default: false + t.datetime :deleted_at + t.integer :deleted_by_id + t.timestamps + end + + add_index :topic_status_updates, :user_id + end +end diff --git a/db/migrate/20170324032913_move_auto_close_columns_to_topic_status_update.rb b/db/migrate/20170324032913_move_auto_close_columns_to_topic_status_update.rb new file mode 100644 index 00000000000..d476468be77 --- /dev/null +++ b/db/migrate/20170324032913_move_auto_close_columns_to_topic_status_update.rb @@ -0,0 +1,39 @@ +class MoveAutoCloseColumnsToTopicStatusUpdate < ActiveRecord::Migration + def up + execute <<~SQL + INSERT INTO topic_status_updates(topic_id, user_id, execute_at, status_type, based_on_last_post, created_at, updated_at) + SELECT + t.id, + t.auto_close_user_id, + t.auto_close_at, + #{TopicStatusUpdate.types[:close]}, + t.auto_close_based_on_last_post, + t.auto_close_started_at, + t.auto_close_started_at + FROM topics t + WHERE t.auto_close_at IS NOT NULL + AND t.auto_close_user_id IS NOT NULL + AND t.auto_close_started_at IS NOT NULL + AND t.deleted_at IS NULL + SQL + + execute <<~SQL + WITH selected AS ( + SELECT tsp.id + FROM topic_status_updates tsp + JOIN topics t + ON t.id = tsp.topic_id + WHERE tsp.execute_at < now() + OR (t.closed AND tsp.execute_at >= now()) + ) + + UPDATE topic_status_updates + SET deleted_at = now(), deleted_by_id = #{Discourse::SYSTEM_USER_ID} + WHERE id in (SELECT * FROM selected) + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20170330041605_add_index_to_topic_status_updates.rb b/db/migrate/20170330041605_add_index_to_topic_status_updates.rb new file mode 100644 index 00000000000..12a981b5fe3 --- /dev/null +++ b/db/migrate/20170330041605_add_index_to_topic_status_updates.rb @@ -0,0 +1,13 @@ +class AddIndexToTopicStatusUpdates < ActiveRecord::Migration + def up + execute <<~SQL + CREATE UNIQUE INDEX idx_topic_id_status_type_deleted_at + ON topic_status_updates(topic_id, status_type) + WHERE deleted_at IS NULL + SQL + end + + def down + execute "DROP INDEX idx_topic_id_status_type_deleted_at" + end +end diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 928c8473623..06a0c651b55 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -390,8 +390,16 @@ class PostCreator end def update_topic_auto_close - if @topic.auto_close_based_on_last_post && @topic.auto_close_hours - @topic.set_auto_close(@topic.auto_close_hours).save + topic_status_update = @topic.topic_status_update + + if topic_status_update && + topic_status_update.based_on_last_post && + topic_status_update.duration > 0 + + @topic.set_or_create_status_update(TopicStatusUpdate.types[:close], + topic_status_update.duration, + based_on_last_post: topic_status_update.based_on_last_post + ) end end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 87569632382..65085906b61 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -262,23 +262,38 @@ describe PostCreator do describe "topic's auto close" do it "doesn't update topic's auto close when it's not based on last post" do - auto_close_time = 1.day.from_now - topic = Fabricate(:topic, auto_close_at: auto_close_time, auto_close_hours: 12) + Timecop.freeze do + topic = Fabricate(:topic).set_or_create_status_update(TopicStatusUpdate.types[:close], 12) - PostCreator.new(topic.user, topic_id: topic.id, raw: "this is a second post").create - topic.reload + PostCreator.new(topic.user, topic_id: topic.id, raw: "this is a second post").create + topic.reload - expect(topic.auto_close_at).to be_within(1.second).of(auto_close_time) + topic_status_update = TopicStatusUpdate.last + expect(topic_status_update.execute_at).to be_within(1.second).of(Time.zone.now + 12.hours) + expect(topic_status_update.created_at).to be_within(1.second).of(Time.zone.now) + end end it "updates topic's auto close date when it's based on last post" do - auto_close_time = 1.day.from_now - topic = Fabricate(:topic, auto_close_at: auto_close_time, auto_close_hours: 12, auto_close_based_on_last_post: true) + SiteSetting.queue_jobs = true - PostCreator.new(topic.user, topic_id: topic.id, raw: "this is a second post").create - topic.reload + Timecop.freeze do + topic = Fabricate(:topic, + topic_status_updates: [Fabricate(:topic_status_update, + based_on_last_post: true, + execute_at: Time.zone.now - 12.hours, + created_at: Time.zone.now - 24.hours + )] + ) - expect(topic.auto_close_at).not_to be_within(1.second).of(auto_close_time) + Fabricate(:post, topic: topic) + + PostCreator.new(topic.user, topic_id: topic.id, raw: "this is a second post").create + + topic_status_update = TopicStatusUpdate.last + expect(topic_status_update.execute_at).to be_within(1.second).of(Time.zone.now + 12.hours) + expect(topic_status_update.created_at).to be_within(1.second).of(Time.zone.now) + end end end @@ -341,8 +356,9 @@ describe PostCreator do context 'when auto-close param is given' do it 'ensures the user can auto-close the topic, but ignores auto-close param silently' do Guardian.any_instance.stubs(:can_moderate?).returns(false) - post = PostCreator.new(user, basic_topic_params.merge(auto_close_time: 2)).create - expect(post.topic.auto_close_at).to eq(nil) + expect { + PostCreator.new(user, basic_topic_params.merge(auto_close_time: 2)).create! + }.to_not change { TopicStatusUpdate.count } end end end diff --git a/spec/components/topic_creator_spec.rb b/spec/components/topic_creator_spec.rb index dcca1dd101a..d47cc8dc112 100644 --- a/spec/components/topic_creator_spec.rb +++ b/spec/components/topic_creator_spec.rb @@ -39,7 +39,7 @@ describe TopicCreator do it "ignores auto_close_time without raising an error" do topic = TopicCreator.create(user, Guardian.new(user), valid_attrs.merge(auto_close_time: '24')) expect(topic).to be_valid - expect(topic.auto_close_at).to eq(nil) + expect(topic.topic_status_update).to eq(nil) end it "category name is case insensitive" do diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 4a5885ff60d..6377403e32b 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -1112,82 +1112,6 @@ describe TopicsController do end - describe 'autoclose' do - - it 'needs you to be logged in' do - expect { - xhr :put, :autoclose, topic_id: 99, auto_close_time: '24', auto_close_based_on_last_post: false - }.to raise_error(Discourse::NotLoggedIn) - end - - it 'needs you to be an admin or mod' do - log_in - xhr :put, :autoclose, topic_id: 99, auto_close_time: '24', auto_close_based_on_last_post: false - expect(response).to be_forbidden - end - - describe 'when logged in' do - before do - @admin = log_in(:admin) - @topic = Fabricate(:topic, user: @admin) - end - - it "can set a topic's auto close time and 'based on last post' property" do - Topic.any_instance.expects(:set_auto_close).with("24", {by_user: @admin, timezone_offset: -240}) - xhr :put, :autoclose, topic_id: @topic.id, auto_close_time: '24', auto_close_based_on_last_post: true, timezone_offset: -240 - json = ::JSON.parse(response.body) - expect(json).to have_key('auto_close_at') - expect(json).to have_key('auto_close_hours') - end - - it "can remove a topic's auto close time" do - Topic.any_instance.expects(:set_auto_close).with(nil, anything) - xhr :put, :autoclose, topic_id: @topic.id, auto_close_time: nil, auto_close_based_on_last_post: false, timezone_offset: -240 - end - - it "will close a topic when the time expires" do - topic = Fabricate(:topic) - Timecop.freeze(20.hours.ago) do - create_post(topic: topic, raw: "This is the body of my cool post in the topic, but it's a bit old now") - end - topic.save - - Jobs.expects(:enqueue_at).at_least_once - xhr :put, :autoclose, topic_id: topic.id, auto_close_time: 24, auto_close_based_on_last_post: true - - topic.reload - expect(topic.closed).to eq(false) - expect(topic.posts.last.raw).to match(/cool post/) - - Timecop.freeze(5.hours.from_now) do - Jobs::CloseTopic.new.execute({topic_id: topic.id, user_id: @admin.id}) - end - - topic.reload - expect(topic.closed).to eq(true) - expect(topic.posts.last.raw).to match(/automatically closed/) - end - - it "will immediately close if the last post is old enough" do - topic = Fabricate(:topic) - Timecop.freeze(20.hours.ago) do - create_post(topic: topic) - end - topic.save - Topic.reset_highest(topic.id) - topic.reload - - xhr :put, :autoclose, topic_id: topic.id, auto_close_time: 10, auto_close_based_on_last_post: true - - topic.reload - expect(topic.closed).to eq(true) - expect(topic.posts.last.raw).to match(/after the last reply/) - expect(topic.posts.last.raw).to match(/10 hours/) - end - end - - end - describe 'make_banner' do it 'needs you to be a staff member' do diff --git a/spec/fabricators/topic_status_update_fabricator.rb b/spec/fabricators/topic_status_update_fabricator.rb new file mode 100644 index 00000000000..c07a92d7d0b --- /dev/null +++ b/spec/fabricators/topic_status_update_fabricator.rb @@ -0,0 +1,6 @@ +Fabricator(:topic_status_update) do + user + topic + execute_at Time.zone.now + 1.hour + status_type TopicStatusUpdate.types[:close] +end diff --git a/spec/integration/managing_topic_status_spec.rb b/spec/integration/managing_topic_status_spec.rb new file mode 100644 index 00000000000..4e59f9c3df7 --- /dev/null +++ b/spec/integration/managing_topic_status_spec.rb @@ -0,0 +1,93 @@ +require 'rails_helper' + +RSpec.describe "Managing a topic's status update", type: :request do + let(:topic) { Fabricate(:topic) } + let(:user) { Fabricate(:user) } + + context 'when a user is not logged in' do + it 'should return the right response' do + expect do + post "/t/#{topic.id}/status_update.json", + time: '24', + status_type: TopicStatusUpdate.types[1] + end.to raise_error(Discourse::NotLoggedIn) + end + end + + context 'when does not have permission' do + it 'should return the right response' do + sign_in(user) + + post "/t/#{topic.id}/status_update.json", + time: '24', + status_type: TopicStatusUpdate.types[1] + + expect(response.status).to eq(403) + expect(JSON.parse(response.body)["error_type"]).to eq('invalid_access') + end + end + + context 'when logged in as an admin' do + let(:admin) { Fabricate(:admin) } + + before do + sign_in(admin) + end + + it 'should be able to create a topic status update' do + time = 24 + + post "/t/#{topic.id}/status_update.json", + time: 24, + status_type: TopicStatusUpdate.types[1] + + expect(response).to be_success + + topic_status_update = TopicStatusUpdate.last + + expect(topic_status_update.topic).to eq(topic) + + expect(topic_status_update.execute_at) + .to be_within(1.second).of(24.hours.from_now) + + json = JSON.parse(response.body) + + expect(DateTime.parse(json['execute_at'])) + .to be_within(1.seconds).of(DateTime.parse(topic_status_update.execute_at.to_s)) + + expect(json['duration']).to eq(topic_status_update.duration) + end + + it 'should be able to delete a topic status update' do + topic.update!(topic_status_updates: [Fabricate(:topic_status_update)]) + + post "/t/#{topic.id}/status_update.json", + time: nil, + status_type: TopicStatusUpdate.types[1] + + expect(response).to be_success + expect(topic.reload.topic_status_update).to eq(nil) + + json = JSON.parse(response.body) + + expect(json['execute_at']).to eq(nil) + expect(json['duration']).to eq(nil) + end + + describe 'invalid status type' do + it 'should raise the right error' do + expect do + post "/t/#{topic.id}/status_update.json", + time: 10, + status_type: 'something' + end.to raise_error(Discourse::InvalidParameters) + end + end + + describe 'when the last post is old enough' do + it 'should close the topic immediately' do + + end + end + end +end diff --git a/spec/integration/topic_auto_close_spec.rb b/spec/integration/topic_auto_close_spec.rb index c57c505c658..3f302c4eeb9 100644 --- a/spec/integration/topic_auto_close_spec.rb +++ b/spec/integration/topic_auto_close_spec.rb @@ -3,29 +3,10 @@ require 'rails_helper' describe Topic do - - def scheduled_jobs_for(job_name, params={}) - "Jobs::#{job_name.to_s.camelcase}".constantize.jobs.select do |job| - job_args = job['args'][0] - matched = true - params.each do |key, value| - unless job_args[key.to_s] == value - matched = false - break - end - end - matched - end - end + let(:job_klass) { Jobs::ToggleTopicClosed } before do - @original_value = SiteSetting.queue_jobs - SiteSetting.queue_jobs = true - Jobs::CloseTopic.jobs.clear - end - - after do - SiteSetting.queue_jobs = @original_value + job_klass.jobs.clear end context 'creating a topic without auto-close' do @@ -35,8 +16,8 @@ describe Topic do let(:category) { nil } it 'should not schedule the topic to auto-close' do - expect(topic.auto_close_at).to eq(nil) - expect(scheduled_jobs_for(:close_topic)).to be_empty + expect(topic.topic_status_update).to eq(nil) + expect(job_klass.jobs).to eq([]) end end @@ -44,29 +25,36 @@ describe Topic do let(:category) { Fabricate(:category, auto_close_hours: nil) } it 'should not schedule the topic to auto-close' do - expect(topic.auto_close_at).to eq(nil) - expect(scheduled_jobs_for(:close_topic)).to be_empty + expect(topic.topic_status_update).to eq(nil) + expect(job_klass.jobs).to eq([]) end end context 'jobs may be queued' do before do + SiteSetting.queue_jobs = true Timecop.freeze(Time.zone.now) end after do Timecop.return - Sidekiq::Extensions::DelayedClass.jobs.clear end context 'category has a default auto-close' do let(:category) { Fabricate(:category, auto_close_hours: 2.0) } it 'should schedule the topic to auto-close' do - expect(topic.auto_close_at).to be_within_one_second_of(2.hours.from_now) - expect(topic.auto_close_started_at).to eq(Time.zone.now) - expect(scheduled_jobs_for(:close_topic, {topic_id: topic.id}).size).to eq(1) - expect(scheduled_jobs_for(:close_topic, {topic_id: category.topic.id})).to be_empty + topic + + topic_status_update = TopicStatusUpdate.last + + expect(topic_status_update.topic).to eq(topic) + expect(topic.topic_status_update.execute_at).to be_within_one_second_of(2.hours.from_now) + + args = job_klass.jobs.last['args'].first + + expect(args["topic_status_update_id"]).to eq(topic.topic_status_update.id) + expect(args["state"]).to eq(true) end context 'topic was created by staff user' do @@ -74,14 +62,28 @@ describe Topic do let(:staff_topic) { Fabricate(:topic, user: admin, category: category) } it 'should schedule the topic to auto-close' do - expect(scheduled_jobs_for(:close_topic, {topic_id: staff_topic.id, user_id: admin.id}).size).to eq(1) + staff_topic + + topic_status_update = TopicStatusUpdate.last + + expect(topic_status_update.topic).to eq(staff_topic) + expect(topic_status_update.execute_at).to be_within_one_second_of(2.hours.from_now) + expect(topic_status_update.user).to eq(admin) + + args = job_klass.jobs.last['args'].first + + expect(args["topic_status_update_id"]).to eq(topic_status_update.id) + expect(args["state"]).to eq(true) end context 'topic is closed manually' do it 'should remove the schedule to auto-close the topic' do - staff_topic.update_status('closed', true, admin) - expect(staff_topic.reload.auto_close_at).to eq(nil) - expect(staff_topic.auto_close_started_at).to eq(nil) + Timecop.freeze do + staff_topic.update_status('closed', true, admin) + + expect(staff_topic.topic_status_update.reload.deleted_at) + .to be_within(1.second).of(Time.zone.now) + end end end end @@ -91,38 +93,20 @@ describe Topic do let(:regular_user_topic) { Fabricate(:topic, user: regular_user, category: category) } it 'should schedule the topic to auto-close' do - expect(scheduled_jobs_for(:close_topic, {topic_id: regular_user_topic.id, user_id: Discourse.system_user.id}).size).to eq(1) + regular_user_topic + + topic_status_update = TopicStatusUpdate.last + + expect(topic_status_update.topic).to eq(regular_user_topic) + expect(topic_status_update.execute_at).to be_within_one_second_of(2.hours.from_now) + expect(topic_status_update.user).to eq(Discourse.system_user) + + args = job_klass.jobs.last['args'].first + + expect(args["topic_status_update_id"]).to eq(topic_status_update.id) + expect(args["state"]).to eq(true) end end - - context 'auto_close_hours of topic was set to 0' do - let(:dont_close_topic) { Fabricate(:topic, auto_close_hours: 0, category: category) } - - it 'should not schedule the topic to auto-close' do - expect(scheduled_jobs_for(:close_topic)).to be_empty - end - end - - context 'two topics in the category' do - let!(:other_topic) { Fabricate(:topic, category: category) } - - it 'should schedule the topic to auto-close' do - topic - - expect(scheduled_jobs_for(:close_topic).size).to eq(2) - end - end - end - - context 'a topic that has been auto-closed' do - let(:admin) { Fabricate(:admin) } - let!(:auto_closed_topic) { Fabricate(:topic, user: admin, closed: true, auto_close_at: 1.day.ago, auto_close_user_id: admin.id, auto_close_started_at: 6.days.ago) } - - it 'should set the right attributes' do - auto_closed_topic.update_status('closed', false, admin) - expect(auto_closed_topic.reload.auto_close_at).to eq(nil) - expect(auto_closed_topic.auto_close_started_at).to eq(nil) - end end end end diff --git a/spec/jobs/close_topic_spec.rb b/spec/jobs/close_topic_spec.rb deleted file mode 100644 index 9efb67ca48d..00000000000 --- a/spec/jobs/close_topic_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -require 'rails_helper' -require_dependency 'jobs/base' - -describe Jobs::CloseTopic do - - let(:admin) { Fabricate.build(:admin) } - - it 'closes a topic that is set to auto-close' do - topic = Fabricate.build(:topic, auto_close_at: Time.zone.now, user: admin) - topic.expects(:update_status).with('autoclosed', true, admin) - Topic.stubs(:find_by).returns(topic) - User.stubs(:find_by).returns(admin) - Jobs::CloseTopic.new.execute( topic_id: 123, user_id: 234 ) - end - - shared_examples_for "cases when CloseTopic does nothing" do - it 'does nothing to the topic' do - topic.expects(:update_status).never - Topic.stubs(:find_by).returns(topic) - User.stubs(:find_by).returns(admin) - Jobs::CloseTopic.new.execute( topic_id: 123, user_id: 234 ) - end - end - - context 'when topic is not set to auto-close' do - subject(:topic) { Fabricate.build(:topic, auto_close_at: nil, user: admin) } - it_behaves_like 'cases when CloseTopic does nothing' - end - - context 'when user is not authorized to close topics' do - subject(:topic) { Fabricate.build(:topic, auto_close_at: 2.days.from_now, user: admin) } - before { Guardian.any_instance.stubs(:can_moderate?).returns(false) } - it_behaves_like 'cases when CloseTopic does nothing' - end - - context 'the topic is already closed' do - subject(:topic) { Fabricate.build(:topic, auto_close_at: 2.days.from_now, user: admin, closed: true) } - it_behaves_like 'cases when CloseTopic does nothing' - end - - context 'the topic has been deleted' do - subject(:topic) { Fabricate.build(:deleted_topic, auto_close_at: 2.days.from_now, user: admin) } - it_behaves_like 'cases when CloseTopic does nothing' - end - -end diff --git a/spec/jobs/toggle_topic_closed_spec.rb b/spec/jobs/toggle_topic_closed_spec.rb new file mode 100644 index 00000000000..c98f70d562c --- /dev/null +++ b/spec/jobs/toggle_topic_closed_spec.rb @@ -0,0 +1,79 @@ +require 'rails_helper' + +describe Jobs::ToggleTopicClosed do + let(:admin) { Fabricate(:admin) } + + let(:topic) do + Fabricate(:topic, + topic_status_updates: [Fabricate(:topic_status_update, user: admin)] + ) + end + + before do + SiteSetting.queue_jobs = true + end + + it 'should be able to close a topic' do + topic + + Timecop.travel(1.hour.from_now) do + described_class.new.execute( + topic_status_update_id: topic.topic_status_update.id, + state: true + ) + + expect(topic.reload.closed).to eq(true) + + expect(Post.last.raw).to eq(I18n.t( + 'topic_statuses.autoclosed_enabled_minutes', count: 60 + )) + end + end + + it 'should be able to open a topic' do + topic.update!(closed: true) + + Timecop.travel(1.hour.from_now) do + described_class.new.execute( + topic_status_update_id: topic.topic_status_update.id, + state: false + ) + + expect(topic.reload.closed).to eq(false) + + expect(Post.last.raw).to eq(I18n.t( + 'topic_statuses.autoclosed_disabled_minutes', count: 60 + )) + end + end + + describe 'when trying to close a topic that has been deleted' do + it 'should not do anything' do + topic.trash! + + Topic.any_instance.expects(:update_status).never + + described_class.new.execute( + topic_status_update_id: topic.topic_status_update.id, + state: true + ) + end + end + + describe 'when user is not authorized to close topics' do + let(:topic) do + Fabricate(:topic, + topic_status_updates: [Fabricate(:topic_status_update, execute_at: 2.hours.from_now)] + ) + end + + it 'should not do anything' do + described_class.new.execute( + topic_status_update_id: topic.topic_status_update.id, + state: false + ) + + expect(topic.reload.closed).to eq(false) + end + end +end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index f8c547a2682..d18d30a63e1 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -343,7 +343,7 @@ describe Category do it "should not set its description topic to auto-close" do category = Fabricate(:category, name: 'Closing Topics', auto_close_hours: 1) - expect(category.topic.auto_close_at).to be_nil + expect(category.topic.topic_status_update).to eq(nil) end describe "creating a new category with the same slug" do diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 6a85a4c50ae..41916d78a83 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -742,6 +742,7 @@ describe Topic do expect(@topic).to be_closed expect(@topic.bumped_at.to_f).to eq(@original_bumped_at) expect(@topic.moderator_posts_count).to eq(1) + expect(@topic.topic_status_updates.first).to eq(nil) end end end @@ -766,20 +767,19 @@ describe Topic do context 'topic was set to close after it was created' do it 'puts the autoclose duration in the moderator post' do - freeze_time(Time.new(2000,1,1)) @topic.created_at = 7.days.ago freeze_time(2.days.ago) - @topic.set_auto_close(48) + @topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 48) + @topic.save! freeze_time(2.days.from_now) @topic.update_status(status, true, @user) expect(@topic.posts.last.raw).to include "closed after 2 days" - end end end @@ -1096,297 +1096,173 @@ describe Topic do end end - describe 'auto-close' do - context 'a new topic' do - context 'auto_close_at is set' do - it 'queues a job to close the topic' do - Timecop.freeze(now) do - Jobs.expects(:enqueue_at).with(7.hours.from_now, :close_topic, all_of( has_key(:topic_id), has_key(:user_id) )) - topic = Fabricate(:topic, user: Fabricate(:admin)) - topic.set_auto_close(7).save - end - end + describe '#set_or_create_status_update' do + let(:topic) { Fabricate.build(:topic) } - it 'when auto_close_user_id is nil, it will use the topic creator as the topic closer' do - topic_creator = Fabricate(:admin) - Jobs.expects(:enqueue_at).with do |datetime, job_name, job_args| - job_args[:user_id] == topic_creator.id - end - topic = Fabricate(:topic, user: topic_creator) - topic.set_auto_close(7).save - end - - it 'when auto_close_user_id is set, it will use it as the topic closer' do - topic_creator = Fabricate(:admin) - topic_closer = Fabricate(:user, admin: true) - Jobs.expects(:enqueue_at).with do |datetime, job_name, job_args| - job_args[:user_id] == topic_closer.id - end - topic = Fabricate(:topic, user: topic_creator) - topic.set_auto_close(7, {by_user: topic_closer}).save - end - - it "ignores the category's default auto-close" do - Timecop.freeze(now) do - Jobs.expects(:enqueue_at).with(7.hours.from_now, :close_topic, all_of( has_key(:topic_id), has_key(:user_id) )) - topic = Fabricate(:topic, user: Fabricate(:admin), ignore_category_auto_close: true, category_id: Fabricate(:category, auto_close_hours: 2).id) - topic.set_auto_close(7).save - end - end - - it 'sets the time when auto_close timer starts' do - Timecop.freeze(now) do - topic = Fabricate(:topic, user: Fabricate(:admin)) - topic.set_auto_close(7).save - expect(topic.auto_close_started_at).to eq(now) - end - end - end + let(:closing_topic) do + Fabricate(:topic, + topic_status_updates: [Fabricate(:topic_status_update, execute_at: 5.hours.from_now)] + ) end - context 'an existing topic' do - it 'when auto_close_at is set, it queues a job to close the topic' do - Timecop.freeze(now) do - topic = Fabricate(:topic) - Jobs.expects(:enqueue_at).with(12.hours.from_now, :close_topic, has_entries(topic_id: topic.id, user_id: topic.user_id)) - topic.auto_close_at = 12.hours.from_now - expect(topic.save).to eq(true) - end - end - - it 'when auto_close_at and auto_closer_user_id are set, it queues a job to close the topic' do - Timecop.freeze(now) do - topic = Fabricate(:topic) - closer = Fabricate(:admin) - Jobs.expects(:enqueue_at).with(12.hours.from_now, :close_topic, has_entries(topic_id: topic.id, user_id: closer.id)) - topic.auto_close_at = 12.hours.from_now - topic.auto_close_user = closer - expect(topic.save).to eq(true) - end - end - - it 'when auto_close_at is removed, it cancels the job to close the topic' do - Jobs.stubs(:enqueue_at).returns(true) - topic = Fabricate(:topic, auto_close_at: 1.day.from_now) - Jobs.expects(:cancel_scheduled_job).with(:close_topic, {topic_id: topic.id}) - topic.auto_close_at = nil - expect(topic.save).to eq(true) - expect(topic.auto_close_user).to eq(nil) - end - - it 'when auto_close_user is removed, it updates the job' do - Timecop.freeze(now) do - Jobs.stubs(:enqueue_at).with(1.day.from_now, :close_topic, anything).returns(true) - topic = Fabricate(:topic, auto_close_at: 1.day.from_now, auto_close_user: Fabricate(:admin)) - Jobs.expects(:cancel_scheduled_job).with(:close_topic, {topic_id: topic.id}) - Jobs.expects(:enqueue_at).with(1.day.from_now, :close_topic, has_entries(topic_id: topic.id, user_id: topic.user_id)) - topic.auto_close_user = nil - expect(topic.save).to eq(true) - end - end - - it 'when auto_close_at value is changed, it reschedules the job' do - Timecop.freeze(now) do - Jobs.stubs(:enqueue_at).returns(true) - topic = Fabricate(:topic, auto_close_at: 1.day.from_now) - Jobs.expects(:cancel_scheduled_job).with(:close_topic, {topic_id: topic.id}) - Jobs.expects(:enqueue_at).with(3.days.from_now, :close_topic, has_entry(topic_id: topic.id)) - topic.auto_close_at = 3.days.from_now - expect(topic.save).to eq(true) - end - end - - it 'when auto_close_user_id is changed, it updates the job' do - Timecop.freeze(now) do - admin = Fabricate(:admin) - Jobs.stubs(:enqueue_at).returns(true) - topic = Fabricate(:topic, auto_close_at: 1.day.from_now) - Jobs.expects(:cancel_scheduled_job).with(:close_topic, {topic_id: topic.id}) - Jobs.expects(:enqueue_at).with(1.day.from_now, :close_topic, has_entries(topic_id: topic.id, user_id: admin.id)) - topic.auto_close_user = admin - expect(topic.save).to eq(true) - end - end - - it 'when auto_close_at and auto_close_user_id are not changed, it should not schedule another CloseTopic job' do - Timecop.freeze(now) do - Jobs.expects(:enqueue_at).with(1.day.from_now, :close_topic, has_key(:topic_id)).once.returns(true) - Jobs.expects(:cancel_scheduled_job).never - topic = Fabricate(:topic, auto_close_at: 1.day.from_now) - topic.title = 'A new title that is long enough' - expect(topic.save).to eq(true) - end - end - - it "ignores the category's default auto-close" do - Timecop.freeze(now) do - mod = Fabricate(:moderator) - # NOTE, only moderators can auto-close, if missing system user is used - topic = Fabricate(:topic, category: Fabricate(:category, auto_close_hours: 14), user: mod) - Jobs.expects(:enqueue_at).with(12.hours.from_now, :close_topic, has_entries(topic_id: topic.id, user_id: topic.user_id)) - topic.auto_close_at = 12.hours.from_now - topic.save - - topic.reload - expect(topic.closed).to eq(false) - - Timecop.freeze(24.hours.from_now) do - Topic.auto_close - topic.reload - expect(topic.closed).to eq(true) - end - - end - end - end - end - - describe 'set_auto_close' do - let(:topic) { Fabricate.build(:topic) } - let(:closing_topic) { Fabricate.build(:topic, auto_close_hours: 5, auto_close_at: 5.hours.from_now, auto_close_started_at: 5.hours.from_now) } - let(:admin) { Fabricate.build(:user, id: 123) } - let(:trust_level_4) { Fabricate.build(:trust_level_4) } + let(:admin) { Fabricate(:admin) } + let(:trust_level_4) { Fabricate(:trust_level_4) } before { Discourse.stubs(:system_user).returns(admin) } it 'can take a number of hours as an integer' do Timecop.freeze(now) do - topic.set_auto_close(72, {by_user: admin}) - expect(topic.auto_close_at).to eq(3.days.from_now) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 72, by_user: admin) + expect(topic.topic_status_updates.first.execute_at).to eq(3.days.from_now) end end it 'can take a number of hours as an integer, with timezone offset' do Timecop.freeze(now) do - topic.set_auto_close(72, {by_user: admin, timezone_offset: 240}) - expect(topic.auto_close_at).to eq(3.days.from_now) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 72, {by_user: admin, timezone_offset: 240}) + expect(topic.topic_status_updates.first.execute_at).to eq(3.days.from_now) end end it 'can take a number of hours as a string' do Timecop.freeze(now) do - topic.set_auto_close('18', {by_user: admin}) - expect(topic.auto_close_at).to eq(18.hours.from_now) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '18', by_user: admin) + expect(topic.topic_status_updates.first.execute_at).to eq(18.hours.from_now) end end it 'can take a number of hours as a string, with timezone offset' do Timecop.freeze(now) do - topic.set_auto_close('18', {by_user: admin, timezone_offset: 240}) - expect(topic.auto_close_at).to eq(18.hours.from_now) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '18', {by_user: admin, timezone_offset: 240}) + expect(topic.topic_status_updates.first.execute_at).to eq(18.hours.from_now) end end it "can take a time later in the day" do Timecop.freeze(now) do - topic.set_auto_close('13:00', {by_user: admin}) - expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,20,13,0)) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '13:00', {by_user: admin}) + expect(topic.topic_status_updates.first.execute_at).to eq(Time.zone.local(2013,11,20,13,0)) end end it "can take a time later in the day, with timezone offset" do Timecop.freeze(now) do - topic.set_auto_close('13:00', {by_user: admin, timezone_offset: 240}) - expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,20,17,0)) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '13:00', {by_user: admin, timezone_offset: 240}) + expect(topic.topic_status_updates.first.execute_at).to eq(Time.zone.local(2013,11,20,17,0)) end end it "can take a time for the next day" do Timecop.freeze(now) do - topic.set_auto_close('5:00', {by_user: admin}) - expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,21,5,0)) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '5:00', {by_user: admin}) + expect(topic.topic_status_updates.first.execute_at).to eq(Time.zone.local(2013,11,21,5,0)) end end it "can take a time for the next day, with timezone offset" do Timecop.freeze(now) do - topic.set_auto_close('1:00', {by_user: admin, timezone_offset: 240}) - expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,21,5,0)) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '1:00', {by_user: admin, timezone_offset: 240}) + expect(topic.topic_status_updates.first.execute_at).to eq(Time.zone.local(2013,11,21,5,0)) end end it "can take a timestamp for a future time" do Timecop.freeze(now) do - topic.set_auto_close('2013-11-22 5:00', {by_user: admin}) - expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,22,5,0)) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '2013-11-22 5:00', {by_user: admin}) + expect(topic.topic_status_updates.first.execute_at).to eq(Time.zone.local(2013,11,22,5,0)) end end it "can take a timestamp for a future time, with timezone offset" do Timecop.freeze(now) do - topic.set_auto_close('2013-11-22 5:00', {by_user: admin, timezone_offset: 240}) - expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,22,9,0)) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '2013-11-22 5:00', {by_user: admin, timezone_offset: 240}) + expect(topic.topic_status_updates.first.execute_at).to eq(Time.zone.local(2013,11,22,9,0)) end end it "sets a validation error when given a timestamp in the past" do Timecop.freeze(now) do - topic.set_auto_close('2013-11-19 5:00', {by_user: admin}) - expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,19,5,0)) - expect(topic.errors[:auto_close_at]).to be_present + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '2013-11-19 5:00', {by_user: admin}) + + expect(topic.topic_status_updates.first.execute_at).to eq(Time.zone.local(2013,11,19,5,0)) + expect(topic.topic_status_updates.first.errors[:execute_at]).to be_present end end it "can take a timestamp with timezone" do Timecop.freeze(now) do - topic.set_auto_close('2013-11-25T01:35:00-08:00', {by_user: admin}) - expect(topic.auto_close_at).to eq(Time.utc(2013,11,25,9,35)) + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '2013-11-25T01:35:00-08:00', {by_user: admin}) + expect(topic.topic_status_updates.first.execute_at).to eq(Time.utc(2013,11,25,9,35)) end end - it 'sets auto_close_user to given user if it is a staff or TL4 user' do - topic.set_auto_close(3, {by_user: admin}) - expect(topic.auto_close_user_id).to eq(admin.id) + it 'sets topic status update user to given user if it is a staff or TL4 user' do + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 3, {by_user: admin}) + expect(topic.topic_status_updates.first.user).to eq(admin) end - it 'sets auto_close_user to given user if it is a TL4 user' do - topic.set_auto_close(3, {by_user: trust_level_4}) - expect(topic.auto_close_user_id).to eq(trust_level_4.id) + it 'sets topic status update user to given user if it is a TL4 user' do + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 3, {by_user: trust_level_4}) + expect(topic.topic_status_updates.first.user).to eq(trust_level_4) end - it 'sets auto_close_user to system user if given user is not staff or a TL4 user' do - topic.set_auto_close(3, {by_user: Fabricate.build(:user, id: 444)}) - expect(topic.auto_close_user_id).to eq(admin.id) + it 'sets topic status update user to system user if given user is not staff or a TL4 user' do + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 3, {by_user: Fabricate.build(:user, id: 444)}) + expect(topic.topic_status_updates.first.user).to eq(admin) end - it 'sets auto_close_user to system user if user is not given and topic creator is not staff nor TL4 user' do - topic.set_auto_close(3) - expect(topic.auto_close_user_id).to eq(admin.id) + it 'sets topic status update user to system user if user is not given and topic creator is not staff nor TL4 user' do + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 3) + expect(topic.topic_status_updates.first.user).to eq(admin) end - it 'sets auto_close_user to topic creator if it is a staff user' do + it 'sets topic status update user to topic creator if it is a staff user' do staff_topic = Fabricate.build(:topic, user: Fabricate.build(:admin, id: 999)) - staff_topic.set_auto_close(3) - expect(staff_topic.auto_close_user_id).to eq(999) + staff_topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 3) + expect(staff_topic.topic_status_updates.first.user_id).to eq(999) end - it 'sets auto_close_user to topic creator if it is a TL4 user' do + it 'sets topic status update user to topic creator if it is a TL4 user' do tl4_topic = Fabricate.build(:topic, user: Fabricate.build(:trust_level_4, id: 998)) - tl4_topic.set_auto_close(3) - expect(tl4_topic.auto_close_user_id).to eq(998) + tl4_topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 3) + expect(tl4_topic.topic_status_updates.first.user_id).to eq(998) end - it 'clears auto_close_at if arg is nil' do - closing_topic.set_auto_close(nil) - expect(closing_topic.auto_close_at).to be_nil + it 'removes close topic status update if arg is nil' do + closing_topic.set_or_create_status_update(TopicStatusUpdate.types[:close], nil) + closing_topic.reload + expect(closing_topic.topic_status_updates.first).to be_nil end - it 'clears auto_close_started_at if arg is nil' do - closing_topic.set_auto_close(nil) - expect(closing_topic.auto_close_started_at).to be_nil - end - - it 'updates auto_close_at if it was already set to close' do + it 'updates topic status update execute_at if it was already set to close' do Timecop.freeze(now) do - closing_topic.set_auto_close(48) - expect(closing_topic.auto_close_at).to eq(2.days.from_now) + closing_topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 48) + expect(closing_topic.reload.topic_status_update.execute_at).to eq(2.days.from_now) end end - it 'does not update auto_close_started_at if it was already set to close' do + it "does not update topic's topic status created_at it was already set to close" do expect{ - closing_topic.set_auto_close(14) - }.to_not change(closing_topic, :auto_close_started_at) + closing_topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 14) + }.to_not change { closing_topic.topic_status_updates.first.created_at } + end + + describe "when category's default auto close is set" do + let(:category) { Fabricate(:category, auto_close_hours: 4) } + let(:topic) { Fabricate(:topic, category: category) } + + it "should be able to override category's default auto close" do + expect(topic.topic_status_updates.first.duration).to eq(4) + + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], 2, by_user: admin) + + expect(topic.reload.closed).to eq(false) + + Timecop.freeze(3.hours.from_now) do + TopicStatusUpdate.ensure_consistency! + expect(topic.reload.closed).to eq(true) + end + end end end diff --git a/spec/models/topic_status_update_spec.rb b/spec/models/topic_status_update_spec.rb index 549adb9e872..f9b1c9a895d 100644 --- a/spec/models/topic_status_update_spec.rb +++ b/spec/models/topic_status_update_spec.rb @@ -1,56 +1,187 @@ -# encoding: UTF-8 - require 'rails_helper' -require_dependency 'post_destroyer' -# TODO - test pinning, create_moderator_post +RSpec.describe TopicStatusUpdate, type: :model do + let(:topic_status_update) { Fabricate(:topic_status_update) } + let(:topic) { Fabricate(:topic) } -describe TopicStatusUpdate do + context "validations" do + describe '#status_type' do + it 'should ensure that only one active topic status update exists' do + topic_status_update.update!(topic: topic) + Fabricate(:topic_status_update, deleted_at: Time.zone.now, topic: topic) - let(:user) { Fabricate(:user) } - let(:admin) { Fabricate(:admin) } + expect { Fabricate(:topic_status_update, topic: topic) } + .to raise_error(ActiveRecord::RecordInvalid) + end + end - it "avoids notifying on automatically closed topics" do - # TODO: TopicStatusUpdate should suppress message bus updates from the users it "pretends to read" - post = PostCreator.create(user, - raw: "this is a test post 123 this is a test post", - title: "hello world title", - ) - # TODO needed so counts sync up, PostCreator really should not give back out-of-date Topic - post.topic.set_auto_close('10') - post.topic.reload + describe '#execute_at' do + describe 'when #execute_at is greater than #created_at' do + it 'should be valid' do + topic_status_update = Fabricate.build(:topic_status_update, + execute_at: Time.zone.now + 1.hour, + user: Fabricate(:user), + topic: Fabricate(:topic) + ) - TopicStatusUpdate.new(post.topic, admin).update!("autoclosed", true) + expect(topic_status_update).to be_valid + end + end - expect(post.topic.posts.count).to eq(2) + describe 'when #execute_at is smaller than #created_at' do + it 'should not be valid' do + topic_status_update = Fabricate.build(:topic_status_update, + execute_at: Time.zone.now - 1.hour, + created_at: Time.zone.now, + user: Fabricate(:user), + topic: Fabricate(:topic) + ) - tu = TopicUser.find_by(user_id: user.id) - expect(tu.last_read_post_number).to eq(2) + expect(topic_status_update).to_not be_valid + end + end + end end - it "adds an autoclosed message" do - topic = create_topic - topic.set_auto_close('10') + context 'callbacks' do + describe 'when #execute_at and #user_id are not changed' do + it 'should not schedule another to update topic' do + Jobs.expects(:enqueue_at).with( + topic_status_update.execute_at, + :toggle_topic_closed, + topic_status_update_id: topic_status_update.id, + state: true + ).once - TopicStatusUpdate.new(topic, admin).update!("autoclosed", true) + topic_status_update - last_post = topic.posts.last - expect(last_post.post_type).to eq(Post.types[:small_action]) - expect(last_post.action_code).to eq('autoclosed.enabled') - expect(last_post.raw).to eq(I18n.t("topic_statuses.autoclosed_enabled_minutes", count: 0)) + Jobs.expects(:cancel_scheduled_job).never + + topic_status_update.update!(topic: Fabricate(:topic)) + end + end + + describe 'when #execute_at value is changed' do + it 'reschedules the job' do + Timecop.freeze do + topic_status_update + + Jobs.expects(:cancel_scheduled_job).with( + :toggle_topic_closed, topic_status_update_id: topic_status_update.id + ) + + Jobs.expects(:enqueue_at).with( + 3.days.from_now, :toggle_topic_closed, + topic_status_update_id: topic_status_update.id, + state: true + ) + + topic_status_update.update!(execute_at: 3.days.from_now, created_at: Time.zone.now) + end + end + + describe 'when execute_at is smaller than the current time' do + it 'should enqueue the job immediately' do + Timecop.freeze do + topic_status_update + + Jobs.expects(:enqueue_at).with( + Time.zone.now, :toggle_topic_closed, + topic_status_update_id: topic_status_update.id, + state: true + ) + + topic_status_update.update!( + execute_at: Time.zone.now - 1.hour, + created_at: Time.zone.now - 2.hour + ) + end + end + end + end + + describe 'when user is changed' do + it 'should update the job' do + Timecop.freeze do + topic_status_update + + Jobs.expects(:cancel_scheduled_job).with( + :toggle_topic_closed, topic_status_update_id: topic_status_update.id + ) + + admin = Fabricate(:admin) + + Jobs.expects(:enqueue_at).with( + topic_status_update.execute_at, + :toggle_topic_closed, + topic_status_update_id: topic_status_update.id, + state: true + ) + + topic_status_update.update!(user: admin) + end + end + end end - it "adds an autoclosed message based on last post" do - topic = create_topic - topic.auto_close_based_on_last_post = true - topic.set_auto_close('10') + describe '.ensure_consistency!' do + before do + SiteSetting.queue_jobs = true + Jobs::ToggleTopicClosed.jobs.clear + end - TopicStatusUpdate.new(topic, admin).update!("autoclosed", true) + it 'should enqueue jobs that have been missed' do + close_topic_status_update = Fabricate(:topic_status_update, + execute_at: Time.zone.now - 1.hour, + created_at: Time.zone.now - 2.hour + ) - last_post = topic.posts.last - expect(last_post.post_type).to eq(Post.types[:small_action]) - expect(last_post.action_code).to eq('autoclosed.enabled') - expect(last_post.raw).to eq(I18n.t("topic_statuses.autoclosed_enabled_lastpost_hours", count: 10)) + open_topic_status_update = Fabricate(:topic_status_update, + status_type: described_class.types[:open], + execute_at: Time.zone.now - 1.hour, + created_at: Time.zone.now - 2.hour + ) + + Fabricate(:topic_status_update) + + expect { described_class.ensure_consistency! } + .to change { Jobs::ToggleTopicClosed.jobs.count }.by(2) + + job_args = Jobs::ToggleTopicClosed.jobs.first["args"].first + + expect(job_args["topic_status_update_id"]).to eq(close_topic_status_update.id) + expect(job_args["state"]).to eq(true) + + job_args = Jobs::ToggleTopicClosed.jobs.last["args"].first + + expect(job_args["topic_status_update_id"]).to eq(open_topic_status_update.id) + expect(job_args["state"]).to eq(false) + end end + describe 'when a open topic status update is created for an open topic' do + it 'should close the topic' do + topic = Fabricate(:topic, closed: false) + + Fabricate(:topic_status_update, + status_type: described_class.types[:open], + topic: topic + ) + + expect(topic.reload.closed).to eq(true) + end + end + + describe 'when a close topic status update is created for a closed topic' do + it 'should open the topic' do + topic = Fabricate(:topic, closed: true) + + Fabricate(:topic_status_update, + status_type: described_class.types[:close], + topic: topic + ) + + expect(topic.reload.closed).to eq(false) + end + end end diff --git a/spec/services/topic_status_updater_spec.rb b/spec/services/topic_status_updater_spec.rb new file mode 100644 index 00000000000..3609082ec64 --- /dev/null +++ b/spec/services/topic_status_updater_spec.rb @@ -0,0 +1,59 @@ +# encoding: UTF-8 + +require 'rails_helper' +require_dependency 'post_destroyer' + +# TODO - test pinning, create_moderator_post + +describe TopicStatusUpdater do + + let(:user) { Fabricate(:user) } + let(:admin) { Fabricate(:admin) } + + it "avoids notifying on automatically closed topics" do + # TODO: TopicStatusUpdater should suppress message bus updates from the users it "pretends to read" + post = PostCreator.create(user, + raw: "this is a test post 123 this is a test post", + title: "hello world title", + ) + # TODO needed so counts sync up, PostCreator really should not give back out-of-date Topic + post.topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '10') + post.topic.reload + + TopicStatusUpdater.new(post.topic, admin).update!("autoclosed", true) + + expect(post.topic.posts.count).to eq(2) + + tu = TopicUser.find_by(user_id: user.id) + expect(tu.last_read_post_number).to eq(2) + end + + it "adds an autoclosed message" do + topic = create_topic + topic.set_or_create_status_update(TopicStatusUpdate.types[:close], '10') + + TopicStatusUpdater.new(topic, admin).update!("autoclosed", true) + + last_post = topic.posts.last + expect(last_post.post_type).to eq(Post.types[:small_action]) + expect(last_post.action_code).to eq('autoclosed.enabled') + expect(last_post.raw).to eq(I18n.t("topic_statuses.autoclosed_enabled_minutes", count: 0)) + end + + it "adds an autoclosed message based on last post" do + topic = create_topic + Fabricate(:post, topic: topic) + + topic.set_or_create_status_update( + TopicStatusUpdate.types[:close], '10', based_on_last_post: true + ) + + TopicStatusUpdater.new(topic, admin).update!("autoclosed", true) + + last_post = topic.posts.last + expect(last_post.post_type).to eq(Post.types[:small_action]) + expect(last_post.action_code).to eq('autoclosed.enabled') + expect(last_post.raw).to eq(I18n.t("topic_statuses.autoclosed_enabled_lastpost_hours", count: 10)) + end + +end