Add tabs to category create/edit modal. Categories can have a default auto-close setting that applies to all new topics created in the category. Add rspec-given and write some integration tests. Tests for topic auto-close with category default

This commit is contained in:
Neil Lalonde 2013-05-15 15:19:41 -04:00
parent 737e16f7e0
commit f3282e33a3
22 changed files with 298 additions and 98 deletions

View file

@ -108,6 +108,7 @@ group :test, :development do
gem 'simplecov', require: false gem 'simplecov', require: false
gem 'terminal-notifier-guard', require: false gem 'terminal-notifier-guard', require: false
gem 'timecop' gem 'timecop'
gem 'rspec-given'
end end
group :development do group :development do

View file

@ -371,6 +371,9 @@ GEM
rspec-core (2.13.1) rspec-core (2.13.1)
rspec-expectations (2.13.0) rspec-expectations (2.13.0)
diff-lcs (>= 1.1.3, < 2.0) diff-lcs (>= 1.1.3, < 2.0)
rspec-given (2.4.1)
rspec (>= 2.11)
sorcerer (>= 0.3.7)
rspec-mocks (2.13.1) rspec-mocks (2.13.1)
rspec-rails (2.13.0) rspec-rails (2.13.0)
actionpack (>= 3.0) actionpack (>= 3.0)
@ -416,6 +419,7 @@ GEM
temple (~> 0.6.3) temple (~> 0.6.3)
tilt (~> 1.3.3) tilt (~> 1.3.3)
slop (3.4.4) slop (3.4.4)
sorcerer (0.3.10)
spork (0.9.2) spork (0.9.2)
temple (0.6.4) temple (0.6.4)
terminal-notifier-guard (1.5.3) terminal-notifier-guard (1.5.3)
@ -511,6 +515,7 @@ DEPENDENCIES
redis-rails redis-rails
rest-client rest-client
rinku rinku
rspec-given
rspec-rails rspec-rails
sanitize sanitize
sass sass

View file

@ -45,7 +45,8 @@ Discourse.Category = Discourse.Model.extend({
text_color: this.get('text_color'), text_color: this.get('text_color'),
hotness: this.get('hotness'), hotness: this.get('hotness'),
secure: this.get('secure'), secure: this.get('secure'),
group_names: this.get('groups').join(",") group_names: this.get('groups').join(","),
auto_close_days: this.get('auto_close_days')
}, },
type: this.get('id') ? 'PUT' : 'POST' type: this.get('id') ? 'PUT' : 'POST'
}); });

View file

@ -538,13 +538,7 @@ Discourse.Composer = Discourse.Model.extend({
var reply = this.get('reply') || ""; var reply = this.get('reply') || "";
while (Discourse.BBCode.QUOTE_REGEXP.test(reply)) { reply = reply.replace(Discourse.BBCode.QUOTE_REGEXP, ""); } while (Discourse.BBCode.QUOTE_REGEXP.test(reply)) { reply = reply.replace(Discourse.BBCode.QUOTE_REGEXP, ""); }
return reply.replace(/\s+/img, " ").trim().length; return reply.replace(/\s+/img, " ").trim().length;
}.property('reply'), }.property('reply')
autoCloseChanged: function() {
if( this.get('auto_close_days') && this.get('auto_close_days').length > 0 ) {
this.set('auto_close_days', this.get('auto_close_days').replace(/[^\d]/g, '') )
}
}.observes('auto_close_days')
}); });

View file

@ -0,0 +1,6 @@
<div class="auto-close-fields">
<i class="icon icon-time"></i>
{{view.label}}
{{view Discourse.TextField valueBinding="view.autoCloseDays" maxlength="3"}}
{{i18n composer.auto_close_units}}
</div>

View file

@ -45,12 +45,7 @@
</div> </div>
<div class="admin-options-form"> <div class="admin-options-form">
<div class="auto-close-fields"> {{view Discourse.AutoCloseFormView autoCloseDaysBinding="content.auto_close_days"}}
<i class="icon icon-time"></i>
{{i18n composer.auto_close_label}}
{{view Discourse.TextField valueBinding="content.auto_close_days" maxlength="5"}}
{{i18n composer.auto_close_units}}
</div>
</div> </div>
{{/if}} {{/if}}

View file

