From d9e5c2317fa84a1400e7603d9503d281ecde4b8b Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 21 Jul 2014 13:39:23 -0400 Subject: [PATCH] FIX: If a topic title edit fails, revert to previous title. --- .../javascripts/admin/models/admin_user.js | 4 +- app/assets/javascripts/admin/models/report.js | 2 +- .../discourse/controllers/topic_controller.js | 13 ++----- .../javascripts/discourse/models/model.js | 39 +++++-------------- .../javascripts/discourse/models/site.js | 2 +- test/javascripts/models/model_test.js | 28 ------------- 6 files changed, 17 insertions(+), 71 deletions(-) diff --git a/app/assets/javascripts/admin/models/admin_user.js b/app/assets/javascripts/admin/models/admin_user.js index 62a8fd73c5a..e6214290103 100644 --- a/app/assets/javascripts/admin/models/admin_user.js +++ b/app/assets/javascripts/admin/models/admin_user.js @@ -313,11 +313,11 @@ Discourse.AdminUser = Discourse.User.extend({ } else { bootbox.alert(I18n.t("admin.user.delete_failed")); if (data.user) { - user.mergeAttributes(data.user); + user.setProperties(data.user); } } }, function() { - Discourse.AdminUser.find( user.get('username') ).then(function(u){ user.mergeAttributes(u); }); + Discourse.AdminUser.find( user.get('username') ).then(function(u){ user.setProperties(u); }); bootbox.alert(I18n.t("admin.user.delete_failed")); }); }; diff --git a/app/assets/javascripts/admin/models/report.js b/app/assets/javascripts/admin/models/report.js index 76ddf066afe..940f8b0e714 100644 --- a/app/assets/javascripts/admin/models/report.js +++ b/app/assets/javascripts/admin/models/report.js @@ -153,7 +153,7 @@ Discourse.Report.reopenClass({ row.percentage = Math.round((row.y / maxY) * 100); }); } - model.mergeAttributes(json.report); + model.setProperties(json.report); model.set('loaded', true); }); return(model); diff --git a/app/assets/javascripts/discourse/controllers/topic_controller.js b/app/assets/javascripts/discourse/controllers/topic_controller.js index 17fd59b5ad6..bb6600545f8 100644 --- a/app/assets/javascripts/discourse/controllers/topic_controller.js +++ b/app/assets/javascripts/discourse/controllers/topic_controller.js @@ -214,7 +214,7 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected this.set('topicSaving', true); // manually update the titles & category - topic.setProperties({ + var backup = topic.setPropertiesBackup({ title: this.get('newTitle'), category_id: parseInt(this.get('newCategoryId'), 10), fancy_title: this.get('newTitle') @@ -224,16 +224,11 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected var self = this; topic.save().then(function(result){ // update the title if it has been changed (cleaned up) server-side - var title = result.basic_topic.title; - var fancy_title = result.basic_topic.fancy_title; - topic.setProperties({ - title: title, - fancy_title: fancy_title - }); + topic.setProperties(Em.getProperties(result.basic_topic, 'title', 'fancy_title')); self.set('topicSaving', false); }, function(error) { - self.set('editingTopic', true); - self.set('topicSaving', false); + self.setProperties({ editingTopic: true, topicSaving: false }); + topic.setProperties(backup); if (error && error.responseText) { bootbox.alert($.parseJSON(error.responseText).errors[0]); } else { diff --git a/app/assets/javascripts/discourse/models/model.js b/app/assets/javascripts/discourse/models/model.js index ea1bcfb1820..084c7c72ad8 100644 --- a/app/assets/javascripts/discourse/models/model.js +++ b/app/assets/javascripts/discourse/models/model.js @@ -1,40 +1,19 @@ -/** - A base object we can use to handle models in the Discourse client application. - - @class Model - @extends Ember.Object - @uses Discourse.Presence - @namespace Discourse - @module Discourse -**/ Discourse.Model = Ember.Object.extend(Discourse.Presence, { - - /** - Update our object from another object - - @method mergeAttributes - @param {Object} attrs The attributes we want to merge with - **/ - mergeAttributes: function(attrs) { - var self = this; - _.each(attrs, function(v, k) { - self.set(k, v); - }); + // Like `setProperties` but returns the original values in case + // we want to roll back + setPropertiesBackup: function(obj) { + var backup = this.getProperties(Ember.keys(obj)); + this.setProperties(obj); + return backup; } }); Discourse.Model.reopenClass({ - - /** - Given an array of values, return them in a hash - - @method extractByKey - @param {Object} collection The collection of values - @param {Object} klass The class to instantiate - **/ extractByKey: function(collection, klass) { var retval = {}; - _.each(collection, function(item) { + if (Ember.isEmpty(collection)) { return retval; } + + collection.forEach(function(item) { retval[item.id] = klass.create(item); }); return retval; diff --git a/app/assets/javascripts/discourse/models/site.js b/app/assets/javascripts/discourse/models/site.js index 1b3d284f80d..4ff7907dd77 100644 --- a/app/assets/javascripts/discourse/models/site.js +++ b/app/assets/javascripts/discourse/models/site.js @@ -64,7 +64,7 @@ Discourse.Site = Discourse.Model.extend({ updateCategory: function(newCategory) { var existingCategory = this.get('categories').findProperty('id', Em.get(newCategory, 'id')); - if (existingCategory) existingCategory.mergeAttributes(newCategory); + if (existingCategory) existingCategory.setProperties(newCategory); } }); diff --git a/test/javascripts/models/model_test.js b/test/javascripts/models/model_test.js index 6d7f26919a5..6404fe88a74 100644 --- a/test/javascripts/models/model_test.js +++ b/test/javascripts/models/model_test.js @@ -4,34 +4,6 @@ test("mixes in Discourse.Presence", function() { ok(Discourse.Presence.detect(Discourse.Model.create())); }); -test("mergeAttributes: merges attributes from another object", function() { - var model = Discourse.Model.create({ - foo: "foo", - bar: "original bar" - }); - - model.mergeAttributes({ - bar: "merged bar", - baz: "baz" - }); - - equal(model.get("foo"), "foo", "leaving original attr intact when only original object contains given key"); - equal(model.get("bar"), "merged bar", "overwriting original attr when both objects contain given key"); - equal(model.get("baz"), "baz", "adding new attr to original object when only merged object contains given key"); -}); - -test("mergeAttributes: respects Ember setters (so observers etc. work)", function() { - var observerHasFired = false; - - var model = Discourse.Model.create({foo: "original foo"}); - model.addObserver("foo", function() { - observerHasFired = true; - }); - model.mergeAttributes({foo: "merged foo"}); - - ok(observerHasFired); -}); - test("extractByKey: converts a list of hashes into a hash of instances of specified class, indexed by their ids", function() { var firstObject = {id: "id_1", foo: "foo_1"}; var secondObject = {id: "id_2", foo: "foo_2"};