Provide a way to mark a model as dirty when relationships or related model properties change.

[timrwood]

Summary

Provide a way to mark a model as dirty when relationships or related model properties change.

Motivation

There is a common pattern to conditionally show a save button if a model needs saving.

For most cases, using the isDirty flag on the model is enough to determine whether a model needs saving or not. However, this flag currently ignores relationships, making it unusable for anything other than simple properties.

Ideally, the user would be able to specify which relationships are saved when the model is saved, and thus, which ones should mark the model as being dirty.

Detailed design

In order to prevent dirtiness from spreading across the entire object graph, dirtying would have to be opt in. Users would specify on each relationship whether it would dirty the model or not.

var Comment = DS.Model.extend({
  post: DS.belongsTo('post', { dirties: true }),
  user: DS.belongsTo('user')
});

var comment = this.store.find('comment', 1);
comment.get('post');    // null
comment.get('user');    // null
comment.get('isDirty'); // false

comment.set('user', someUser);
comment.get('isDirty'); // false

comment.set('post', somePost);
comment.get('isDirty'); // true

comment.set('post', null);
comment.get('isDirty'); // false

For hasMany relationships, adding or removing a relationship would dirty the model.

var Post = DS.Model.extend({
  tags:     DS.hasMany('tag', { dirties: true }),
  comments: DS.hasMany('comment')
});

var post = this.store.find('post', 1);
post.get('isDirty');         // false
post.get('tags.length');     // 0
post.get('comments.length'); // 0

post.get('comments').pushObject(someComment);
post.get('comments.length'); // 1
post.get('isDirty');         // false

post.get('tags').addObject(someTag);
post.get('tags.length'); // 1
post.get('isDirty');     // true

post.save();
post.get('isDirty'); // false

post.get('tags').removeObject(someTag);
post.get('tags.length'); // 0
post.get('isDirty');     // true

Sometimes, entire related models are serialized when a model is saved. In those circumstances, it would be useful to allow the model to be dirtied when any of the related model properties are saved.

var User = DS.Model.extend({
  address: DS.belongsTo('address', { dirties: 'properties' })
});

var Address = DS.Model.extend({
  street: DS.attr('string')
});

var user = this.store.find('user', 1);
user.get('isDirty'); // false
user.get('address.street'); // 4th Ave

user.set('address.street', 'Main Street');
user.get('isDirty'); // true

user.set('address.street', '4th Ave');
user.get('isDirty'); // false

Drawbacks

This would significantly increase the complexity of the isDirty computed property.

There would probably need to be checks in place to prevent recursive dirty checking. In the example below, once either of the models is made dirty, they would never become clean again by reverting a property, as each one's isDirty state is dependent on the other's.

var A = DS.Model.extend({
  b: DS.belongsTo('b', { dirties: true })
});
var B = DS.Model.extend({
  a: DS.belongsTo('a', { dirties: true })
});

Alternatives

Paul Ferrett's isFilthy computed property

Steven Lindberg's Dependent Relationships

Tim Wood's Dirty Relationships Mixins

Unresolved questions

The syntax feels like it could use some bikeshedding.

DS.belongsTo('other', { dirtiesOn: 'relationship' })
DS.belongsTo('other', { dirtiesOn: 'properties' })

Conceptually, a relationship should be considered dirty if the relationship is being saved by the model, so it could be named something related to saving.

DS.belongsTo('other', { savesRelation: true })
DS.belongsTo('other', { savesModel: true })

[calvinmetcalf]

There is also a simpler need, to tell if you are pointing to a different model as opposed to if the model you are pointing to changed

[timrwood]

@calvinmetcalf, yeah, that use case should be covered with the dirty on relationship change option.

var Post = DS.Model.extend({
  user: DS.belongsTo('user', { dirties: true })
});

var post = this.store.find('post', 1);
var user1 = this.store.find('user', 1);
var user2 = this.store.find('user', 2);

post.get('user');    // user1
post.get('isDirty'); // false

// changing to another user dirties the model
post.set('user', user2);
post.get('isDirty'); // true

// unsetting the user dirties the model
post.set('user', null);
post.get('isDirty'); // true

// changing back to the original user makes the model clean again
post.set('user', user1);
post.get('isDirty'); // false

[ahacking]

I've gone down this path and backed out because I need to control the tainting of the graph based on context. As a result, I quite firmly believe that inter-model dirtying is a controller concern.

