From d23ef1d090bd0ee6e476d1b46aa6c913dc6a8c7f Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Fri, 31 May 2013 15:22:34 -0400
Subject: [PATCH] FIX: You could update a topic to have a title that's too
short if the TextCleaner removed extra characters. Additionally, updating the
title will not return an error message to the client app if the operation
fails (rather than failing silently.)
---
.../javascripts/discourse/views/topic_view.js | 15 ++++++++++++++-
app/controllers/topics_controller.rb | 11 ++++++++---
app/models/post.rb | 2 ++
app/models/topic.rb | 2 +-
spec/controllers/topics_controller_spec.rb | 5 +++++
spec/models/topic_spec.rb | 9 +++++++++
6 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/app/assets/javascripts/discourse/views/topic_view.js b/app/assets/javascripts/discourse/views/topic_view.js
index a4d2a4054..384c01622 100644
--- a/app/assets/javascripts/discourse/views/topic_view.js
+++ b/app/assets/javascripts/discourse/views/topic_view.js
@@ -306,6 +306,11 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, {
},
finishedEdit: function() {
+
+ // TODO: This should be in a controller and use proper text fields
+
+ var topicView = this;
+
if (this.get('editingTopic')) {
var topic = this.get('topic');
// retrieve the title from the text field
@@ -326,9 +331,17 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, {
title: title,
fancy_title: title
});
+
+ }, function(error) {
+ topicView.set('editingTopic', true);
+ if (error && error.responseText) {
+ bootbox.alert($.parseJSON(error.responseText).errors[0]);
+ } else {
+ bootbox.alert(Em.String.i18n('generic_error'));
+ }
});
// close editing mode
- this.set('editingTopic', false);
+ topicView.set('editingTopic', false);
}
},
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 5f19c2c6d..d9be939db 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -62,14 +62,19 @@ class TopicsController < ApplicationController
topic.archetype = "regular" if params[:archetype] == 'regular'
end
+ success = false
Topic.transaction do
- topic.save
- topic.change_category(params[:category])
+ success = topic.save
+ topic.change_category(params[:category]) if success
end
# this is used to return the title to the client as it may have been
# changed by "TextCleaner"
- render_serialized(topic, BasicTopicSerializer)
+ if success
+ render_serialized(topic, BasicTopicSerializer)
+ else
+ render_json_error(topic)
+ end
end
def similar_to
diff --git a/app/models/post.rb b/app/models/post.rb
index ae977efcd..7c4525e61 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -327,6 +327,8 @@ class Post < ActiveRecord::Base
# TODO: Move some of this into an asynchronous job?
# TODO: Move into PostCreator
after_create do
+
+ Rails.logger.info (">" * 30) + "#{no_bump} #{created_at}"
# Update attributes on the topic - featured users and last posted.
attrs = {last_posted_at: created_at, last_post_user_id: user_id}
attrs[:bumped_at] = created_at unless no_bump
diff --git a/app/models/topic.rb b/app/models/topic.rb
index 25c624608..80b3a30d4 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -48,7 +48,7 @@ class Topic < ActiveRecord::Base
:case_sensitive => false,
:collection => Proc.new{ Topic.listable_topics } }
- after_validation do
+ before_validation do
self.title = TextCleaner.clean_title(TextSentinel.title_sentinel(title).text) if errors[:title].empty?
end
diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb
index 8e6255535..fe600a116 100644
--- a/spec/controllers/topics_controller_spec.rb
+++ b/spec/controllers/topics_controller_spec.rb
@@ -487,6 +487,11 @@ describe TopicsController do
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category: 'incredible'
end
+ it "returns errors with invalid titles" do
+ xhr :put, :update, topic_id: @topic.id, slug: @topic.title, title: 'asdf'
+ expect(response).not_to be_success
+ end
+
end
end
end
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index c2fd08125..e65ab4cb4 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -43,6 +43,15 @@ describe Topic do
end
+ context "updating a title to be shorter" do
+ let!(:topic) { Fabricate(:topic) }
+
+ it "doesn't update it to be shorter due to cleaning using TextCleaner" do
+ topic.title = 'unread glitch'
+ topic.save.should be_false
+ end
+ end
+
context 'topic title uniqueness' do
let!(:topic) { Fabricate(:topic) }