@ -1,11 +1,6 @@
<div class="modal-body"> <div class="modal-body">
<form> <form>
<div class="auto-close-fields"> {{view Discourse.AutoCloseFormView autoCloseDaysBinding="view.auto_close_days"}}
<i class="icon icon-time"></i>
{{i18n composer.auto_close_label}}
{{view Discourse.TextField valueBinding="view.auto_close_days" maxlength="5"}}
{{i18n composer.auto_close_units}}
</div>
</form> </form>
</div> </div>
<div class="modal-footer"> <div class="modal-footer">

View file

@ -1,7 +1,22 @@
{{#with view.category}} {{#with view.category}}
<div {{bindAttr class="view.loading:invisible"}}> <div {{bindAttr class="view.loading:invisible"}}>
<ul class="nav nav-pills">
<li {{bindAttr class="view.generalSelected:active"}}>
<a href="#" {{action selectGeneral target="view"}}>{{i18n category.general}}</a>
</li>
{{#unless is_uncategorized}}
<li {{bindAttr class="view.securitySelected:active"}}>
<a href="#" {{action selectSecurity target="view"}}>{{i18n category.security}}</a>
</li>
<li {{bindAttr class="view.settingsSelected:active"}}>
<a href="#" {{action selectSettings target="view"}}>{{i18n category.settings}}</a>
</li>
{{/unless}}
</ul>
<div class="modal-body"> <div class="modal-body">
<div {{bindAttr class=":modal-tab :general-tab view.generalSelected::invisible"}}>
<form> <form>
<section class='field'> <section class='field'>
<label>{{i18n category.name}}</label> <label>{{i18n category.name}}</label>
@ -22,31 +37,6 @@
<button class="btn btn-small" {{action showCategoryTopic target="view"}}>{{i18n category.change_in_category_topic}}</button> <button class="btn btn-small" {{action showCategoryTopic target="view"}}>{{i18n category.change_in_category_topic}}</button>
{{/if}} {{/if}}
</section> </section>
<section class='field'>
<label>{{i18n category.security}}</label>
<label>
{{view Ember.Checkbox checkedBinding="secure"}}
{{i18n category.is_secure}}
</label>
{{#if secure}}
<div>
{{i18n category.allowed_groups}}
{{#each groups}}
{{this}} <a {{action removeGroup this target="view"}}>x</a>
{{/each}}
</div>
{{view Ember.Select contentBinding="availableGroups" valueBinding="view.selectedGroup"}}
<button {{action addGroup target="view"}}>{{i18n category.add_group}}</button>
{{/if}}
</section>
<!-- Sam, disabled for now
<section class='field'>
<label>{{i18n category.hotness}}</label>
{{view Discourse.HotnessView hotnessBinding="hotness"}}
</section>
-->
{{/unless}} {{/unless}}
<section class='field'> <section class='field'>
@ -67,9 +57,40 @@
</div> </div>
</div> </div>
</section> </section>
</form> </form>
</div> </div>
{{#unless is_uncategorized}}
<div {{bindAttr class=":modal-tab :options-tab view.securitySelected::invisible"}}>
<section class='field'>
<label>{{i18n category.security}}</label>
<label>
{{view Ember.Checkbox checkedBinding="secure"}}
{{i18n category.is_secure}}
</label>
{{#if secure}}
<div>
{{i18n category.allowed_groups}}
{{#each groups}}
{{this}} <a {{action removeGroup this target="view"}}>x</a>
{{/each}}
</div>
{{view Ember.Select contentBinding="availableGroups" valueBinding="view.selectedGroup"}}
<button {{action addGroup target="view"}}>{{i18n category.add_group}}</button>
{{/if}}
</section>
</div>
<div {{bindAttr class=":modal-tab :options-tab view.settingsSelected::invisible"}}>
<section class='field'>
{{view Discourse.AutoCloseFormView autoCloseDaysBinding="auto_close_days" labelKey="category.auto_close_label"}}
</section>
<section class='field'>
<label>{{i18n category.hotness}}</label>
{{view Discourse.HotnessView hotnessBinding="hotness"}}
</section>
</div>
{{/unless}}
</div>
<div class="modal-footer"> <div class="modal-footer">
<button class='btn btn-primary' {{bindAttr disabled="view.disabled"}} {{action saveCategory target="view"}}>{{view.buttonTitle}}</button> <button class='btn btn-primary' {{bindAttr disabled="view.disabled"}} {{action saveCategory target="view"}}>{{view.buttonTitle}}</button>
{{#if view.deleteVisible}} {{#if view.deleteVisible}}

View file

@ -0,0 +1,21 @@
/**
This view renders the form to set or change a topic or category's auto-close setting.
@class AutoCloseFormView
@extends Ember.View
@namespace Discourse
@module Discourse
**/
Discourse.AutoCloseFormView = Ember.View.extend({
templateName: 'auto_close_form',
label: function() {
return Em.String.i18n( this.get('labelKey') || 'composer.auto_close_label' );
}.property('labelKey'),
autoCloseChanged: function() {
if( this.get('autoCloseDays') && this.get('autoCloseDays').length > 0 ) {
this.set('autoCloseDays', this.get('autoCloseDays').replace(/[^\d]/g, '') )
}
}.observes('autoCloseDays')
});

View file

@ -8,11 +8,32 @@
**/ **/
Discourse.EditCategoryView = Discourse.ModalBodyView.extend({ Discourse.EditCategoryView = Discourse.ModalBodyView.extend({
templateName: 'modal/edit_category', templateName: 'modal/edit_category',
appControllerBinding: 'Discourse.appController', generalSelected: Ember.computed.equal('selectedTab', 'general'),
securitySelected: Ember.computed.equal('selectedTab', 'security'),
// black & white only for foreground colors settingsSelected: Ember.computed.equal('selectedTab', 'settings'),
foregroundColors: ['FFFFFF', '000000'], foregroundColors: ['FFFFFF', '000000'],
init: function() {
this._super();
this.set('selectedTab', 'general');
},
modalClass: function() {
return "edit-category-modal " + (this.present('category.description') ? 'full' : 'small');
}.property(),
selectGeneral: function() {
this.set('selectedTab', 'general');
},
selectSecurity: function() {
this.set('selectedTab', 'security');
},
selectSettings: function() {
this.set('selectedTab', 'settings');
},
disabled: function() { disabled: function() {
if (this.get('saving') || this.get('deleting')) return true; if (this.get('saving') || this.get('deleting')) return true;
if (!this.get('category.name')) return true; if (!this.get('category.name')) return true;

View file

@ -192,3 +192,17 @@
margin-left: 20px; margin-left: 20px;
width: 95% !important; width: 95% !important;
} }
.edit-category-modal {
.modal-body {
position: relative;
height: 350px;
}
&.small .modal-body {
height: 280px;
}
}
.modal-tab {
position: absolute;
}

View file

@ -48,7 +48,7 @@ class CategoriesController < ApplicationController
end end
def category_param_keys def category_param_keys
[required_param_keys, :hotness, :secure, :group_names].flatten! [required_param_keys, :hotness, :secure, :group_names, :auto_close_days].flatten!
end end
def category_params def category_params

View file

@ -95,6 +95,10 @@ class Topic < ActiveRecord::Base
before_create do before_create do
self.bumped_at ||= Time.now self.bumped_at ||= Time.now
self.last_post_user_id ||= user_id self.last_post_user_id ||= user_id
if !@ignore_category_auto_close and self.category and self.category.auto_close_days and self.auto_close_at.nil?
self.auto_close_at = self.category.auto_close_days.days.from_now
self.auto_close_user = (self.user.staff? ? self.user : Discourse.system_user)
end
end end
after_create do after_create do
@ -112,6 +116,7 @@ class Topic < ActiveRecord::Base
before_save do before_save do
if (auto_close_at_changed? and !auto_close_at_was.nil?) or (auto_close_user_id_changed? and auto_close_at) if (auto_close_at_changed? and !auto_close_at_was.nil?) or (auto_close_user_id_changed? and auto_close_at)
Jobs.cancel_scheduled_job(:close_topic, {topic_id: id}) Jobs.cancel_scheduled_job(:close_topic, {topic_id: id})
true
end end
end end
@ -736,6 +741,7 @@ class Topic < ActiveRecord::Base
end end
def auto_close_days=(num_days) def auto_close_days=(num_days)
@ignore_category_auto_close = true
self.auto_close_at = (num_days and num_days.to_i > 0.0 ? num_days.to_i.days.from_now : nil) self.auto_close_at = (num_days and num_days.to_i > 0.0 ? num_days.to_i.days.from_now : nil)
end end

View file

@ -1,6 +1,6 @@
class CategorySerializer < BasicCategorySerializer class CategorySerializer < BasicCategorySerializer
attributes :secure, :groups, :available_groups attributes :secure, :groups, :available_groups, :auto_close_days
def groups def groups
@groups ||= object.groups.order("name").all.map(&:name) @groups ||= object.groups.order("name").all.map(&:name)

View file

@ -775,6 +775,8 @@ en:
edit_long: "Edit Category" edit_long: "Edit Category"
edit_uncategorized: "Edit Uncategorized" edit_uncategorized: "Edit Uncategorized"
view: 'View Topics in Category' view: 'View Topics in Category'
general: 'General'
settings: 'Settings'
delete: 'Delete Category' delete: 'Delete Category'
create: 'Create Category' create: 'Create Category'
save: 'Save Category' save: 'Save Category'
@ -800,6 +802,7 @@ en:
add_group: "Add Group" add_group: "Add Group"
security: "Security" security: "Security"
allowed_groups: "Allowed Groups:" allowed_groups: "Allowed Groups:"
auto_close_label: "Auto-close topics after:"
flagging: flagging:

View file

@ -0,0 +1,5 @@
class AddAutoCloseDaysToCategories < ActiveRecord::Migration
def change
add_column :categories, :auto_close_days, :float
end
end

View file

@ -103,15 +103,19 @@ module Jobs
enqueue_in( [(datetime - Time.zone.now).to_i, 0].max, job_name, opts ) enqueue_in( [(datetime - Time.zone.now).to_i, 0].max, job_name, opts )
end end
# TODO: should take job_name like enqueue methods
def self.cancel_scheduled_job(job_name, params={}) def self.cancel_scheduled_job(job_name, params={})
jobs = scheduled_for(job_name, params)
return false if jobs.empty?
jobs.each { |job| job.delete }
true
end
def self.scheduled_for(job_name, params={})
job_class = "Jobs::#{job_name.to_s.camelcase}" job_class = "Jobs::#{job_name.to_s.camelcase}"
matched = true Sidekiq::ScheduledSet.new.select do |scheduled_job|
Sidekiq::ScheduledSet.new.each do |scheduled_job|
if scheduled_job.klass == 'Sidekiq::Extensions::DelayedClass' if scheduled_job.klass == 'Sidekiq::Extensions::DelayedClass'
job_args = YAML.load(scheduled_job.args[0]) job_args = YAML.load(scheduled_job.args[0])
if job_args[0] == job_class if job_args[0].to_s == job_class and job_args[2] and job_args[2][0]
next unless job_args[2] and job_args[2][0]
matched = true matched = true
params.each do |key, value| params.each do |key, value|
unless job_args[2][0][key] == value unless job_args[2][0][key] == value
@ -119,13 +123,14 @@ module Jobs
break break
end end
end end
next unless matched
end
scheduled_job.delete
break
end
end
matched matched
else
false
end
else
false
end
end
end end
end end

View file

@ -3,7 +3,7 @@ module Jobs
def execute(args) def execute(args)
topic = Topic.find(args[:topic_id]) topic = Topic.find(args[:topic_id])
if topic.auto_close_at if topic and topic.auto_close_at and !topic.closed? and !topic.deleted_at
closer = User.find(args[:user_id]) closer = User.find(args[:user_id])
if Guardian.new(closer).can_moderate?(topic) if Guardian.new(closer).can_moderate?(topic)
topic.update_status('autoclosed', true, closer) topic.update_status('autoclosed', true, closer)

View file

@ -13,23 +13,34 @@ describe Jobs::CloseTopic do
Jobs::CloseTopic.new.execute( topic_id: 123, user_id: 234 ) Jobs::CloseTopic.new.execute( topic_id: 123, user_id: 234 )
end end
it 'does nothing if the topic is not set to auto-close' do shared_examples_for "cases when CloseTopic does nothing" do
topic = Fabricate.build(:topic, auto_close_at: nil, user: admin) it 'does nothing to the topic' do
topic.expects(:update_status).never topic.expects(:update_status).never
Topic.stubs(:find).returns(topic) Topic.stubs(:find).returns(topic)
User.stubs(:find).returns(admin) User.stubs(:find).returns(admin)
Jobs::CloseTopic.new.execute( topic_id: 123, user_id: 234 ) Jobs::CloseTopic.new.execute( topic_id: 123, user_id: 234 )
end end
it 'does nothing if the user is not authorized to close the topic' do
topic = Fabricate.build(:topic, auto_close_at: Time.zone.now, user: admin)
topic.expects(:update_status).never
Topic.stubs(:find).returns(topic)
User.stubs(:find).returns(admin)
Guardian.any_instance.stubs(:can_moderate?).returns(false)
Jobs::CloseTopic.new.execute( topic_id: 123, user_id: 234 )
end end
it 'does nothing if the topic is already closed' 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 end

View file

View file

@ -0,0 +1,85 @@
# encoding: UTF-8
require 'spec_helper'
require 'sidekiq/testing'
describe Topic do
def scheduled_jobs_for(job_name, params={})
Sidekiq::Extensions::DelayedClass.jobs.select do |job|
job_args = YAML.load(job['args'][0])
if job_args[0].to_s == "Jobs::#{job_name.to_s.camelcase}" and job_args[2] and job_args[2][0]
matched = true
params.each do |key, value|
unless job_args[2][0][key] == value
matched = false
break
end
end
matched
end
end
end
before { SiteSetting.stubs(:queue_jobs).returns(true) }
context 'creating a topic without auto-close' do
Given(:topic) { Fabricate(:topic, category: category) }
context 'uncategorized' do
Given(:category) { nil }
Then { topic.auto_close_at.should be_nil }
And { scheduled_jobs_for(:close_topic).should be_empty }
end
context 'category without default auto-close' do
Given(:category) { Fabricate(:category, auto_close_days: nil) }
Then { topic.auto_close_at.should be_nil }
And { scheduled_jobs_for(:close_topic).should be_empty }
end
context 'jobs may be queued' do
before do
Timecop.freeze(Time.zone.now)
end
after do
Timecop.return
Sidekiq::Extensions::DelayedClass.jobs.clear
end
context 'category has a default auto-close' do
Given(:category) { Fabricate(:category, auto_close_days: 2.0) }
Then { topic.auto_close_at.should == 2.days.from_now }
And { scheduled_jobs_for(:close_topic, {topic_id: topic.id}).should have(1).job }
And { scheduled_jobs_for(:close_topic, {topic_id: category.topic.id}).should be_empty }
context 'topic was created by staff user' do
Given(:admin) { Fabricate(:admin) }
Given(:staff_topic) { Fabricate(:topic, user: admin, category: category) }
Then { scheduled_jobs_for(:close_topic, {topic_id: staff_topic.id, user_id: admin.id}).should have(1).job }
end
context 'topic was created by a non-staff user' do
Given!(:system_user) { Fabricate(:admin) }
Given { Discourse.stubs(:system_user).returns(system_user) }
Given(:regular_user) { Fabricate(:user) }
Given(:regular_user_topic) { Fabricate(:topic, user: regular_user, category: category) }
Then { scheduled_jobs_for(:close_topic, {topic_id: regular_user_topic.id, user_id: system_user.id}).should have(1).job }
end
context 'auto_close_days of topic was set to 0' do
Given(:dont_close_topic) { Fabricate(:topic, auto_close_days: 0, category: category) }
Then { scheduled_jobs_for(:close_topic).should be_empty }
end
context 'two topics in the category' do
Given!(:other_topic) { Fabricate(:topic, category: category) }
When { topic } # create the second topic
Then { scheduled_jobs_for(:close_topic).should have(2).jobs }
end
end
end
end
end

View file

@ -985,11 +985,6 @@ describe Topic do
describe 'auto-close' do describe 'auto-close' do
context 'a new topic' do context 'a new topic' do
it 'when auto_close_at is not present, it does not queue a job to close the topic' do
Jobs.expects(:enqueue_at).never
Fabricate(:topic)
end
context 'auto_close_at is set' do context 'auto_close_at is set' do
it 'queues a job to close the topic' do it 'queues a job to close the topic' do
Timecop.freeze(Time.zone.now) do Timecop.freeze(Time.zone.now) do
@ -1014,6 +1009,13 @@ describe Topic do
end end
Fabricate(:topic, auto_close_at: 7.days.from_now, auto_close_user: topic_closer, user: topic_creator) Fabricate(:topic, auto_close_at: 7.days.from_now, auto_close_user: topic_closer, user: topic_creator)
end end
it "ignores the category's default auto-close" do
Timecop.freeze(Time.zone.now) do
Jobs.expects(:enqueue_at).with(7.days.from_now, :close_topic, all_of( has_key(:topic_id), has_key(:user_id) ))
Fabricate(:topic, auto_close_at: 7.days.from_now, user: Fabricate(:admin), category: Fabricate(:category, auto_close_days: 2))
end
end
end end
end end
@ -1090,6 +1092,15 @@ describe Topic do
topic.save.should be_true topic.save.should be_true
end end
end end
it "ignores the category's default auto-close" do
Timecop.freeze(Time.zone.now) do
topic = Fabricate(:topic, category: Fabricate(:category, auto_close_days: 14))
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.should be_true
end
end
end end
end end