Keeping this concern in the controller layer also aligns with orchestration of model persistence and edit buffering (ie not modifying models until a user makes a positive action to save).

The main difficulty with dirty tracking in the model layer is that it cannot be aware of different edit contexts, eg:

• a list of items with a click-able like or star rating (single attribute change, implicit save)
• changing task duration in a project management application may need to adjust parent task duration recursively up the task hierarchy
• changing a task assignment may change just the task or create a new task assignment model and delete the current one
• changing the start date on a parent task may need to adjust start date of all child tasks.

The models involved in each editing context are very different and dirty rules in the model layer cannot accommodate all of the contexts without causing run-away dirtying of a large portion of the model graph.

When you implement controllers to support your views, your controllers collectively orchestrate model mutation. Since your views inherently form a tree without cycles, the resulting controller hierarchy to support the view is in many cases also a tree. This makes dirty tracking, undo/redo/edit-buffering a much more simple problem as controllers can notify their parent controllers (or alternatively observe the dirty state of their children). Model mutation and persistence is orchestrated through the controllers, such that model layer dirty tracking is basically an unneeded appendage. I would be happy if model dirtying went away completely, I'd much prefer effort go into out of the box support for current state vs server state delta calculation for generating PATCH requests, and controller support for model buffering/undo/redo management.

The only case where I believe dirtying between models makes sense given the current model dirty support is for embedded models that must be persisted via their "parent" as is the case for the embedded belongsTo and embedded hasMany cases. But even so, the controller approach handles this very naturally.

I ultimately see dirty tracking at this level as a mis-feature and would certainly not want to pay the cost of the dirty tracking unnecessarily invaliding thousands of models, or having the dirty tracking configuration misaligned with the edit contexts my controllers support and what my views render.

[slindberg]

Thanks for submitting this @timrwood! 🍻

I've have the opposite experience of @ahacking: I first went down the road of managing relationship dependencies outside of models, which resulted in a mess of extremely difficult to maintain callback soup, amounting to a large parallel state machine glued to ED's individual model states. At one point I decided to patch ED to support dependent relationships, which has resulted in much more straightforward application code. I can simply check for isDirty in templates to enable a button that calls save on the parent model, which recursively saves all dependent relations before saving itself.

I think the difference in our experiences is in large part due to our needs: my app uses dependent dirtying relationships only for a single model that is self-referential, and whose relationship graph is essentially a set of isolated trees which inherently avoids the run-away dirtying problem. It would also not work well without special model logic for cascading saves, so I agree that relationship dirtying only makes sense when child record persistence is linked to the parent record's persistence.

However, I think there are two different problems at play: dirtying based on child model dirty state, and dirtying based on relationship references only. As @ahacking pointed out, the former comes with big tradeoffs, whereas I think that the latter is much safer. The problem with the latter is that it depends on how the relationships are serialized, which is hidden by the serializer: if model A hasMany B, and B contains the only FK to A, it doesn't make sense that A should be dirtied when a B is added/removed from it, but that logic only exists in the serializer. Ideally, a model would only become dirty as a result of a relationship change if it is responsible for persisting that change. I believe that @igorT is a proponent of incorporating relationship dirtying into the serialization process for this reason.

On a side note, one of the gotchas I've run into with hasMany relationships dirtying has to do with the order of relations. In some cases it makes perfect sense to dirty the parent model when the order of relationships change, i.e. when precedence is implied by the order and not through a model attribute. In other cases it doesn't make sense, say when you create a number of relations client side and save them all at once, the order they are persisted in may not be the order you created them in, resulting in a dirty record that was just saved.

I think that treating hasMany relationships as unordered sets where the API does not have to guarantee order is by far the safest way to handle dependent relation dirtying. However this may be a tough sell, as I've worked with more than a few that make use of implicit relation sort order.

[ahacking]

@slindberg That's an interesting experience. I plan on posting my controller pattern at some point for others to use/review as the pattern is working very well for me. Like you I have recursive trees (actually models that are recursive in multiple dimensions), but the code is still very uncomplicated.

Like you I found its not really possible to isolate all serialization concerns in model serializers as the controller does have to orchestrate complex changes depending on the server api. An api that just accepts an arbitrary patch set for an arbitrary model graph is utopia, whilst one that requires one to persist a model graph piece-meal with data dependencies (ie requires server generated keys from resource A so you can then save resource B containing the foreign key of A) is beyond what model level serializers can realistically deal with in a general way (ie you end up writing controllers). You also have cases where you are orchestrating persistence between different systems (ie upload an image to server A, which provides a url, and then persist a user model change on server B linking the users avatar to the image).

