FIX: editing a post wasn't showing error messages from the server

app/assets/javascripts/discourse/controllers/composer.js.es6

@@ -211,13 +211,13 @@ export default DiscourseController.extend({
       }
     }
 
-    var staged = false,
-        disableJumpReply = Discourse.User.currentProp('disable_jump_reply');
-    var promise = composer.save({
+    var staged = false;
+    const disableJumpReply = Discourse.User.currentProp('disable_jump_reply');
+
+    const promise = composer.save({
       imageSizes: this.get('view').imageSizes(),
       editReason: this.get("editReason")
     }).then(function(opts) {
-
       // If we replied as a new topic successfully, remove the draft.
       if (self.get('replyAsNewTopicDraft')) {
         self.destroyDraft();
@@ -240,21 +240,21 @@ export default DiscourseController.extend({
           Discourse.URL.routeTo(opts.post.get('url'));
         }
       }
-    }, function(error) {
+    }).catch(function(error) {
       composer.set('disableDrafts', false);
       bootbox.alert(error);
     });
 
 
-    if ( this.get('controllers.application.currentRouteName').split('.')[0] === 'topic' &&
-         composer.get('topic.id') === this.get('controllers.topic.model.id') ) {
+    if (this.get('controllers.application.currentRouteName').split('.')[0] === 'topic' &&
+        composer.get('topic.id') === this.get('controllers.topic.model.id')) {
       staged = composer.get('stagedPost');
     }
 
     Em.run.schedule('afterRender', function() {
       if (staged && !disableJumpReply) {
-        var postNumber = staged.get('post_number');
-        Discourse.URL.jumpToPost(postNumber, {skipIfOnScreen: true});
+        const postNumber = staged.get('post_number');
+        Discourse.URL.jumpToPost(postNumber, { skipIfOnScreen: true });
         self.appEvents.trigger('post:highlight', postNumber);
       }
     });

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

@@ -123,7 +123,6 @@ Discourse.Post = Discourse.Model.extend({
   save: function(complete, error) {
     var self = this;
     if (!this.get('newPost')) {
-
       // We're updating a post
       return Discourse.ajax("/posts/" + (this.get('id')), {
         type: 'PUT',
@@ -137,13 +136,12 @@ Discourse.Post = Discourse.Model.extend({
         self.set('version', result.post.version);
         if (result.category) Discourse.Site.current().updateCategory(result.category);
         if (complete) complete(Discourse.Post.create(result.post));
-      }, function(result) {
+      }).catch(function(result) {
         // Post failed to update
         if (error) error(result);
       });
 
     } else {
-
       // We're saving a post
       var data = this.getProperties(Discourse.Composer.serializedFieldsForCreate());
       data.reply_to_post_number = this.get('reply_to_post_number');
@@ -162,7 +160,7 @@ Discourse.Post = Discourse.Model.extend({
       }).then(function(result) {
         // Post created
         if (complete) complete(Discourse.Post.create(result));
-      }, function(result) {
+      }).catch(function(result) {
         // Failed to create a post
         if (error) error(result);
       });

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

@@ -385,7 +385,7 @@ const Composer = Discourse.Model.extend({
   },
 
   save(opts) {
-    if( !this.get('cantSubmitPost') ) {
+    if (!this.get('cantSubmitPost')) {
       return this.get('editingPost') ? this.editPost(opts) : this.createPost(opts);
     }
   },
@@ -409,8 +409,9 @@ const Composer = Discourse.Model.extend({
   // When you edit a post
   editPost(opts) {
     const post = this.get('post'),
-        oldCooked = post.get('cooked'),
-        self = this;
+          oldCooked = post.get('cooked'),
+          self = this;
+
     let promise;
 
     // Update the title if we've changed it, otherwise consider it a
@@ -418,7 +419,6 @@ const Composer = Discourse.Model.extend({
     if (this.get('title') &&
         post.get('post_number') === 1 &&
         this.get('topic.details.can_edit')) {
-
       const topicProps = this.getProperties(Object.keys(_edit_topic_serializer));
       promise = Discourse.Topic.update(this.get('topic'), topicProps);
     } else {
@@ -431,33 +431,26 @@ const Composer = Discourse.Model.extend({
       imageSizes: opts.imageSizes,
       cooked: this.getCookedHtml()
     });
+
     this.set('composeState', CLOSED);
 
     return promise.then(function() {
       return post.save(function(result) {
         post.updateFromPost(result);
         self.clearState();
-      }).catch(function(error) {
-        const response = $.parseJSON(error.responseText);
-        if (response && response.errors) {
-          return(response.errors[0]);
-        } else {
-          return(I18n.t('generic_error'));
-        }
+      }, function (error) {
         post.set('cooked', oldCooked);
         self.set('composeState', OPEN);
+        const response = $.parseJSON(error.responseText);
+        throw response && response.errors ? response.errors[0] : I18n.t('generic_error');
       });
     });
   },
 
   serialize(serializer, dest) {
-    if (!dest) {
-      dest = {};
-    }
-
-    const self = this;
-    Object.keys(serializer).forEach(function(f) {
-      const val = self.get(serializer[f]);
+    dest = dest || {};
+    Object.keys(serializer).forEach(f => {
+      const val = this.get(serializer[f]);
       if (typeof val !== 'undefined') {
         Ember.set(dest, f, val);
       }
@@ -468,9 +461,10 @@ const Composer = Discourse.Model.extend({
   // Create a new Post
   createPost(opts) {
     const post = this.get('post'),
-        topic = this.get('topic'),
-        currentUser = Discourse.User.current(),
-        postStream = this.get('topic.postStream');
+          topic = this.get('topic'),
+          currentUser = Discourse.User.current(),
+          postStream = this.get('topic.postStream');
+
     let addedToStream = false;
 
     // Build the post object
@@ -530,10 +524,10 @@ const Composer = Discourse.Model.extend({
       }
     }
 
-    const composer = this;
-    const promise =  new Ember.RSVP.Promise(function(resolve, reject) {
-
+    const composer = this,
+          promise = new Ember.RSVP.Promise(function(resolve, reject) {
       composer.set('composeState', SAVING);
+
       createdPost.save(function(result) {
         let saving = true;
 

app/controllers/application_controller.rb

@@ -316,9 +316,7 @@ class ApplicationController < ActionController::Base
     #   type   - a machine-readable description of the error
     #   status - HTTP status code to return
     def render_json_error(obj, opts={})
-      if opts.is_a? Fixnum
-        opts = {status: opts}
-      end
+      opts = { status: opts } if opts.is_a?(Fixnum)
       render json: MultiJson.dump(create_errors_json(obj, opts[:type])), status: opts[:status] || 422
     end
 

lib/json_error.rb

@@ -9,18 +9,17 @@ module JsonError
   private
 
   def create_errors_array(obj)
-
     # If we're passed a string, assume that is the error message
-    return {errors: [obj]} if obj.is_a?(String)
+    return { errors: [obj] } if obj.is_a?(String)
 
     # If it's an AR exception target the record
     obj = obj.record if obj.is_a?(ActiveRecord::RecordInvalid)
 
     # If it looks like an activerecord object, extract its messages
-    return {errors: obj.errors.full_messages } if obj.respond_to?(:errors) && obj.errors.present?
+    return { errors: obj.errors.full_messages } if obj.respond_to?(:errors) && obj.errors.present?
 
     # If we're passed an array, it's an array of error messages
-    return {errors: obj.map {|e| e.to_s}} if obj.is_a?(Array) && obj.present?
+    return { errors: obj.map(&:to_s) } if obj.is_a?(Array) && obj.present?
 
     # Log a warning (unless obj is nil)
     Rails.logger.warn("create_errors_json called with unrecognized type: #{obj.inspect}") if obj
@@ -30,7 +29,7 @@ module JsonError
   end
 
   def self.generic_error
-    {errors: [I18n.t('js.generic_error')]}
+    { errors: [I18n.t('js.generic_error')] }
   end
 
 end

test/javascripts/integration/about-test.js.es6

@@ -1,8 +1,8 @@
 integration("About");
 
-test("viewing", function() {
+test("viewing", () => {
   visit("/about");
-  andThen(function() {
+  andThen(() => {
     ok(exists('.about.admins .user-small'), 'has admins');
     ok(exists('.about.moderators .user-small'), 'has moderators');
     ok(exists('.about.stats tr td'), 'has stats');

test/javascripts/integration/badges-test.js.es6

@@ -1,13 +1,13 @@
 integration("Badges");
 
-test("Visit Badge Pages", function() {
+test("Visit Badge Pages", () => {
   visit("/badges");
-  andThen(function() {
+  andThen(() => {
     ok(exists('.badges-listing tr'), "has a list of badges");
   });
 
   visit("/badges/9/autobiographer");
-  andThen(function() {
+  andThen(() => {
     ok(exists('.badges-listing tr'), "has the badge in the listing");
     ok(exists('.badge-user'), "has the list of users with that badge");
   });

test/javascripts/integration/create-account-user-fields-test.js.es6

@@ -8,11 +8,11 @@ integration("Create Account - User Fields", {
   }
 });
 
-test("create account with user fields", function() {
+test("create account with user fields", () => {
   visit("/");
   click("header .sign-up-button");
 
-  andThen(function() {
+  andThen(() => {
     ok(exists('.create-account'), "it shows the create account modal");
     ok(exists('.user-field'), "it has at least one user field");
     ok(exists('.modal-footer .btn-primary:disabled'), 'create account is disabled at first');
@@ -23,24 +23,24 @@ test("create account with user fields", function() {
   fillIn('#new-account-email', 'good.tuna@test.com');
   fillIn('#new-account-username', 'goodtuna');
 
-  andThen(function() {
+  andThen(() => {
     ok(exists('#username-validation.good'), 'the username validation is good');
     ok(exists('.modal-footer .btn-primary:disabled'), 'create account is still disabled due to lack of user fields');
   });
 
   fillIn(".user-field input[type=text]:first", "Barky");
 
-  andThen(function() {
+  andThen(() => {
     ok(exists('.modal-footer .btn-primary:disabled'), 'create account is disabled because field is not checked');
   });
 
   click(".user-field input[type=checkbox]");
-  andThen(function() {
+  andThen(() => {
     not(exists('.modal-footer .btn-primary:disabled'), 'create account is enabled because field is not checked');
   });
 
   click(".user-field input[type=checkbox]");
-  andThen(function() {
+  andThen(() => {
     ok(exists('.modal-footer .btn-primary:disabled'), 'unclicking the checkbox disables the submit');
   });
 

test/javascripts/integration/directory-test.js.es6

@@ -1,8 +1,8 @@
 integration("User Directory");
 
-test("Visit Page", function() {
+test("Visit Page", () => {
   visit("/directory/all");
-  andThen(function() {
+  andThen(() => {
     ok(exists('.directory table tr'), "has a list of users");
   });
 });

test/javascripts/integration/groups-test.js.es6

@@ -1,12 +1,13 @@
 integration("Groups");
 
-test("Browsing Groups", function() {
+test("Browsing Groups", () => {
   visit("/groups/discourse");
-  andThen(function() {
+  andThen(() => {
     ok(count('.user-stream .item') > 0, "it has stream items");
   });
+
   visit("/groups/discourse/members");
-  andThen(function() {
+  andThen(() => {
     ok(count('.group-members tr') > 0, "it lists group members");
   });
 });

test/javascripts/integration/header-anonymous-test.js.es6

@@ -1,8 +1,8 @@
 integration("Header (Anonymous)");
 
-test("header", function() {
+test("header", () => {
   visit("/");
-  andThen(function() {
+  andThen(() => {
     ok(exists("header"), "is rendered");
     ok(exists(".logo-big"), "it renders the large logo by default");
     not(exists("#notifications-dropdown li"), "no notifications at first");
@@ -12,17 +12,17 @@ test("header", function() {
   });
 
   // Logo changing
-  andThen(function() {
+  andThen(() => {
     controllerFor('header').set("showExtraInfo", true);
   });
 
-  andThen(function() {
+  andThen(() => {
     ok(exists(".logo-small"), "it shows the small logo when `showExtraInfo` is enabled");
   });
 
   // Site Map
   click("#site-map");
-  andThen(function() {
+  andThen(() => {
     ok(exists('#site-map-dropdown'), "is rendered after user opens it");
     ok(exists("#site-map-dropdown .faq-link"), "it shows the faq link");
     ok(exists("#site-map-dropdown .category-links"), "has categories correctly bound");
@@ -30,14 +30,14 @@ test("header", function() {
 
   // Search
   click("#search-button");
-  andThen(function() {
+  andThen(() => {
     ok(exists("#search-dropdown:visible"), "after clicking a button search box opens");
     not(exists("#search-dropdown .heading"), "initially, immediately after opening, search box is empty");
   });
 
   // Perform Search
   fillIn("#search-term", "hello");
-  andThen(function() {
+  andThen(() => {
     ok(exists("#search-dropdown .heading"), "when user completes a search, search box shows search results");
     equal(find("#search-dropdown .results a:first").attr("href"), "/t/hello-bar-integration-issues/17638", "there is a search result");
   });

test/javascripts/integration/header-test-staff.js.es6

@@ -4,12 +4,12 @@ integration("Header (Staff)", {
           site_flagged_posts_count: 1 }
 });
 
-test("header", function() {
+test("header", () => {
   visit("/");
 
   // Notifications
   click("#user-notifications");
-  andThen(function() {
+  andThen(() => {
     var $items = $("#notifications-dropdown li");
     ok(exists($items), "is lazily populated after user opens it");
     ok($items.first().hasClass("read"), "correctly binds items' 'read' class");
@@ -17,14 +17,14 @@ test("header", function() {
 
   // Site Map
   click("#site-map");
-  andThen(function() {
+  andThen(() => {
     ok(exists("#site-map-dropdown .admin-link"), "it has the admin link");
     ok(exists("#site-map-dropdown .flagged-posts.badge-notification"), "it displays flag notifications");
   });
 
   // User dropdown
   click("#current-user");
-  andThen(function() {
+  andThen(() => {
     ok(exists("#user-dropdown:visible"), "is lazily rendered after user opens it");
     ok(exists("#user-dropdown .user-dropdown-links"), "has showing / hiding user-dropdown links correctly bound");
   });

test/javascripts/integration/login-required-test.js.es6

@@ -4,39 +4,39 @@ integration("Login Required", {
   }
 });
 
-test("redirect", function() {
+test("redirect", () => {
   visit('/latest');
-  andThen(function() {
+  andThen(() => {
     equal(currentPath(), "login", "it redirects them to login");
   });
 
   click('#site-logo');
-  andThen(function() {
+  andThen(() => {
     equal(currentPath(), "login", "clicking the logo keeps them on login");
   });
 
   click('header .login-button');
-  andThen(function() {
+  andThen(() => {
     ok(exists('.login-modal'), "they can still access the login modal");
   });
 
   click('.modal-header .close');
-  andThen(function() {
+  andThen(() => {
     ok(!exists('.login-modal'), "it closes the login modal");
   });
 
   click('#search-button');
-  andThen(function() {
+  andThen(() => {
     ok(exists('.login-modal'), "clicking search opens the login modal");
   });
 
   click('.modal-header .close');
-  andThen(function() {
+  andThen(() => {
     ok(!exists('.login-modal'), "it closes the login modal");
   });
 
   click('#site-map');
-  andThen(function() {
+  andThen(() => {
     ok(exists('.login-modal'), "site map opens the login modal");
   });
 });

test/javascripts/integration/modal-test.js.es6

@@ -1,29 +1,29 @@
 integration("Modal");
 
-test("modal", function() {
+test("modal", () => {
   visit('/');
 
-  andThen(function() {
+  andThen(() => {
     ok(find('#discourse-modal:visible').length === 0, 'there is no modal at first');
   });
 
   click('.login-button');
-  andThen(function() {
+  andThen(() => {
     ok(find('#discourse-modal:visible').length === 1, 'modal should appear');
   });
 
   click('.modal-outer-container');
-  andThen(function() {
+  andThen(() => {
     ok(find('#discourse-modal:visible').length === 0, 'modal should disappear when you click outside');
   });
 
   click('.login-button');
-  andThen(function() {
+  andThen(() => {
     ok(find('#discourse-modal:visible').length === 1, 'modal should reappear');
   });
 
   keyEvent('#main-outlet', 'keyup', 27);
-  andThen(function() {
+  andThen(() => {
     ok(find('#discourse-modal:visible').length === 0, 'ESC should close the modal');
   });
 });

test/javascripts/integration/sign-in-test.js.es6

@@ -1,9 +1,9 @@
 integration("Signing In");
 
-test("sign in", function() {
+test("sign in", () => {
   visit("/");
   click("header .login-button");
-  andThen(function() {
+  andThen(() => {
     ok(exists('.login-modal'), "it shows the login modal");
   });
 
@@ -11,7 +11,7 @@ test("sign in", function() {
   fillIn('#login-account-name', 'eviltrout');
   fillIn('#login-account-password', 'incorrect');
   click('.modal-footer .btn-primary');
-  andThen(function() {
+  andThen(() => {
     ok(exists('#modal-alert:visible', 'it displays the login error'));
     not(exists('.modal-footer .btn-primary:disabled'), "enables the login button");
   });
@@ -19,16 +19,16 @@ test("sign in", function() {
   // Use the correct password
   fillIn('#login-account-password', 'correct');
   click('.modal-footer .btn-primary');
-  andThen(function() {
+  andThen(() => {
     ok(exists('.modal-footer .btn-primary:disabled'), "disables the login button");
   });
 });
 
-test("create account", function() {
+test("create account", () => {
   visit("/");
   click("header .sign-up-button");
 
-  andThen(function() {
+  andThen(() => {
     ok(exists('.create-account'), "it shows the create account modal");
     ok(exists('.modal-footer .btn-primary:disabled'), 'create account is disabled at first');
   });
@@ -39,19 +39,19 @@ test("create account", function() {
   // Check username
   fillIn('#new-account-email', 'good.tuna@test.com');
   fillIn('#new-account-username', 'taken');
-  andThen(function() {
+  andThen(() => {
     ok(exists('#username-validation.bad'), 'the username validation is bad');
     ok(exists('.modal-footer .btn-primary:disabled'), 'create account is still disabled');
   });
 
   fillIn('#new-account-username', 'goodtuna');
-  andThen(function() {
+  andThen(() => {
     ok(exists('#username-validation.good'), 'the username validation is good');
     not(exists('.modal-footer .btn-primary:disabled'), 'create account is enabled');
   });
 
   click('.modal-footer .btn-primary');
-  andThen(function() {
+  andThen(() => {
     ok(exists('.modal-footer .btn-primary:disabled'), "create account is disabled");
   });
 

test/javascripts/integration/static-test.js.es6

@@ -1,28 +1,28 @@
 integration("Static");
 
-test("Static Pages", function() {
+test("Static Pages", () => {
   visit("/faq");
-  andThen(function() {
+  andThen(() => {
     ok(exists(".body-page"), "The content is present");
   });
 
   visit("/guidelines");
-  andThen(function() {
+  andThen(() => {
     ok(exists(".body-page"), "The content is present");
   });
 
   visit("/tos");
-  andThen(function() {
+  andThen(() => {
     ok(exists(".body-page"), "The content is present");
   });
 
   visit("/privacy");
-  andThen(function() {
+  andThen(() => {
     ok(exists(".body-page"), "The content is present");
   });
 
   visit("/login");
-  andThen(function() {
+  andThen(() => {
     equal(currentPath(), "discovery.latest", "it redirects them to latest unless `login_required`");
   });
 });

test/javascripts/integration/topic-discovery-test.js.es6

@@ -1,21 +1,21 @@
 integration("Topic Discovery");
 
-test("Visit Discovery Pages", function() {
+test("Visit Discovery Pages", () => {
   visit("/");
-  andThen(function() {
+  andThen(() => {
     ok(exists(".topic-list"), "The list of topics was rendered");
     ok(exists('.topic-list .topic-list-item'), "has topics");
   });
 
   visit("/c/bug");
-  andThen(function() {
+  andThen(() => {
     ok(exists(".topic-list"), "The list of topics was rendered");
     ok(exists('.topic-list .topic-list-item'), "has topics");
     ok($('body.category-bug').length, "has a custom css class for the category id on the body");
   });
 
   visit("/categories");
-  andThen(function() {
+  andThen(() => {
     ok($('body.category-bug').length === 0, "removes the custom category class");
 
     ok(exists('.category'), "has a list of categories");
@@ -23,7 +23,7 @@ test("Visit Discovery Pages", function() {
   });
 
   visit("/top");
-  andThen(function() {
+  andThen(() => {
     ok($('body.categories-list').length === 0, "removes the `categories-list` class");
     ok(exists('.topic-list .topic-list-item'), "has topics");
   });

test/javascripts/integration/topic-test.js.es6

@@ -1,16 +1,16 @@
 integration("View Topic");
 
-test("Enter a Topic", function() {
+test("Enter a Topic", () => {
   visit("/t/internationalization-localization/280");
-  andThen(function() {
+  andThen(() => {
     ok(exists("#topic"), "The topic was rendered");
     ok(exists("#topic .post-cloak"), "The topic has cloaked posts");
   });
 });
 
-test("Enter without an id", function() {
+test("Enter without an id", () => {
   visit("/t/internationalization-localization");
-  andThen(function() {
+  andThen(() => {
     ok(exists("#topic"), "The topic was rendered");
   });
 });

test/javascripts/integration/unknown-test.js.es6

@@ -1,9 +1,9 @@
 integration("Unknown");
 
-test("Unknown URL", function() {
+test("Unknown URL", () => {
   expect(1);
   visit("/url-that-doesn't-exist");
-  andThen(function() {
+  andThen(() => {
     ok(exists(".page-not-found"), "The not found content is present");
   });
 });

test/javascripts/integration/user-card-test.js.es6

@@ -1,12 +1,12 @@
 integration("User Card");
 
-test("card", function() {
+test("card", () => {
   visit('/');
 
   ok(invisible('#user-card'), 'user card is invisible by default');
   click('a[data-user-card=eviltrout]:first');
 
-  andThen(function() {
+  andThen(() => {
     ok(visible('#user-card'), 'card should appear');
   });
 

test/javascripts/integration/user-test.js.es6

@@ -1,34 +1,40 @@
 integration("User");
 
 function hasStream() {
-  andThen(function() {
+  andThen(() => {
     ok(exists('.user-main .about'), 'it has the about section');
     ok(count('.user-stream .item') > 0, 'it has stream items');
   });
 }
 
 function hasTopicList() {
-  andThen(function() {
+  andThen(() => {
     equal(count('.user-stream .item'), 0, "has no stream displayed");
     ok(count('.topic-list tr') > 0, 'it has a topic list');
   });
 }
 
-test("Filters", function() {
+test("Filters", () => {
   expect(14);
 
   visit("/users/eviltrout");
   hasStream();
+
   visit("/users/eviltrout/activity/topics");
   hasTopicList();
+
   visit("/users/eviltrout/activity/posts");
   hasStream();
+
   visit("/users/eviltrout/activity/replies");
   hasStream();
+
   visit("/users/eviltrout/activity/likes-given");
   hasStream();
+
   visit("/users/eviltrout/activity/likes-received");
   hasStream();
+
   visit("/users/eviltrout/activity/edits");
   hasStream();
 });