From f3282e33a352007b88769de7bcb20d78d5497db7 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 15 May 2013 15:19:41 -0400 Subject: [PATCH] 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 --- Gemfile | 1 + Gemfile.lock | 5 + .../javascripts/discourse/models/category.js | 3 +- .../javascripts/discourse/models/composer.js | 8 +- .../templates/auto_close_form.js.handlebars | 6 + .../templates/composer.js.handlebars | 7 +- .../templates/modal/auto_close.js.handlebars | 7 +- .../modal/edit_category.js.handlebars | 105 +++++++++++------- .../discourse/views/auto_close_form_view.js | 21 ++++ .../views/modal/edit_category_view.js | 27 ++++- .../stylesheets/application/modal.css.scss | 16 ++- app/controllers/categories_controller.rb | 2 +- app/models/topic.rb | 6 + app/serializers/category_serializer.rb | 2 +- config/locales/client.en.yml | 3 + ...93551_add_auto_close_days_to_categories.rb | 5 + lib/jobs.rb | 23 ++-- lib/jobs/close_topic.rb | 2 +- spec/components/jobs/close_topic_spec.rb | 41 ++++--- spec/integration/.gitkeep | 0 spec/integration/topic_auto_close_spec.rb | 85 ++++++++++++++ spec/models/topic_spec.rb | 21 +++- 22 files changed, 298 insertions(+), 98 deletions(-) create mode 100644 app/assets/javascripts/discourse/templates/auto_close_form.js.handlebars create mode 100644 app/assets/javascripts/discourse/views/auto_close_form_view.js create mode 100644 db/migrate/20130515193551_add_auto_close_days_to_categories.rb create mode 100644 spec/integration/.gitkeep create mode 100644 spec/integration/topic_auto_close_spec.rb diff --git a/Gemfile b/Gemfile index 3b5625afd..8c546ed4e 100644 --- a/Gemfile +++ b/Gemfile @@ -108,6 +108,7 @@ group :test, :development do gem 'simplecov', require: false gem 'terminal-notifier-guard', require: false gem 'timecop' + gem 'rspec-given' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index c2e7346a8..c35b94be2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -371,6 +371,9 @@ GEM rspec-core (2.13.1) rspec-expectations (2.13.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-rails (2.13.0) actionpack (>= 3.0) @@ -416,6 +419,7 @@ GEM temple (~> 0.6.3) tilt (~> 1.3.3) slop (3.4.4) + sorcerer (0.3.10) spork (0.9.2) temple (0.6.4) terminal-notifier-guard (1.5.3) @@ -511,6 +515,7 @@ DEPENDENCIES redis-rails rest-client rinku + rspec-given rspec-rails sanitize sass diff --git a/app/assets/javascripts/discourse/models/category.js b/app/assets/javascripts/discourse/models/category.js index 4044a41e5..24db481ed 100644 --- a/app/assets/javascripts/discourse/models/category.js +++ b/app/assets/javascripts/discourse/models/category.js @@ -45,7 +45,8 @@ Discourse.Category = Discourse.Model.extend({ text_color: this.get('text_color'), hotness: this.get('hotness'), 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' }); diff --git a/app/assets/javascripts/discourse/models/composer.js b/app/assets/javascripts/discourse/models/composer.js index 101cb883d..e4d7d5159 100644 --- a/app/assets/javascripts/discourse/models/composer.js +++ b/app/assets/javascripts/discourse/models/composer.js @@ -538,13 +538,7 @@ Discourse.Composer = Discourse.Model.extend({ var reply = this.get('reply') || ""; while (Discourse.BBCode.QUOTE_REGEXP.test(reply)) { reply = reply.replace(Discourse.BBCode.QUOTE_REGEXP, ""); } return reply.replace(/\s+/img, " ").trim().length; - }.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') + }.property('reply') }); diff --git a/app/assets/javascripts/discourse/templates/auto_close_form.js.handlebars b/app/assets/javascripts/discourse/templates/auto_close_form.js.handlebars new file mode 100644 index 000000000..eff55b4f0 --- /dev/null +++ b/app/assets/javascripts/discourse/templates/auto_close_form.js.handlebars @@ -0,0 +1,6 @@ +
+ + {{view.label}} + {{view Discourse.TextField valueBinding="view.autoCloseDays" maxlength="3"}} + {{i18n composer.auto_close_units}} +
diff --git a/app/assets/javascripts/discourse/templates/composer.js.handlebars b/app/assets/javascripts/discourse/templates/composer.js.handlebars index 70cdd2cdb..4150619b4 100644 --- a/app/assets/javascripts/discourse/templates/composer.js.handlebars +++ b/app/assets/javascripts/discourse/templates/composer.js.handlebars @@ -45,12 +45,7 @@
-
- - {{i18n composer.auto_close_label}} - {{view Discourse.TextField valueBinding="content.auto_close_days" maxlength="5"}} - {{i18n composer.auto_close_units}} -
+ {{view Discourse.AutoCloseFormView autoCloseDaysBinding="content.auto_close_days"}}
{{/if}} diff --git a/app/assets/javascripts/discourse/templates/modal/auto_close.js.handlebars b/app/assets/javascripts/discourse/templates/modal/auto_close.js.handlebars index dc3229eaa..c43ab6282 100644 --- a/app/assets/javascripts/discourse/templates/modal/auto_close.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/auto_close.js.handlebars @@ -1,11 +1,6 @@
-
- - {{i18n composer.auto_close_label}} - {{view Discourse.TextField valueBinding="view.auto_close_days" maxlength="5"}} - {{i18n composer.auto_close_units}} -
+ {{view Discourse.AutoCloseFormView autoCloseDaysBinding="view.auto_close_days"}}
diff --git a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars index 74fcc303c..8969e1c51 100644 --- a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars @@ -1,28 +1,66 @@ {{#with view.category}}
+ +
-
-
- - {{view Discourse.TextField valueBinding="name" placeholderKey="category.name_placeholder" maxlength="50"}} -
- - {{#unless is_uncategorized}} +
+
- - - {{#if description}} - {{description}} - {{else}} - {{i18n category.no_description}} - {{/if}} - {{#if topic_url}} -
- - {{/if}} + + {{view Discourse.TextField valueBinding="name" placeholderKey="category.name_placeholder" maxlength="50"}}
+ {{#unless is_uncategorized}} +
+ + + {{#if description}} + {{description}} + {{else}} + {{i18n category.no_description}} + {{/if}} + {{#if topic_url}} +
+ + {{/if}} +
+ {{/unless}} + +
+ +
+ {{view.categoryName}} + +
+ {{i18n category.background_color}}: + #{{view Discourse.TextField valueBinding="color" placeholderKey="category.color_placeholder" maxlength="6"}} + {{view Discourse.ColorsView colorsBinding="view.backgroundColors" usedColorsBinding="view.usedBackgroundColors" valueBinding="color"}} +
+ +
+ {{i18n category.foreground_color}}: + #{{view Discourse.TextField valueBinding="text_color" placeholderKey="category.color_placeholder" maxlength="6"}} + {{view Discourse.ColorsView colorsBinding="view.foregroundColors" valueBinding="text_color"}} +
+
+
+ +
+ {{#unless is_uncategorized}} +
+
+
+
+ {{view Discourse.AutoCloseFormView autoCloseDaysBinding="auto_close_days" labelKey="category.auto_close_label"}} +
- - {{/unless}} - -
- -
- {{view.categoryName}} - -
- {{i18n category.background_color}}: - #{{view Discourse.TextField valueBinding="color" placeholderKey="category.color_placeholder" maxlength="6"}} - {{view Discourse.ColorsView colorsBinding="view.backgroundColors" usedColorsBinding="view.usedBackgroundColors" valueBinding="color"}} -
- -
- {{i18n category.foreground_color}}: - #{{view Discourse.TextField valueBinding="text_color" placeholderKey="category.color_placeholder" maxlength="6"}} - {{view Discourse.ColorsView colorsBinding="view.foregroundColors" valueBinding="text_color"}} -
-
-
- - +
+ {{/unless}}
diff --git a/app/assets/javascripts/discourse/views/auto_close_form_view.js b/app/assets/javascripts/discourse/views/auto_close_form_view.js new file mode 100644 index 000000000..ccbe6a672 --- /dev/null +++ b/app/assets/javascripts/discourse/views/auto_close_form_view.js @@ -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') +}); \ No newline at end of file diff --git a/app/assets/javascripts/discourse/views/modal/edit_category_view.js b/app/assets/javascripts/discourse/views/modal/edit_category_view.js index 264c76efb..951981b57 100644 --- a/app/assets/javascripts/discourse/views/modal/edit_category_view.js +++ b/app/assets/javascripts/discourse/views/modal/edit_category_view.js @@ -8,11 +8,32 @@ **/ Discourse.EditCategoryView = Discourse.ModalBodyView.extend({ templateName: 'modal/edit_category', - appControllerBinding: 'Discourse.appController', - - // black & white only for foreground colors + generalSelected: Ember.computed.equal('selectedTab', 'general'), + securitySelected: Ember.computed.equal('selectedTab', 'security'), + settingsSelected: Ember.computed.equal('selectedTab', 'settings'), 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() { if (this.get('saving') || this.get('deleting')) return true; if (!this.get('category.name')) return true; diff --git a/app/assets/stylesheets/application/modal.css.scss b/app/assets/stylesheets/application/modal.css.scss index a27297547..99bdad6fe 100644 --- a/app/assets/stylesheets/application/modal.css.scss +++ b/app/assets/stylesheets/application/modal.css.scss @@ -191,4 +191,18 @@ .flag-message { margin-left: 20px; width: 95% !important; -} \ No newline at end of file +} + +.edit-category-modal { + .modal-body { + position: relative; + height: 350px; + } + &.small .modal-body { + height: 280px; + } +} + +.modal-tab { + position: absolute; +} diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 5db92be7e..db8e07bcf 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -48,7 +48,7 @@ class CategoriesController < ApplicationController end def category_param_keys - [required_param_keys, :hotness, :secure, :group_names].flatten! + [required_param_keys, :hotness, :secure, :group_names, :auto_close_days].flatten! end def category_params diff --git a/app/models/topic.rb b/app/models/topic.rb index 4ff2a501d..a521b60b1 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -95,6 +95,10 @@ class Topic < ActiveRecord::Base before_create do self.bumped_at ||= Time.now 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 after_create do @@ -112,6 +116,7 @@ class Topic < ActiveRecord::Base before_save do 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}) + true end end @@ -736,6 +741,7 @@ class Topic < ActiveRecord::Base end 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) end diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb index 2e8037422..95c851c28 100644 --- a/app/serializers/category_serializer.rb +++ b/app/serializers/category_serializer.rb @@ -1,6 +1,6 @@ class CategorySerializer < BasicCategorySerializer - attributes :secure, :groups, :available_groups + attributes :secure, :groups, :available_groups, :auto_close_days def groups @groups ||= object.groups.order("name").all.map(&:name) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b7f906320..c6c89bd7d 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -775,6 +775,8 @@ en: edit_long: "Edit Category" edit_uncategorized: "Edit Uncategorized" view: 'View Topics in Category' + general: 'General' + settings: 'Settings' delete: 'Delete Category' create: 'Create Category' save: 'Save Category' @@ -800,6 +802,7 @@ en: add_group: "Add Group" security: "Security" allowed_groups: "Allowed Groups:" + auto_close_label: "Auto-close topics after:" flagging: diff --git a/db/migrate/20130515193551_add_auto_close_days_to_categories.rb b/db/migrate/20130515193551_add_auto_close_days_to_categories.rb new file mode 100644 index 000000000..d2e16e0c1 --- /dev/null +++ b/db/migrate/20130515193551_add_auto_close_days_to_categories.rb @@ -0,0 +1,5 @@ +class AddAutoCloseDaysToCategories < ActiveRecord::Migration + def change + add_column :categories, :auto_close_days, :float + end +end diff --git a/lib/jobs.rb b/lib/jobs.rb index f413cf1e5..31032c4a1 100644 --- a/lib/jobs.rb +++ b/lib/jobs.rb @@ -103,15 +103,19 @@ module Jobs enqueue_in( [(datetime - Time.zone.now).to_i, 0].max, job_name, opts ) end - # TODO: should take job_name like enqueue methods 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}" - matched = true - Sidekiq::ScheduledSet.new.each do |scheduled_job| + Sidekiq::ScheduledSet.new.select do |scheduled_job| if scheduled_job.klass == 'Sidekiq::Extensions::DelayedClass' job_args = YAML.load(scheduled_job.args[0]) - if job_args[0] == job_class - next unless job_args[2] and job_args[2][0] + if job_args[0].to_s == job_class and job_args[2] and job_args[2][0] matched = true params.each do |key, value| unless job_args[2][0][key] == value @@ -119,13 +123,14 @@ module Jobs break end end - next unless matched + matched + else + false end - scheduled_job.delete - break + else + false end end - matched end end diff --git a/lib/jobs/close_topic.rb b/lib/jobs/close_topic.rb index 61c8d42b5..32565c48b 100644 --- a/lib/jobs/close_topic.rb +++ b/lib/jobs/close_topic.rb @@ -3,7 +3,7 @@ module Jobs def execute(args) 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]) if Guardian.new(closer).can_moderate?(topic) topic.update_status('autoclosed', true, closer) diff --git a/spec/components/jobs/close_topic_spec.rb b/spec/components/jobs/close_topic_spec.rb index 3850917ba..2ff4a7808 100644 --- a/spec/components/jobs/close_topic_spec.rb +++ b/spec/components/jobs/close_topic_spec.rb @@ -13,23 +13,34 @@ describe Jobs::CloseTopic do Jobs::CloseTopic.new.execute( topic_id: 123, user_id: 234 ) end - it 'does nothing if the topic is not set to auto-close' do - topic = Fabricate.build(:topic, auto_close_at: nil, user: admin) - topic.expects(:update_status).never - Topic.stubs(:find).returns(topic) - User.stubs(:find).returns(admin) - Jobs::CloseTopic.new.execute( topic_id: 123, user_id: 234 ) + shared_examples_for "cases when CloseTopic does nothing" do + it 'does nothing to the topic' do + topic.expects(:update_status).never + Topic.stubs(:find).returns(topic) + User.stubs(:find).returns(admin) + 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 ) + 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 - it 'does nothing if the topic is already closed' + 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 -end \ No newline at end of file + 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/integration/.gitkeep b/spec/integration/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/spec/integration/topic_auto_close_spec.rb b/spec/integration/topic_auto_close_spec.rb new file mode 100644 index 000000000..97670f841 --- /dev/null +++ b/spec/integration/topic_auto_close_spec.rb @@ -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 diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 992476f6f..ad7f12f3e 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -985,11 +985,6 @@ describe Topic do describe 'auto-close' 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 it 'queues a job to close the topic' do Timecop.freeze(Time.zone.now) do @@ -1014,6 +1009,13 @@ describe Topic do end Fabricate(:topic, auto_close_at: 7.days.from_now, auto_close_user: topic_closer, user: topic_creator) 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 @@ -1090,6 +1092,15 @@ describe Topic do topic.save.should be_true 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