From c61817cc802a74fc0abcf67344d75f6a94a5c713 Mon Sep 17 00:00:00 2001 From: David Arvelo Date: Fri, 27 Jun 2014 23:01:56 -0400 Subject: [PATCH] Fix tag updating on model save in editor controller closes #3131 - create a hook in the editor controller that fires on a model's save events - use this hook to perform all the things that need to happen on save, regardless of where the save originated - remove logic from instances of model.save() that now belongs in the modelSaved hook - detach the model event listeners on willTransition in the editor routes --- core/client/mixins/editor-base-controller.js | 23 ++++++++++++++------ core/client/mixins/editor-route-base.js | 20 +++++++++++++++++ core/client/routes/editor/edit.js | 6 +++++ core/client/routes/editor/new.js | 6 +++++ 4 files changed, 48 insertions(+), 7 deletions(-) diff --git a/core/client/mixins/editor-base-controller.js b/core/client/mixins/editor-base-controller.js index a75438a1f8..dd4ee9bb85 100644 --- a/core/client/mixins/editor-base-controller.js +++ b/core/client/mixins/editor-base-controller.js @@ -63,6 +63,22 @@ var EditorControllerMixin = Ember.Mixin.create(MarkerManager, { return hashCurrent === hashPrevious; }, + // a hook created in editor-route-base's setupController + modelSaved: function () { + var model = this.get('model'); + + // safer to updateTags on save in one place + // rather than in all other places save is called + model.updateTags(); + + // set previousTagNames to current tagNames for isDirty check + this.set('previousTagNames', this.get('tagNames')); + + // `updateTags` triggers `isDirty => true`. + // for a saved model it would otherwise be false. + this.set('isDirty', false); + }, + // an ugly hack, but necessary to watch all the model's properties // and more, without having to be explicit and do it manually isDirty: Ember.computed.apply(Ember, watchedProps.concat(function (key, value) { @@ -76,7 +92,6 @@ var EditorControllerMixin = Ember.Mixin.create(MarkerManager, { changedAttributes; if (!this.tagNamesEqual()) { - this.set('previousTagNames', this.get('tagNames')); return true; } @@ -183,13 +198,7 @@ var EditorControllerMixin = Ember.Mixin.create(MarkerManager, { this.set('status', status); return this.get('model').save().then(function (model) { - model.updateTags(); - // `updateTags` triggers `isDirty => true`. - // for a saved model it would otherwise be false. - self.set('isDirty', false); - self.showSaveNotification(prevStatus, model.get('status'), isNew ? true : false); - return model; }).catch(function (errors) { self.showErrorNotification(prevStatus, self.get('status'), errors); diff --git a/core/client/mixins/editor-route-base.js b/core/client/mixins/editor-route-base.js index dc12ce1d86..17a66f4325 100644 --- a/core/client/mixins/editor-route-base.js +++ b/core/client/mixins/editor-route-base.js @@ -22,6 +22,26 @@ var EditorRouteBase = Ember.Mixin.create(styleBody, ShortcutsRoute, loadingIndic } }, + attachModelHooks: function (controller, model) { + // this will allow us to track when the model is saved and update the controller + // so that we can be sure controller.isDirty is correct, without having to update the + // controller on each instance of `model.save()`. + // + // another reason we can't do this on `model.save().then()` is because the post-settings-menu + // also saves the model, and passing messages is difficult because we have two + // types of editor controllers, and the PSM also exists on the posts.post route. + // + // The reason we can't just keep this functionality in the editor controller is + // because we need to remove these handlers on `willTransition` in the editor route. + model.on('didCreate', controller, controller.get('modelSaved')); + model.on('didUpdate', controller, controller.get('modelSaved')); + }, + + detachModelHooks: function (controller, model) { + model.off('didCreate', controller, controller.get('modelSaved')); + model.off('didUpdate', controller, controller.get('modelSaved')); + }, + shortcuts: { //General Editor shortcuts 'ctrl+s, command+s': 'save', diff --git a/core/client/routes/editor/edit.js b/core/client/routes/editor/edit.js index e069aaa992..3aed57c71e 100644 --- a/core/client/routes/editor/edit.js +++ b/core/client/routes/editor/edit.js @@ -46,6 +46,9 @@ var EditorEditRoute = AuthenticatedRoute.extend(base, { controller.set('scratch', model.get('markdown')); // used to check if anything has changed in the editor controller.set('previousTagNames', model.get('tags').mapBy('name')); + + // attach model-related listeners created in editor-route-base + this.attachModelHooks(controller, model); }, actions: { @@ -73,6 +76,9 @@ var EditorEditRoute = AuthenticatedRoute.extend(base, { // since the transition is now certain to complete.. window.onbeforeunload = null; + + // remove model-related listeners created in editor-route-base + this.detachModelHooks(controller, model); } } }); diff --git a/core/client/routes/editor/new.js b/core/client/routes/editor/new.js index 04413f337c..ea3dab42aa 100644 --- a/core/client/routes/editor/new.js +++ b/core/client/routes/editor/new.js @@ -14,6 +14,9 @@ var EditorNewRoute = AuthenticatedRoute.extend(base, { // used to check if anything has changed in the editor controller.set('previousTagNames', Ember.A()); + + // attach model-related listeners created in editor-route-base + this.attachModelHooks(controller, model); }, actions: { @@ -46,6 +49,9 @@ var EditorNewRoute = AuthenticatedRoute.extend(base, { // since the transition is now certain to complete.. window.onbeforeunload = null; + + // remove model-related listeners created in editor-route-base + this.detachModelHooks(controller, model); } } });