Skip to content

Commit c61817c

Browse files
committed
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
1 parent 2d64372 commit c61817c

File tree

4 files changed

+48
-7
lines changed

4 files changed

+48
-7
lines changed

core/client/mixins/editor-base-controller.js

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,22 @@ var EditorControllerMixin = Ember.Mixin.create(MarkerManager, {
6363
return hashCurrent === hashPrevious;
6464
},
6565

66+
// a hook created in editor-route-base's setupController
67+
modelSaved: function () {
68+
var model = this.get('model');
69+
70+
// safer to updateTags on save in one place
71+
// rather than in all other places save is called
72+
model.updateTags();
73+
74+
// set previousTagNames to current tagNames for isDirty check
75+
this.set('previousTagNames', this.get('tagNames'));
76+
77+
// `updateTags` triggers `isDirty => true`.
78+
// for a saved model it would otherwise be false.
79+
this.set('isDirty', false);
80+
},
81+
6682
// an ugly hack, but necessary to watch all the model's properties
6783
// and more, without having to be explicit and do it manually
6884
isDirty: Ember.computed.apply(Ember, watchedProps.concat(function (key, value) {
@@ -76,7 +92,6 @@ var EditorControllerMixin = Ember.Mixin.create(MarkerManager, {
7692
changedAttributes;
7793

7894
if (!this.tagNamesEqual()) {
79-
this.set('previousTagNames', this.get('tagNames'));
8095
return true;
8196
}
8297

@@ -183,13 +198,7 @@ var EditorControllerMixin = Ember.Mixin.create(MarkerManager, {
183198
this.set('status', status);
184199

185200
return this.get('model').save().then(function (model) {
186-
model.updateTags();
187-
// `updateTags` triggers `isDirty => true`.
188-
// for a saved model it would otherwise be false.
189-
self.set('isDirty', false);
190-
191201
self.showSaveNotification(prevStatus, model.get('status'), isNew ? true : false);
192-
193202
return model;
194203
}).catch(function (errors) {
195204
self.showErrorNotification(prevStatus, self.get('status'), errors);

core/client/mixins/editor-route-base.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,26 @@ var EditorRouteBase = Ember.Mixin.create(styleBody, ShortcutsRoute, loadingIndic
2222
}
2323
},
2424

25+
attachModelHooks: function (controller, model) {
26+
// this will allow us to track when the model is saved and update the controller
27+
// so that we can be sure controller.isDirty is correct, without having to update the
28+
// controller on each instance of `model.save()`.
29+
//
30+
// another reason we can't do this on `model.save().then()` is because the post-settings-menu
31+
// also saves the model, and passing messages is difficult because we have two
32+
// types of editor controllers, and the PSM also exists on the posts.post route.
33+
//
34+
// The reason we can't just keep this functionality in the editor controller is
35+
// because we need to remove these handlers on `willTransition` in the editor route.
36+
model.on('didCreate', controller, controller.get('modelSaved'));
37+
model.on('didUpdate', controller, controller.get('modelSaved'));
38+
},
39+
40+
detachModelHooks: function (controller, model) {
41+
model.off('didCreate', controller, controller.get('modelSaved'));
42+
model.off('didUpdate', controller, controller.get('modelSaved'));
43+
},
44+
2545
shortcuts: {
2646
//General Editor shortcuts
2747
'ctrl+s, command+s': 'save',

core/client/routes/editor/edit.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ var EditorEditRoute = AuthenticatedRoute.extend(base, {
4646
controller.set('scratch', model.get('markdown'));
4747
// used to check if anything has changed in the editor
4848
controller.set('previousTagNames', model.get('tags').mapBy('name'));
49+
50+
// attach model-related listeners created in editor-route-base
51+
this.attachModelHooks(controller, model);
4952
},
5053

5154
actions: {
@@ -73,6 +76,9 @@ var EditorEditRoute = AuthenticatedRoute.extend(base, {
7376

7477
// since the transition is now certain to complete..
7578
window.onbeforeunload = null;
79+
80+
// remove model-related listeners created in editor-route-base
81+
this.detachModelHooks(controller, model);
7682
}
7783
}
7884
});

core/client/routes/editor/new.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ var EditorNewRoute = AuthenticatedRoute.extend(base, {
1414

1515
// used to check if anything has changed in the editor
1616
controller.set('previousTagNames', Ember.A());
17+
18+
// attach model-related listeners created in editor-route-base
19+
this.attachModelHooks(controller, model);
1720
},
1821

1922
actions: {
@@ -46,6 +49,9 @@ var EditorNewRoute = AuthenticatedRoute.extend(base, {
4649

4750
// since the transition is now certain to complete..
4851
window.onbeforeunload = null;
52+
53+
// remove model-related listeners created in editor-route-base
54+
this.detachModelHooks(controller, model);
4955
}
5056
}
5157
});

0 commit comments

Comments
 (0)