For all these reasons and the ones I mentioned previously I have found controller mediation to the models and controller orchestration of persistence to be the compelling solution. It also allows one to create a façade over your models to better support the way you want users to interact with views and handle persistence concerns that in the view may just be a check-box but may materialize a join model bridging two or more models. Once you move beyond simple CRUD views there doesn't appear to be any other choice but intelligent controllers.

I agree with keeping relationships as unordered sets is the safest assumption. Sorting is something that should live in the controller. I accept that server ordering of result sets is important (eg paging offset/limit infinite scroll) and certainly there may be hasMany ordering or some kind of natural ordering defined on the server which is not reflected explicitly in model attributes by server, but those cases CAN be handled by assigning a position (or rank) attribute in your model serializer so that controllers can re-sort on position/rank or any combination of attributes.

[timrwood]

The more I think about this, it seems savesRelationship and savesModel are better ways to describe this.

If we have a way to describe what a model is responsible for persisting, we can make use of that information for both isDirty and the serializer.

savesRelationship would make the model responsible for persisting the FK. Thus, if the FK changed, the model would be considered dirty. As @slindberg mentioned, hasMany ordering may be important, so something along the lines of savesOrderedRelationships might be needed.

savesModel would make the model A responsible for persisting model B. Thus if the FK or any of model B's properties changed, the model A would be considered dirty.

The serializers could now use this data to figure out which FK or models to serialize with the primary model. It would be pretty cool to add { savesModel: true } in ED and accepts_nested_attributes_for in Rails and not have to fiddle with serializeHasMany or serializeBelongsTo.

[ahacking]

@timrwood My understanding is that ED already went down a path of trying to be smart about how relationships get persisted and failed hard. If I'm not mistaken the "ED reboot" took things back to brass tacks so people could start to be successful with Ember and ED. Certainly in my case the bad press surrounding ED kept me away from using Ember in anger for about a year.

No two api's or endpoints are alike, in some cases relationships are managed from the hasMany side, and in others from the belongsTo side, and some from both sides!, and then you have embedded records, request/transaction ordering and request aggregation, and POST/PUT vs PATCH, results vs no-content, partial results, server assigned ID's vs client ids/uuids and so on.

I have to respectfully say that whilst AMS is certainly a valid use case for ED, it hardly represents the world of JSON apis, and it should not be the primary driver for the persistence layer requirements for Ember or ED. I can see its influence all over ED, and I have to say its not a particularly great influence and results in a lot of unnecessary data munging when using ED with other apis.

I am earnestly trying to provide some perspective here and avoid a myopic view that results from focusing too closely on a single implementation of server persistence and trying to extrapolate that as the way persistence in ED should work.

To be quite plain, I'm mostly happy where ED currently draws the line. I like that I can be explicit about model persistence in my controllers or routes, ie "do this, then that, and all these in parallel", I can also handle server validation errors or other persistence related errors intelligently and naturally. This is not to be understated, as once you relinquish control to the model framework, its pretty much game over, or fight-fight-fight, agghhh ... hello $.ajax()

[slindberg]

@ahacking The biggest mistake ED made early on was to impose too much convention without enough configuration – being too restrictive – which was largely corrected by the introduction of per-type adapters and serializers. But another mistake was trying to support too much out of the box – being too complex – which was partly corrected by backing out support for embedded records and changing the serializer API to support embedded records through data transformations. The trick is finding the middle ground between too restrictive and too complex, while still being able to supporting common use cases.

So perhaps you are right that the notion of savesRelationship/savesModel is beyond the scope of ED itself, but I also think that the idea of dependent relationships is a valid data-layer concern (which I know you disagree with). The middle ground here is for ED to expose enough about its internals for an extension to provide support for dependent relations, much like DS.EmbeddedRecordsMixin adds support for embedded records. Devs with complex model relationships can simply choose not to use the extension, while devs with simpler needs (or out-of-the-box solutions like AMS) can use it at their own peril. Perhaps this could be accomplished by simply adding a way to manually flip the dirty flag, like many people have suggested.

I guess what I'm saying is that the line that ED draws has to be the lowest common denominator, but that shouldn't preclude supporting more complex use cases as packaged add-ons. Even if the result of this RFC is that dependent relations won't be supported by ED core, it could lead to API tweaks/additions that make it possible for those that choose to use it.

[macu]

