PaulUithol / Backbone-relational

Get and set relations (one-to-one, one-to-many, many-to-one) for Backbone models
http://backbonerelational.org
MIT License
2.34k stars 330 forks source link

Cloning a backbone model with relations does not perform deep cloning #50

Open aquam8 opened 13 years ago

aquam8 commented 13 years ago

When cloning a simple backbone object with a HasOne relationalship (by calling model.clone() ) the relation is not cloned. There's only a pointer to the original object (same cid) so it's not deep-copied.

Also i noticed that cloning a model with a HasMany relationship does not copy the models (array) at all.

I think the clone() method needs to be supported by backbone.relational.

None of the test cases (in test/test.js) handle clone() - missing in test coverage.

Let me know your feedback Thanks lot and well done for the library btw!

PaulUithol commented 13 years ago

Well, that's very much possible. I've never used clone, didn't even know I missed it, and no-one complained up till now ;). I'll take a look when time permits.

jamalex commented 13 years ago

This could be helpful for me as well. I'm working on a course management system, and professors need to be able to create clones of old courses when they reteach the same course again (and want to keep the old course intact). I was thinking to write something to manually clone the models and re-save them (especially since there may be components, such as student roster/grades, that shouldn't be included), but if there were some way to integrate this with Backbone-relational, that would be great! Thanks.

PaulUithol commented 12 years ago

Hmm.. after thinking about it for a bit, I wouldn't quite know how the semantics should work with regards to HasOne and HasMany relations? I've taken a quick look at some other ORM implementations, but I can't find much on how (if at all) they handle cloning a model including it's relations.

For example, suppose you have a Book which HasMany Pages (and a reverse relation in which each Page HasOne Book). If I were to clone a Page, it could probably still reference the same Book (it's id would need to be unset though on clone).

But what if I were to clone the book? If the HasMany in the cloned book is supposed to be referencing the same Pages as the old book, these pages would need to point both to the old and to the cloned book. Which is not possible for a HasOne relation.

rjharmon commented 12 years ago

I'm encountering this use case myself. @PaulUithol: if I clone the book, I'd expect all its Pages to be cloned without Id's (-> isNew() == true). Thus when the new book is saved, all the pages would be created afresh. I can't think of any way to meaningfully clone pages without making them new instances.

That said, it seems like HasMany's need to either be cloned or not-cloned per the requirements of any particular case. If it's not too nasty, making it controllable both with default settings (for declarative behavior) and with clone-time settings (for behavior responsive to user's runtime requirements) would be nice.

Has anyone done anything around this as of now?

aquam8 commented 12 years ago

Works around is that you need to clone the nested object yourself. Like:

   var format_to_save = selected_format.clone();   // clone() is not

going to clone your nested models format_to_save.set({ selected: false, // Reset the model to not selected. labels: selected_format.get('labels').models, // Deep-copy the labels question: selected_format.get('question').clone() // Deep-copy the question (clone) });

With Format a relationship model with:

relations: [ { type: 'HasMany', key: 'labels', relatedModel: LabelModel, collectionType: LabelList, }, { type: 'HasOne', key: 'question', relatedModel: QuestionModel } ],

It's just inconvenient and hard to maintain.

Matt

On 8 December 2011 13:36, rjharmon < reply@reply.github.com

wrote:

I'm encountering this use case myself. @PaulUithol: if I clone the book, I'd expect all its Pages to be cloned without Id's (-> isNew() == true). Thus when the new book is saved, all the pages would be created afresh. I can't think of any way to meaningfully clone pages without making them new instances.

That said, it seems like HasMany's need to either be cloned or not-cloned per the requirements of any particular case. If it's not too nasty, making it controllable both with default settings (for declarative behavior) and with clone-time settings (for behavior responsive to user's runtime requirements) would be nice.

Has anyone done anything around this as of now?


Reply to this email directly or view it on GitHub:

https://github.com/PaulUithol/Backbone-relational/issues/50#issuecomment-3057027

PaulUithol commented 12 years ago

Ok, for now I've at least made sure clone doesn't break existing HasOne relations; I'm still not quite sure how to proceed, so for now relations simply aren't cloned at all. Cloning everything (including HasMany relations) certainly won't do it though; in a lot of cases, this would double the amount of models you have.

TimBrunsmo commented 12 years ago

This should probably be an optional behavior (if implemented).

I need deep cloning for for schedules with nested intervals. When a user wants to add a new day, schedule (including intervals) should be cloned from last day.

Using @aquam8's work around for now.

@rjharmon "I can't think of any way to meaningfully clone pages without making them new instances.": me neither.

nthsense commented 11 years ago

I'd suggest that when you clone a HasMany relation, you would probably NOT want to clone the children by default. However, I would expect that the collection would be cloned so that it could contain a different subset of children. At least in the version of Backbone.Relational I'm using, modifications to the clone's HasMany collection result in modifications to the original.