From 37673e3412d638a72e56e3f16716c7a2041404d7 Mon Sep 17 00:00:00 2001 From: Viktor Palmkvist Date: Thu, 18 Jul 2013 00:58:25 +0200 Subject: [PATCH] Make the composer and TopicCreator use category id instead of category name Also fixes #1171 Includes backwards compatibility for topic creation --- .../discourse/controllers/list_controller.js | 2 +- .../javascripts/discourse/models/composer.js | 27 +++++++------------ .../templates/composer.js.handlebars | 2 +- lib/topic_creator.rb | 9 ++++++- spec/components/post_creator_spec.rb | 4 +-- spec/models/category_featured_topic_spec.rb | 4 +-- test/javascripts/models/composer_test.js | 4 +-- 7 files changed, 26 insertions(+), 26 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/list_controller.js b/app/assets/javascripts/discourse/controllers/list_controller.js index 0fb4fd929..07bc684c4 100644 --- a/app/assets/javascripts/discourse/controllers/list_controller.js +++ b/app/assets/javascripts/discourse/controllers/list_controller.js @@ -115,7 +115,7 @@ Discourse.ListController = Discourse.Controller.extend({ // Create topic button createTopic: function() { this.get('controllers.composer').open({ - categoryName: this.get('category.name'), + categoryId: this.get('category.id'), action: Discourse.Composer.CREATE_TOPIC, draft: this.get('draft'), draftKey: this.get('draft_key'), diff --git a/app/assets/javascripts/discourse/models/composer.js b/app/assets/javascripts/discourse/models/composer.js index bb33e6456..8981feb34 100644 --- a/app/assets/javascripts/discourse/models/composer.js +++ b/app/assets/javascripts/discourse/models/composer.js @@ -121,10 +121,10 @@ Discourse.Composer = Discourse.Model.extend({ // reply is always required if (this.get('missingReplyCharacters') > 0) return true; - if (this.get('canCategorize') && !Discourse.SiteSettings.allow_uncategorized_topics && !this.get('categoryName')) return true; + if (this.get('canCategorize') && !Discourse.SiteSettings.allow_uncategorized_topics && !this.get('categoryId')) return true; return false; - }.property('loading', 'canEditTitle', 'titleLength', 'targetUsernames', 'replyLength', 'categoryName', 'missingReplyCharacters'), + }.property('loading', 'canEditTitle', 'titleLength', 'targetUsernames', 'replyLength', 'categoryId', 'missingReplyCharacters'), /** Is the title's length valid? @@ -328,14 +328,14 @@ Discourse.Composer = Discourse.Model.extend({ } this.setProperties({ - categoryName: opts.categoryName || this.get('topic.category.name'), + categoryId: opts.categoryId || this.get('topic.category.id'), archetypeId: opts.archetypeId || Discourse.Site.currentProp('default_archetype'), metaData: opts.metaData ? Em.Object.create(opts.metaData) : null, reply: opts.reply || this.get("reply") || "" }); - if (!this.get('categoryName') && !Discourse.SiteSettings.allow_uncategorized_topics && Discourse.Category.list().length > 0) { - this.set('categoryName', Discourse.Category.list()[0].get('name')); + 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) { @@ -398,16 +398,9 @@ Discourse.Composer = Discourse.Model.extend({ var topic = this.get('topic'); topic.setProperties({ title: this.get('title'), - fancy_title: this.get('title') + fancy_title: this.get('title'), + category_id: parseInt(this.get('categoryId'), 10) }); - - var category = Discourse.Category.list().findProperty('name', this.get('categoryName')); - if (category) { - topic.setProperties({ - categoryName: category.get('name'), - category_id: category.get('id') - }); - } topic.save(); } @@ -447,7 +440,7 @@ Discourse.Composer = Discourse.Model.extend({ var createdPost = Discourse.Post.create({ raw: this.get('reply'), title: this.get('title'), - category: this.get('categoryName'), + category: this.get('categoryId'), topic_id: this.get('topic.id'), reply_to_post_number: post ? post.get('post_number') : null, imageSizes: opts.imageSizes, @@ -544,7 +537,7 @@ Discourse.Composer = Discourse.Model.extend({ reply: this.get('reply'), action: this.get('action'), title: this.get('title'), - categoryName: this.get('categoryName'), + categoryId: this.get('categoryId'), postId: this.get('post.id'), archetypeId: this.get('archetypeId'), metaData: this.get('metaData'), @@ -599,7 +592,7 @@ Discourse.Composer.reopenClass({ topic: topic, action: draft.action, title: draft.title, - categoryName: draft.categoryName, + categoryId: draft.categoryId, postId: draft.postId, archetypeId: draft.archetypeId, reply: draft.reply, diff --git a/app/assets/javascripts/discourse/templates/composer.js.handlebars b/app/assets/javascripts/discourse/templates/composer.js.handlebars index daed39384..cb47da8e1 100644 --- a/app/assets/javascripts/discourse/templates/composer.js.handlebars +++ b/app/assets/javascripts/discourse/templates/composer.js.handlebars @@ -30,7 +30,7 @@ {{#unless model.creatingPrivateMessage}}
- {{categoryChooser valueAttribute="name" value=model.categoryName}} + {{categoryChooser valueAttribute="id" value=model.categoryId}} {{popupInputTip validation=view.categoryValidation shownAt=view.showCategoryTip}}
{{#if model.archetype.hasOptions}} diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index f11e4a341..e60051e1a 100644 --- a/lib/topic_creator.rb +++ b/lib/topic_creator.rb @@ -39,7 +39,14 @@ class TopicCreator topic_params[:archetype] = @opts[:archetype] if @opts[:archetype].present? topic_params[:subtype] = @opts[:subtype] if @opts[:subtype].present? - category = Category.where(name: @opts[:category]).first + # Temporary fix to allow older clients to create topics. + # When all clients are updated the category variable should + # be set directly to the contents of the if statement. + category = if (@opts[:category].is_a? Integer) || (@opts[:category] =~ /^\d+$/) + Category.where(id: @opts[:category]).first + else + Category.where(name: @opts[:category]).first + end @guardian.ensure_can_create!(Topic,category) topic_params[:category_id] = category.id if category.present? diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 767e1ca13..528367c01 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -17,7 +17,7 @@ describe PostCreator do let(:image_sizes) { {'http://an.image.host/image.jpg' => {"width" => 111, "height" => 222}} } let(:creator) { PostCreator.new(user, basic_topic_params) } - let(:creator_with_category) { PostCreator.new(user, basic_topic_params.merge(category: category.name )) } + let(:creator_with_category) { PostCreator.new(user, basic_topic_params.merge(category: category.id )) } let(:creator_with_meta_data) { PostCreator.new(user, basic_topic_params.merge(meta_data: {hello: "world"} )) } let(:creator_with_image_sizes) { PostCreator.new(user, basic_topic_params.merge(image_sizes: image_sizes)) } @@ -75,7 +75,7 @@ describe PostCreator do reply = nil messages = MessageBus.track_publish do - created_post = PostCreator.new(admin, basic_topic_params.merge(category: cat.name)).create + created_post = PostCreator.new(admin, basic_topic_params.merge(category: cat.id)).create reply = PostCreator.new(admin, raw: "this is my test reply 123 testing", topic_id: created_post.topic_id).create end diff --git a/spec/models/category_featured_topic_spec.rb b/spec/models/category_featured_topic_spec.rb index bde98f0ee..28119a786 100644 --- a/spec/models/category_featured_topic_spec.rb +++ b/spec/models/category_featured_topic_spec.rb @@ -8,7 +8,7 @@ describe CategoryFeaturedTopic do context 'feature_topics_for' do let(:user) { Fabricate(:user) } let(:category) { Fabricate(:category) } - let!(:category_post) { PostCreator.create(user, raw: "I put this post in the category", title: "categorize THIS", category: category.name) } + let!(:category_post) { PostCreator.create(user, raw: "I put this post in the category", title: "categorize THIS", category: category.id) } it "should feature topics for a secure category" do @@ -26,7 +26,7 @@ describe CategoryFeaturedTopic do end it 'should not include invisible topics' do - invisible_post = PostCreator.create(user, raw: "Don't look at this post because it's awful.", title: "not visible to anyone", category: category.name) + invisible_post = PostCreator.create(user, raw: "Don't look at this post because it's awful.", title: "not visible to anyone", category: category.id) invisible_post.topic.update_status('visible', false, Fabricate(:admin)) CategoryFeaturedTopic.feature_topics_for(category) CategoryFeaturedTopic.count.should == 1 diff --git a/test/javascripts/models/composer_test.js b/test/javascripts/models/composer_test.js index 001fc230c..b0c71193d 100644 --- a/test/javascripts/models/composer_test.js +++ b/test/javascripts/models/composer_test.js @@ -180,11 +180,11 @@ test('clearState', function() { test('initial category when uncategorized is allowed', function() { Discourse.SiteSettings.allow_uncategorized_topics = true; var composer = Discourse.Composer.open({action: 'createTopic', draftKey: 'asfd', draftSequence: 1}); - equal(composer.get('categoryName'),undefined,"Uncategorized by default"); + equal(composer.get('categoryId'),undefined,"Uncategorized by default"); }); test('initial category when uncategorized is not allowed', function() { Discourse.SiteSettings.allow_uncategorized_topics = false; var composer = Discourse.Composer.open({action: 'createTopic', draftKey: 'asfd', draftSequence: 1}); - ok(composer.get('categoryName') !== undefined, "Not uncategorized by default"); + ok(composer.get('categoryId') !== undefined, "Not uncategorized by default"); }); \ No newline at end of file