I'll chime in, since I had to work around this recently. My use case was changing a belongsTo property, which represented the status of a record from among available statuses (unsent, sent, processing, completed, etc), or the category the record is assigned to, or other things.

There was some ugliness involved to dirty the model when changing the belongsTo relationship, but recently it stopped working, so I've rewritten the belongsTo as an attr('string'), and now use an intermediate computed property to translate between ID and the corresponding model. For this, the computed property needs access to the store. For example:

// Make the store available to this model.
App.inject('model:my-model', 'store', 'store:main');
Ember.MODEL_FACTORY_INJECTIONS = true;

App.MyModel = Model.extend({
    // ...

    // Do not access this field if expecting a Category object.
    // See the accompanying categoryObject field below.
    category: attr('string'),

    // This computed property represents the category field,
    // allowing get/set using category objects.
    categoryObject: function(key, categoryObject, previousValue) {
        if (arguments.length > 1) {
            // Set the category field to the ID of the given categoryObject.
            this.set('category', categoryObject.get('id'));
            return categoryObject;
        } else {
            // Return the appropriate model from the store.
            var catID = this.get('category');
            return this.store.getById('category', catID);
        }
    }.property(),

    // ...
});

It is much cleaner than the original code, but still feels like a hack. It would be OK to retrieve the related model elsewhere, e.g. in the controller or in Handlebars helpers where properties from the related objects are displayed in templates, but I prefer having straightforward access to the related model.

It isn't too much hassle to use intermediate computed properties like this, but it would be nice if changing belongsTo would at least dirty the model that owns the relationship.

[ahacking]

@macu I think the ED relationship CPs are not named appropriately. They really should NOT convey or tie cardinality to an ownership semantic. I have models where belongsTo is just wrong, and it's especially wrong with an embedded belongsTo.

It would be better to have manyToOne, oneToMany and oneToOne and perhaps manyToMany and manyToOne if many-many is ever supported.

This would avoid ownership expectations that dont actually apply when describing cardinality.

Things to consider is that if the "owning model" also has a "belongs to" then there is the possibility based on all the relationships you have that dirtying will run like wild fire through the model graph and I certainly don't want that.

In general it will not be possible to express dirty rules that work in all editing contexts for your application. Try as you may to limit directions in which dirty state propagates and you will end up with dirty islands that will invariably get bridged through some model and dirty state will propagate through the model graph where you don't want it. Then you will be forced to put firewalls in your models (ie turn off dirty propagation) for use cases that trigger the unwanted propagation and instead handle the dirty tracking in your controllers.

[DougPuchalski]

Accidentally undeleted previous comment. Paraphrasing:

Since the model declares relationships it should also declare state relationships. Controller's job is what to do about it.

I'm thinking a different property--hasDirtyAssociates or something similar, leaving isDirty as-is. No additional config necessary except for special cases.

Recursion is only an issue for serializer, nothing new here. First a model is marked as dirty, then iterates over its non-dirty relationships. No loopback possible.

[ahacking]

@aceofspades Models are relatively simple and don't understand the business logic or the context in which they are being mutated. By your reasoning since models declare relationships they should also understand all the business rules and know the significance of a mutation and react intelligently and also able to distinguish changing just an attribute that affects just the model vs when it should propagate.

Can you re-read the examples I provided previously where different editing contexts are involved, some where a changing a parent affects children and some where changing a child affects parents. What if you have copy on write business rules as I do which depend on context?

Basically what I am saying is controllers exist for a reason to handle business rules "ie what makes sense and must be done for a given situation" and whilst models and model relationships are involved they do not define or know the context and cannot behave appropriately so it makes no sense to make dirty propagation run like wild fire and dirty a large part of the model graph automatically just because something is reachable via relationships and something that changed.

I'm not concerned with loopback as that's simple to detect, I'm concerned that its useless, meaningless and a performance killer to propagate dirtyness without the context of a change being known. Controllers have the context and know the significance of a change and its scope of effect, whilst models never can.

By your simple logic, adding a comment to a post should dirty the user making the comment as it belongs to the user, it should also dirty the post as the comment belongs to the post and since the post belongs to a user that user should be dirtied also. But since you don't even want any additional config, not only would the original comment be dirty but every comment that user ever made and every post that user ever made and every post that user ever commented on and every other users comment on every post the user made and every user and their posts and comments ad-infinitum until every reachable model is marked dirty. I don't understand the value of blind dirty propagation and for all intents I say its rather meaningless.

[timrwood]

