When creating a topic, don't select a category by default when allow_uncategorized_topics is false. Also, added category validation on the server to enforce allow_uncategorized_topics.

This commit is contained in:
Neil Lalonde 2013-10-08 14:40:31 -04:00
parent 32af23884e
commit bccb37b6f3
9 changed files with 92 additions and 20 deletions

View file

@ -339,10 +339,6 @@ Discourse.Composer = Discourse.Model.extend({
reply: opts.reply || this.get("reply") || "" reply: opts.reply || this.get("reply") || ""
}); });
if (!this.get('categoryId') && !Discourse.SiteSettings.allow_uncategorized_topics && Discourse.Category.list().length > 0) {
this.set('categoryId', Discourse.Category.list()[0].get('id'));
}
if (opts.postId) { if (opts.postId) {
this.set('loading', true); this.set('loading', true);
Discourse.Post.load(opts.postId).then(function(result) { Discourse.Post.load(opts.postId).then(function(result) {

View file

@ -21,7 +21,11 @@ Discourse.CategoryChooserView = Discourse.ComboboxView.extend({
}, },
none: function() { none: function() {
if (Discourse.SiteSettings.allow_uncategorized_topics || this.get('showUncategorized')) return 'category.none'; if (!Discourse.SiteSettings.allow_uncategorized_topics) {
return 'category.choose';
} else if (Discourse.SiteSettings.allow_uncategorized_topics || this.get('showUncategorized')) {
return 'category.none';
}
}.property('showUncategorized'), }.property('showUncategorized'),
template: function(text, templateData) { template: function(text, templateData) {

View file

@ -335,7 +335,7 @@
.category-input .popup-tip { .category-input .popup-tip {
width: 240px; width: 240px;
left: 432px; left: 432px;
top: -7px; top: -19px;
} }
} }

View file

@ -108,7 +108,7 @@ class TopicsController < ApplicationController
success = false success = false
Topic.transaction do Topic.transaction do
success = topic.save success = topic.save
topic.change_category(params[:category]) if success success = topic.change_category(params[:category]) if success
end end
# this is used to return the title to the client as it may have been # this is used to return the title to the client as it may have been

View file

@ -56,6 +56,12 @@ class Topic < ActiveRecord::Base
:case_sensitive => false, :case_sensitive => false,
:collection => Proc.new{ Topic.listable_topics } } :collection => Proc.new{ Topic.listable_topics } }
# The allow_uncategorized_topics site setting can be changed at any time, so there may be
# existing topics with nil category. We'll allow that, but when someone tries to make a new
# topic or change a topic's category, perform validation.
attr_accessor :do_category_validation
validates :category_id, :presence => { :if => Proc.new { @do_category_validation && !SiteSetting.allow_uncategorized_topics } }
before_validation do before_validation do
self.sanitize_title self.sanitize_title
self.title = TextCleaner.clean_title(TextSentinel.title_sentinel(title).text) if errors[:title].empty? self.title = TextCleaner.clean_title(TextSentinel.title_sentinel(title).text) if errors[:title].empty?
@ -135,6 +141,7 @@ 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
self.do_category_validation = true
if !@ignore_category_auto_close and self.category and self.category.auto_close_days and self.auto_close_at.nil? if !@ignore_category_auto_close and self.category and self.category.auto_close_days and self.auto_close_at.nil?
set_auto_close(self.category.auto_close_days) set_auto_close(self.category.auto_close_days)
end end
@ -316,8 +323,8 @@ class Topic < ActiveRecord::Base
def changed_to_category(cat) def changed_to_category(cat)
return if cat.blank? return true if cat.blank?
return if Category.where(topic_id: id).first.present? return true if Category.where(topic_id: id).first.present?
Topic.transaction do Topic.transaction do
old_category = category old_category = category
@ -327,13 +334,16 @@ class Topic < ActiveRecord::Base
end end
self.category_id = cat.id self.category_id = cat.id
save if save
CategoryFeaturedTopic.feature_topics_for(old_category) CategoryFeaturedTopic.feature_topics_for(old_category)
Category.where(id: cat.id).update_all 'topic_count = topic_count + 1' Category.where(id: cat.id).update_all 'topic_count = topic_count + 1'
CategoryFeaturedTopic.feature_topics_for(cat) unless old_category.try(:id) == cat.try(:id) CategoryFeaturedTopic.feature_topics_for(cat) unless old_category.try(:id) == cat.try(:id)
else
return false
end end
end end
true
end
def add_moderator_post(user, text, opts={}) def add_moderator_post(user, text, opts={})
new_post = nil new_post = nil
@ -362,6 +372,8 @@ class Topic < ActiveRecord::Base
# Changes the category to a new name # Changes the category to a new name
def change_category(name) def change_category(name)
self.do_category_validation = true
# If the category name is blank, reset the attribute # If the category name is blank, reset the attribute
if name.blank? if name.blank?
if category_id.present? if category_id.present?
@ -369,12 +381,11 @@ class Topic < ActiveRecord::Base
Category.where(id: category_id).update_all 'topic_count = topic_count - 1' Category.where(id: category_id).update_all 'topic_count = topic_count - 1'
end end
self.category_id = nil self.category_id = nil
save return save
return
end end
cat = Category.where(name: name).first cat = Category.where(name: name).first
return if cat == category return true if cat == category
changed_to_category(cat) changed_to_category(cat)
end end

View file

@ -925,6 +925,7 @@ en:
category: category:
can: 'can&hellip; ' can: 'can&hellip; '
none: '(no category)' none: '(no category)'
choose: 'Select a category&hellip;'
edit: 'edit' edit: 'edit'
edit_long: "Edit Category" edit_long: "Edit Category"
edit_uncategorized: "Edit Uncategorized" edit_uncategorized: "Edit Uncategorized"

View file

@ -637,7 +637,7 @@ describe TopicsController do
end end
it 'triggers a change of category' do it 'triggers a change of category' do
Topic.any_instance.expects(:change_category).with('incredible') Topic.any_instance.expects(:change_category).with('incredible').returns(true)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category: 'incredible' xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category: 'incredible'
end end
@ -646,6 +646,24 @@ describe TopicsController do
expect(response).not_to be_success expect(response).not_to be_success
end end
it "returns errors with invalid categories" do
Topic.any_instance.expects(:change_category).returns(false)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category: ''
expect(response).not_to be_success
end
context "allow_uncategorized_topics is false" do
before do
SiteSetting.stubs(:allow_uncategorized_topics).returns(false)
end
it "can add a category to an uncategorized topic" do
Topic.any_instance.expects(:change_category).with('incredible').returns(true)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category: 'incredible'
response.should be_success
end
end
end end
end end
end end

View file

@ -175,6 +175,38 @@ describe Topic do
end end
context 'category validation' do
context 'allow_uncategorized_topics is false' do
before do
SiteSetting.stubs(:allow_uncategorized_topics).returns(false)
end
it "does not allow nil category" do
topic = Fabricate(:topic, category: nil)
topic.should_not be_valid
topic.errors[:category_id].should be_present
end
it 'passes for topics with a category' do
Fabricate.build(:topic, category: Fabricate(:category)).should be_valid
end
end
context 'allow_uncategorized_topics is true' do
before do
SiteSetting.stubs(:allow_uncategorized_topics).returns(true)
end
it "passes for topics with nil category" do
Fabricate.build(:topic, category: nil).should be_valid
end
it 'passes for topics with a category' do
Fabricate.build(:topic, category: Fabricate(:category)).should be_valid
end
end
end
context 'similar_to' do context 'similar_to' do
@ -825,7 +857,18 @@ describe Topic do
it "should lower the original category's topic count" do it "should lower the original category's topic count" do
@category.topic_count.should == 0 @category.topic_count.should == 0
end end
end
context 'when allow_uncategorized_topics is false' do
before do
SiteSetting.stubs(:allow_uncategorized_topics).returns(false)
end
let!(:topic) { Fabricate(:topic, category: Fabricate(:category)) }
it 'returns false' do
topic.change_category('').should eq(false) # don't use "be_false" here because it would also match nil
end
end end
describe 'when the category exists' do describe 'when the category exists' do
@ -838,7 +881,6 @@ describe Topic do
@topic.category_id.should be_blank @topic.category_id.should be_blank
@category.topic_count.should == 0 @category.topic_count.should == 0
end end
end end
end end

View file

@ -186,5 +186,5 @@ test('initial category when uncategorized is allowed', function() {
test('initial category when uncategorized is not allowed', function() { test('initial category when uncategorized is not allowed', function() {
Discourse.SiteSettings.allow_uncategorized_topics = false; Discourse.SiteSettings.allow_uncategorized_topics = false;
var composer = Discourse.Composer.open({action: 'createTopic', draftKey: 'asfd', draftSequence: 1}); var composer = Discourse.Composer.open({action: 'createTopic', draftKey: 'asfd', draftSequence: 1});
ok(composer.get('categoryId') !== undefined, "Not uncategorized by default"); ok(composer.get('categoryId') === undefined, "Uncategorized by default. Must choose a category.");
}); });