FIX: Unintended edits with composer

app/assets/javascripts/discourse.js

@@ -107,12 +107,6 @@ Discourse = Ember.Application.createWithMixins({
     this.set('notifyCount', count);
   },
 
-  openComposer: function(opts) {
-    // TODO, remove container link
-    var composer = Discourse.__container__.lookup('controller:composer');
-    if (composer) composer.open(opts);
-  },
-
   /**
     Establishes global DOM events and bindings via jQuery.
 

app/assets/javascripts/discourse/controllers/composer_controller.js

@@ -202,6 +202,7 @@ Discourse.ComposerController = Discourse.Controller.extend({
   **/
   open: function(opts) {
     if (!opts) opts = {};
+
     var promise = opts.promise || Ember.Deferred.create();
     opts.promise = promise;
     this.set('typedReply', false);
@@ -273,7 +274,8 @@ Discourse.ComposerController = Discourse.Controller.extend({
       }
     }
 
-    composer = composer || Discourse.Composer.open(opts);
+    composer = composer || Discourse.Composer.create();
+    composer.open(opts);
     this.set('model', composer);
     composer.set('composeState', Discourse.Composer.OPEN);
     promise.resolve();

app/assets/javascripts/discourse/models/composer.js

@@ -7,19 +7,17 @@
   @module Discourse
 **/
 
-var CLOSED, CREATE_TOPIC, DRAFT, EDIT, OPEN, PRIVATE_MESSAGE, REPLY, REPLY_AS_NEW_TOPIC_KEY, SAVING;
+var CLOSED = 'closed',
+    SAVING = 'saving',
+    OPEN = 'open',
+    DRAFT = 'draft',
 
-CLOSED = 'closed';
-SAVING = 'saving';
-OPEN = 'open';
-DRAFT = 'draft';
-
-// The actions the composer can take
-CREATE_TOPIC = 'createTopic';
-PRIVATE_MESSAGE = 'privateMessage';
-REPLY = 'reply';
-EDIT = 'edit';
-REPLY_AS_NEW_TOPIC_KEY = "reply_as_new_topic";
+    // The actions the composer can take
+    CREATE_TOPIC = 'createTopic',
+    PRIVATE_MESSAGE = 'privateMessage',
+    REPLY = 'reply',
+    EDIT = 'edit',
+    REPLY_AS_NEW_TOPIC_KEY = "reply_as_new_topic";
 
 Discourse.Composer = Discourse.Model.extend({
 
@@ -70,23 +68,14 @@ Discourse.Composer = Discourse.Model.extend({
     Discourse.KeyValueStore.set({ key: 'composer.showPreview', value: this.get('showPreview') });
   },
 
-  // Import a quote from the post
   importQuote: function() {
-    var post = this.get('post');
-
-    // If we don't have a post, check the topic for the first one
-    if (!post) {
-      var posts = this.get('topic.posts');
-      if (posts && posts.length > 0) {
-        post = posts[0];
-      }
-    }
-
-    if (post) {
+    // If there is no current post, use the post id from the stream
+    var postId = this.get('post.id') || this.get('topic.postStream.firstPostId');
+    if (postId) {
       this.set('loading', true);
       var composer = this;
-      Discourse.Post.load(post.get('id')).then(function(result) {
-        composer.appendText(Discourse.BBCode.buildQuoteBBCode(post, result.get('raw')));
+      return Discourse.Post.load(postId).then(function(post) {
+        composer.appendText(Discourse.BBCode.buildQuoteBBCode(post, post.get('raw')));
         composer.set('loading', false);
       });
     }
@@ -211,7 +200,6 @@ Discourse.Composer = Discourse.Model.extend({
   */
   open: function(opts) {
     if (!opts) opts = {};
-
     this.set('loading', false);
 
     var replyBlank = Em.isEmpty(this.get("reply"));
@@ -221,7 +209,7 @@ Discourse.Composer = Discourse.Model.extend({
         (opts.action !== this.get('action') || ((opts.reply || opts.action === this.EDIT) && this.get('reply') !== this.get('originalText'))) &&
         !opts.tested) {
       opts.tested = true;
-      this.cancel(function() { composer.open(opts); });
+      //composer.cancel(function() { composer.open(opts); });
       return;
     }
 
@@ -288,16 +276,18 @@ Discourse.Composer = Discourse.Model.extend({
 
   // When you edit a post
   editPost: function(opts) {
-    var post = this.get('post');
-    var oldCooked = post.get('cooked');
-    var composer = this;
+    var post = this.get('post'),
+        oldCooked = post.get('cooked'),
+        composer = this;
 
     // Update the title if we've changed it
     if (this.get('title') && post.get('post_number') === 1) {
       var topic = this.get('topic');
-      topic.set('title', this.get('title'));
-      topic.set('fancy_title', this.get('title'));
-      topic.set('categoryName', this.get('categoryName'));
+      topic.setProperties({
+        title: this.get('title'),
+        fancy_title: this.get('title'),
+        categoryName: this.get('categoryName')
+      });
       topic.save();
     }
 
@@ -310,24 +300,9 @@ Discourse.Composer = Discourse.Model.extend({
 
     return Ember.Deferred.promise(function(promise) {
       post.save(function(savedPost) {
-        var posts = composer.get('topic.posts');
-
-        composer.set('originalText', composer.get('reply'));
-
-        // perhaps our post came from elsewhere eg. draft
-        var idx = -1;
-        var postNumber = post.get('post_number');
-        _.each(posts,function(p,i) {
-          if (p.get('post_number') === postNumber) {
-            idx = i;
-          }
-        });
-        if (idx > -1) {
-          savedPost.set('topic', composer.get('topic'));
-          posts.replace(idx, 1, [savedPost]);
-          promise.resolve({ post: post });
-          composer.set('topic.draft_sequence', savedPost.draft_sequence);
-        }
+        composer.set('originalText', '');
+        composer.set('reply', '');
+        composer.set('post', null);
       }, function(error) {
         var response = $.parseJSON(error.responseText);
         if (response && response.errors) {
@@ -402,8 +377,12 @@ Discourse.Composer = Discourse.Model.extend({
           saving = false;
         }
 
-        composer.set('reply', '');
-        composer.set('createdPost', createdPost);
+        composer.setProperties({
+          reply: '',
+          createdPost: createdPost,
+          title: ''
+        });
+
         if (addedToStream) {
           composer.set('composeState', CLOSED);
         } else if (saving) {

app/assets/javascripts/discourse/models/post_stream.js

@@ -24,18 +24,14 @@ Discourse.PostStream = Em.Object.extend({
 
     @property hasPosts
   **/
-  hasPosts: function() {
-    return this.get('posts.length') > 0;
-  }.property('posts.length'),
+  hasPosts: Em.computed.gt('posts.length', 0),
 
   /**
     Do we have a stream list of post ids?
 
     @property hasStream
   **/
-  hasStream: function() {
-    return this.get('filteredPostsCount') > 0;
-  }.property('filteredPostsCount'),
+  hasStream: Em.computed.gt('filteredPostsCount', 0),
 
   /**
     Can we append more posts to our current stream?
@@ -44,7 +40,6 @@ Discourse.PostStream = Em.Object.extend({
   **/
   canAppendMore: Em.computed.and('notLoading', 'hasPosts', 'lastPostNotLoaded'),
 
-
   /**
     Can we prepend more posts to our current stream?
 
@@ -59,11 +54,20 @@ Discourse.PostStream = Em.Object.extend({
   **/
   firstPostLoaded: function() {
     if (!this.get('hasLoadedData')) { return false; }
-    return !!this.get('posts').findProperty('id', this.get('stream')[0]);
-  }.property('hasLoadedData', 'posts.[]', 'stream.@each'),
+    return !!this.get('posts').findProperty('id', this.get('firstPostId'));
+  }.property('hasLoadedData', 'posts.[]', 'firstPostId'),
 
   firstPostNotLoaded: Em.computed.not('firstPostLoaded'),
 
+  /**
+    Returns the id of the first post in the set
+
+    @property firstPostId
+  **/
+  firstPostId: function() {
+    return this.get('stream')[0];
+  }.property('stream.@each'),
+
   /**
     Returns the id of the last post in the set
 

app/assets/javascripts/discourse/routes/topic_from_params_route.js

@@ -30,7 +30,9 @@ Discourse.TopicFromParamsRoute = Discourse.Route.extend({
       }
     }
 
-    var topicController = this.controllerFor('topic');
+    var topicController = this.controllerFor('topic'),
+        composerController = this.controllerFor('composer');
+
     postStream.refresh(params).then(function () {
 
       // The post we requested might not exist. Let's find the closest post
@@ -43,7 +45,7 @@ Discourse.TopicFromParamsRoute = Discourse.Route.extend({
       });
 
       if (topic.present('draft')) {
-        Discourse.openComposer({
+        composerController.open({
           draft: Discourse.Draft.getLocal(topic.get('draft_key'), topic.get('draft')),
           draftKey: topic.get('draft_key'),
           draftSequence: topic.get('draft_sequence'),

test/javascripts/models/composer_test.js

@@ -1,8 +1,6 @@
 module("Discourse.Composer");
 
-
 test('replyLength', function() {
-
   var replyLength = function(val, expectedLength, text) {
     var composer = Discourse.Composer.create({ reply: val });
     equal(composer.get('replyLength'), expectedLength);
@@ -13,10 +11,8 @@ test('replyLength', function() {
   replyLength("ba sic\n\nreply", 12, "count only significant whitespaces");
   replyLength("1[quote=]not counted[/quote]2[quote=]at all[/quote]3", 3, "removes quotes");
   replyLength("1[quote=]not[quote=]counted[/quote]yay[/quote]2", 2, "handles nested quotes correctly");
-
 });
 
-
 test('missingReplyCharacters', function() {
   var missingReplyCharacters = function(val, isPM, expected, message) {
     var composer = Discourse.Composer.create({ reply: val, creatingPrivateMessage: isPM });
@@ -36,3 +32,63 @@ test('missingTitleCharacters', function() {
   missingTitleCharacters('hi', false, Discourse.SiteSettings.min_topic_title_length - 2, 'too short post title');
   missingTitleCharacters('z', true,  Discourse.SiteSettings.min_private_message_title_length - 1, 'too short pm title');
 });
+
+
+test('wouldLoseChanges', function() {
+  var composer = Discourse.Composer.create();
+  ok(!composer.get('wouldLoseChanges'), "by default it's false");
+
+  composer.setProperties({
+    originalText: "hello",
+    reply: "hello"
+  });
+
+  ok(!composer.get('wouldLoseChanges'), "it's false when the originalText is the same as the reply");
+  composer.set('reply', 'hello world');
+  ok(composer.get('wouldLoseChanges'), "it's true when the reply changes");
+});
+
+
+
+test('importQuote with no data', function() {
+  this.stub(Discourse.Post, 'load');
+  var composer = Discourse.Composer.create();
+  composer.importQuote();
+  blank(composer.get('reply'), 'importing with no topic adds nothing');
+  ok(!Discourse.Post.load.calledOnce, "load is not called");
+
+  composer = Discourse.Composer.create({topic: Discourse.Topic.create()});
+  composer.importQuote();
+  blank(composer.get('reply'), 'importing with no posts in a topic adds nothing');
+  ok(!Discourse.Post.load.calledOnce, "load is not called");
+});
+
+asyncTest('importQuote with a post', function() {
+  expect(1);
+
+  this.stub(Discourse.Post, 'load').withArgs(123).returns(Em.Deferred.promise(function (p) {
+    p.resolve(Discourse.Post.create({raw: "let's quote"}));
+  }));
+
+  var composer = Discourse.Composer.create({post: Discourse.Post.create({id: 123})});
+  composer.importQuote().then(function () {
+    start();
+    ok(composer.get('reply').indexOf("let's quote") > -1, "it quoted the post");
+  });
+});
+
+asyncTest('importQuote with no post', function() {
+  expect(1);
+
+  this.stub(Discourse.Post, 'load').withArgs(4).returns(Em.Deferred.promise(function (p) {
+    p.resolve(Discourse.Post.create({raw: 'quote me'}));
+  }));
+
+  var composer = Discourse.Composer.create({topic: Discourse.Topic.create()});
+  composer.set('topic.postStream.stream', [4, 5]);
+  composer.importQuote().then(function () {
+    start();
+    ok(composer.get('reply').indexOf('quote me') > -1, "it contains the word quote me");
+  });
+
+});