every comment that user ever made and every post that user ever made and every post that user ever commented on and every other users comment on every post the user made and every user and their posts and comments ad-infinitum until every reachable model is marked dirty

@ahacking, I think this is a bit of a over-exaggeration.

The proposal is based on the idea that a relationship can be in 1 of 3 dirtying modes.

1. Never dirty the record when the related record changes. This is the default.
2. Dirty the record when the related record id changes.
3. Dirty the record when the related record id changes, or the related record is dirty.

In order for your scenario to happen, posts, users, and comments would all need to have their dirtying modes set to the third mode.

I think a better way to model that would be something like the following.

Post = Model.extend({
  comments: hasMany('comment'),
  user: belongsTo('user', { dirties: 'relationship' })
});

Comment = Model.extend({
  post: belongsTo('post', { dirties: 'relationship' }),
  user: belongsTo('user', { dirties: 'relationship' })
});

User = Model.extend({
  posts: hasMany('post'),
  comments: hasMany('comment')
});

With this setup, users would never be dirtied. Comments would only be dirtied if the post or user changed to a different model (not if any of their properties changed). Posts would only be dirtied if the user changed to a different model.

[ahacking]

@timrwood yes in mode 3 that's exactly what would happen and was what @aceofspades proposed with no configuration and mindless dirty propagation so its not an exageration.

Your solution still doesn't work for situations like I have stated previously where model changes can propagate in either direction based on CONTEXT.

It seems no one is understanding editing context. In a simple crud like form per model you may get some use of static model based dirty propagation rules but it is not possible, even in a simple project/task relationship to declare a rule that works well for multiple contexts.

The reason I am opposed to it is that it can't work generally and you have to adopt a different approach entirely when the static rules can't handle it, eg users ask for the ability to change something on a closely related model and now your dirty rules hit the wall. The rules are brittle and susceptible to unintended run away propagation. In the lifetime of an app I think the proposal goes against the Ember way as developers have to adopt a completely different strategy when the static dirty rules can't cope or dont match with the stuff rendered in the view.

[DougPuchalski]

@ahacking Yes, models are simple, but they declare attributes and associations. If dirty tracking of attributes is well-established to be in the model, it seems odd to me to do it elsewhere for associations. For the simplest and I would think the majority of cases that's going to do the job without the extra indirection.

Of course there are valid use-cases for overrides in a controller based on context. I would think the current pattern of an itemController overriding things and otherwise delegating to the model should handle your use-case perfectly, to be used when the context calls for it. But why push complexity of the special cases onto the simple ones?

+@timrwood Per config, I strongly prefer the fail-safe. Per mode 3, that bug will be found a lot more easily than if some deeply nested attribute that never got saved when expected. (A bug where there's an infinite loop would be hard not to notice!) Either way, some wasted cycles or a hard failure is benign compared to data loss.

To reiterate, my comment called for a separate property for dirty associations vs. properties, so it's always opt-in anyway. It's up to your controller to figure out what to do with the information.

If you are building an object graph that include a user like the example, this seems like a pretty clear case where you would explicitly terminate traversal at the lowest level, the model. You're never going to want to go there, and if you do it's probably a special case. When it does get more complex than that, override it in the controller and call it a day.

[ahacking]

@aceofspades I actually don't find dirty tracking in the model useful either. What I would find useful is the ability to save a model with a change-set and only once it has successfully committed, actually update the models attributes and relationships. But alas ED doesn't work that way and gives me something that is less useful such as tracking dirty state.

Of course there are valid use-cases for overrides in a controller based on context. I would think the current pattern of an itemController overriding things and otherwise delegating to the model should handle your use-case perfectly, to be used when the context calls for it. But why push complexity of the special cases onto the simple ones?

Yes, exactly. From my perspective, mediating models via controllers is the simple case. Why implement a mis-feature like dirty-tracking on models, why have this complexity and overhead there at all when we know it can only cater for some rather limited use cases? Controllers mediating changes to models is very natural and straight forward and the Ember way. I do have to ask myself given controllers mediate access to models, perhaps the answer is not ED and just using POJOs with ajax or something like Backbone sync. Why do I say that?

... Its fairly common to use a buffered proxy in controllers to deal with ED design issues, so that user edits don't take effect immediately in models, so that in flight requests don't ruin/block the user experience. I would say ED maintainers need to take a careful look at what is required to implement edit views and forms in ambitious applications; ie edit views composed of several models subject to business rules and what that means in terms of managing the edit context, enabling/disabling the save button, reverting changes, or undo/redo, not polluting the model graph with interim changes that have not been accepted/committed by the user, what it means for orchestrating the persistence of those changed models/relationships, and what it means in handling and recovering from failures (eg server validation errors) and also at least think about offline/online scenarios. I think its no accident that the Ember flagship applications like Discourse and Ghost have had to find other solutions because of the disconnect from what they really need and what ED currently provides and the way it provides it.

Whilst I accept there are some simple use cases served by this feature, I cannot accept that this feature is a solution to anything in a real application. I don't want to pay for mis-features in my application js payload that don't provide a uniform solution and I don't want to see features added to Ember or ED that promote brittleness and maintenance traps.

I'm happy to elaborate further on the use cases and drivers I have listed above, as it is desirable from my perspective to have a data access solution like ED that fits with how ambitious applications need to work.

Its ok if we disagree, I see it firmly as a controller layer concern, whilst some are saying it should be in models to support simple use cases. I feel I've said enough on why its a controller layer concern but if anything I've said isn't clear I'm happy to elaborate, other than that I've said my piece.

[macu]

@ahacking I fully agree with your earlier comments, where you said "belongs to" is misleading and does not accurately describe all one-to-one relationships. But I find it very difficult to follow your current argument. While I agree in principle about opposing brittle mis-features, it seems that you and I have a fundamentally different approach in thinking about models on the client. I can hardly imagine the headache of copying model properties into controller properties for mutation, and then sending patch requests to the server before seeing updates in the model, only to extract the updated properties into mutable controller properties again. Perhaps I've misunderstood.

The ability to bind model attributes to form components, make updates directly to models, and see those updates reflected across multiple computed properties and templates simultaneously is used extensively in the applications I've developed. I find it fits my use cases perfectly to have the ED model reflect "dirty" (and provide a rollback method) when current state differs from the last state reported by the server. The extra care required to handle dirty models when the user navigates to a different route is often necessary anyway (e.g. prompting the user to save/discard changes, or cancel navigation). I don't see a need or benefit to keeping modifications out of the model until saved. (Unless you think the data store should act exclusively as a cache, updated only to reflect the latest data from the server.)

We can all agree that the issue of propagating dirtiness is complicated. Any talk about introducing new configurations for model relationships should probably consider the difference between embedded and independent records in serialization. (Currently, "embedded" in Ember Data is only meaningful in serialization/deserialization.) For example, if an embedded record cannot be identified to the server and updated independently of its parent, then the parent should be dirtied when the embedded record is modified. (This kind of embeddedness probably will not be supported.) In all other cases, I can't see why dirtiness must be propagated, as individual models can be modified and saved while other models continue to refer to them.

I do see a need to dirty a model when its belongsTo is set to point at a different referent, or its hasMany array is modified (IDs added or removed), regardless of the dirtiness of the related models. This would make belongsTo and hasMany like attr, dirtying the model when changed. Any new records added to a hasMany collection would need to be saved independently. Any deleted records would be removed from hasMany and belongsTo relationships. This is the only complicated area that really stands out to me, that if belongsTo or hasMany is modified to refer to a new record, then the model can't be saved until the new record is saved and assigned an ID; and similarly, if a record is deleted, then all belongsTo and hasMany relationships that referred to it must be updated, and so those models would be dirtied, but not all of those models are guaranteed to be present in the client-side data store. As a further complication, the rollback of a pending delete would rollback the affected relationships.

These are the kinds of issues that I think need to be addressed in the discussion about propagating dirtiness. Somewhere along the line, I missed the point where it was determined that dirtiness should propagate further than one degree beyond the source.

[ahacking]

@macu,

@ahacking I fully agree with your earlier comments, where you said "belongs to" is misleading and does not accurately describe all one-to-one relationships. But I find it very difficult to follow your current argument. While I agree in principle about opposing brittle mis-features, it seems that you and I have a fundamentally different approach in thinking about models on the client. I can hardly imagine the headache of copying model properties into controller properties for mutation, and then sending patch requests to the server before seeing updates in the model, only to extract the updated properties into mutable controller properties again. Perhaps I've misunderstood.

I simply use a custom buffered proxy mixin on my controller which allows reads to come from the model and writes to be buffered. It uses unknownProperty and setUnknownProperty with very simple logic to stash model changes in a buffer object. Computed properties on my controller just work, if a model attribute changes (eg via an async model refresh) that hasn't been modified by the user, it just works, the user will see latest attributes updated from the server but their own edits are preserved. Come save/persistence time, I have the set of changed attributes in the change buffer (I don't even need to compute a delta!). Simple and elegant and very little code, totally headache free and approx 100 lines total. Navigating away from a route or cancel/revert just discards the buffer, undo/redo is a push or pop of the buffer to/from an undo stack.

For intra-model buffered handling any sub-controllers either specified by the view or by the controller simply notify their parent controller on dirty/clean state transition. Again at save time my controller has a list of dirty child controllers and can orchestrate model save, eg sometimes you need to create/save B before A or save/create B with A, or save A, then B then C. It is very clean and easy to orchestrate when the controller has a dirtyControllers array like I have or specific sub controllers for the B before A cases. The thing I don't like is pushing the buffered changes into the model in order to perform the save, I'd rather only commit the change in the model once the server has committed. So yes, I prefer to think of models as representing the most recent server state. Anything else to support the view (computed properties and buffered state) is in the controller. The reason I don't want to push changes into the model is that I don't usually want other views to update until the operation settles.

The annoying thing is it takes a fair bit of experience using Ember to even know how to pull things together for something as simple as a form meeting basic user expectations. Even though the implementation is simple and quite succinct, its the kind of pattern which I feel should be in Ember proper, irrespective of the data layer being used (another reason NOT to put dirty tracking in the model). As a result there is no need for models to track dirty state or "guess" via static configuration as to what the view or controllers might need to include in scope as being in the 'dirty set', its defined by the view and the controllers supporting the view.

We can all agree that the issue of propagating dirtiness is complicated. Any talk about introducing new configurations for model relationships should probably consider the difference between embedded and independent records in serialization. (Currently, "embedded" in Ember Data is only meaningful in serialization/deserialization.) For example, if an embedded record cannot be identified to the server and updated independently of its parent, then the parent should be dirtied when the embedded record is modified. (This kind of embeddedness probably will not be supported.) In all other cases, I can't see why dirtiness must be propagated any more than a single step, as individual models can be modified and saved while other models continue to refer to them.

Yes dirty tracking has a number of considerations, which is why I have been rather vocal about about it.

I TOTALLY agree that the embedded handling is not just a serialization concern. Routes/Controllers (and indeed views) are often developed with the knowledge that a model is embedded (since you typically don't save an embedded model except via its parent), and templates wouldn't label a button "Save", they may just label it "Accept"/"Done".

I do see a need to dirty a model when its belongsTo is set to point at a different referent, or its hasMany array is modified (IDs added or removed), regardless of the dirtiness of the related models. This would make belongsTo and hasMany like attr, dirtying the model when changed. Any new records added to a hasMany collection would need to be saved independently. Any deleted records would be removed from hasMany and belongsTo relationships. This is the only complicated area that really stands out to me, that if belongsTo or hasMany is modified to refer to a new record, then the model can't be saved until the new record is saved and assigned an ID; and similarly, if a record is deleted, then all belongsTo and hasMany relationships that referred to it must be updated, and so those models would be dirtied, but not all of those models are guaranteed to be present in the data store. As a further complication, the rollback of a pending delete would rollback the affected relationships.

Yes there is complexity there which is largely avoided when controllers orchestrate the persistence of what they know is dirty, its usually a case of A then B, or A with B, or B then A, or A then B then C...

[ahacking]

@macu

These are the kinds of issues that I think need to be addressed in the discussion about propagating dirtiness. Somewhere along the line, I missed the point where it was determined that dirtiness should propagate further than one degree beyond the source.

I failed to answer your statement earlier sorry, I think setting a belongsTo on a model should dirty the model that has the foreign key. But any further 'dirty propagation' could be achieved using CP's on each hasMany proxy. There may be recursive issues to deal with, but the property value caching may evaporate it.

[dbackeus]

+1 for this. Would prefer the saves... suggestion rather than dirties....

[Kilowhisky]

@webark Sweet! that's pretty much what i need. No more cluster frack of promises just to compare record equality! This should work nicely with the belongsTo fix that @decasia has.

For now though i'm back to just saving the relationship in a temp variable after the model hook as my attempt at using the mixin didn't quite work well enough.

[Awem]

OK, so we have emberjs/data#4361 and emberjs/data#3698. Both try to provide relationship rollback. However, there is no indication that either of them is going to be merged soon. That is bad. I would like to help, but without feedback from ember-data devs that is impossible. Anyone willing to provide an outlook?

[meszike123]

There was not much progress on this ticket for a long time, so I would like to open the discussion once again, because I believe relationships bring a lot of value to ember-data, and they are very important.

We have found an interesting and quite simple way of marking relationships dirty on model, where the basic idea is that the relationships have canonicalValue, which only changes, when the model is saved, or is reloaded.

As our models are quite complicated, we needed two levels of dirty checking. Sometimes for us it is enough to know of the reference (belongsTo) or the array of references have changed (hasMany), but sometimes we need to dirty the model, if the data in the referenced model has changed. And of course not only mark the model dirty, but rollback it, without reloading it from backend

A simple implementation of this approach is implemented in the https://github.com/meszike123/ember-managed-relationships addon.

I understand that this is probably not the best approach, and it would be better if relationships themselves will be refactored, but it might help somebody, till this is implemented in Ember.

[vigpatrick]

@meszike123 I had severall problem with ember-data-route and the fact that my model wasn't set to dirty when i edit an relationship record. I used ember-managed-relationship as you suggest and it does the job pretty well. I had to change some code in ember-data-route for the 'isDirty' and the rollback, but it work.

Thank you

[danielspaniel]

I have new add on that does change tracking and full on relationship rollback. It is fast and efficient.

https://github.com/danielspaniel/ember-data-change-tracker

[ghost]

Any news on this? I need to know in my model if some relationship (children) is still dirty...

[danielspaniel]

@johnunclesam https://github.com/danielspaniel/ember-data-change-tracker also does dirty tracking for relationships. You can setup computed property isDirtyRelations or just use isDirty

[ghost]

Thanks @danielspaniel, I don't need all of that code, (BEUATIFUL, AMAZING CODE!), because I need this just once in one model. Maybe you can make this code: http://paulferrett.com/2014/ember-model-isdirty-when-belongsto-changes/ actually in Ember 2? How to?

[sukima]

I don't know if this matters now but for me I did this trick to avoid more addons:

// app/model/my-model.js
export default Model.extend({
  foo: hasMany('bar'),

  registerSnapshot() {
    let foo = this.get('foo').toArray();
    this.set('_currentSnapshot', { foo });
  },

  rollbackSnapshot() {
    let relations = this.get('_currentSnapshot');
    if (relations) {
      this.get('foo').setObjects(relations.foo);
    }
    this.set('_currentSnapshot', null);
    this.rollbackAttributes();
  }
});

// app/components/my-form.js
export default Component.extend({
  saveForm() {
    let model = this.get('model');
    return model.save().then(() => {
      model.registerSnapshot();
    });
  },

  didInsertElement() {
    this._super(...arguments);
    this.get('model').registerSnapshot();
  },

  willDestroyElement() {
    this._super(...arguments);
    Ember.run.scheduleOnce('afterRender', () => {
      this.get('model').rollbackSnapshot();
    });
  }
});

[runspired]

Dirty tracking of relationships is now possible to be implemented by an addon via the RecordData RFC (#293). Many addons now exist that handle this and which can be further improved by making use of this feature.

I believe we should close this in favor of encouraging RecordData approaches or improving RecordData interfaces to make it smoother to do.

[runspired]

After discussing with the rest of the team, there is agreement on the points I made above.

[hjdivad]

@igorT @runspired reopening to remind us to give more detailed guidance + test re custom RecordData

[runspired]

I've reached out to ember-data-change-tracker to offer review/guidance for reimplementing via RecordData. We need to expand the documentation as well to include how to use RecordData. That is being tracked here emberjs/data#6359

[navels]

Seems that ember-data-change-tracker isn't actively maintained and given the outstanding issues probably not something we want to incorporate into our app. Here in 2023, assuming Ember 4+ (we are on 3.28 so that would be even better but we are upgrading soon), is there a recommended approach or addon?

[runspired]

@navels implementing a custom cache in 4.12 is probably the nicest way

[seanCodes]

@runspired Are there any examples or documentation explaining how to use custom caches? Using something like the following I’m getting errors because the requestManger is not defined (Ember 4.12 + Ember Data 4.12):

// app/services/store.js

import Cache from '@ember-data/json-api'
import Store from '@ember-data/store'

class CustomCache extends Cache {
  mutate(mutation) {
    console.debug('CustomCache.mutate', mutation)
    return super.mutate(mutation)
  }
}

export default class extends Store {
  createCache(wrapper) {
    return new CustomCache(wrapper)
  }
}