Why do we detach the table children from the toplevel family?

[open-dynaMIX]

Out of context for this PR, but why do we detach the table children from the toplevel family? This doesn't make any sense IMHO, and puts rocks in our path when fetching the document for validation etc

Originally posted by @winged in #505

[czosel]

/cc @sliverc

[czosel]

Related: #212 (comment)

[czosel]

From what I gathered until now, table rows should always have the same family ID as the root document (in fact, table questions were the reason document families were introduced in the first place). The fact that table rows can have different family IDs seems to be a bug.
I just tested this locally and for me the first row had the right family ID, while the second didn't.

Concerning the question "Why do we detach in the first place?" - Could it be that this is a security measure in case the root document reference is changed in update?

[winged]

OK with some more analysis, I think I know why this is done:

The detaching happens so the code is correct in any update case, even when the containing document changes. So the behaviour is this:

1. detach (by deleting AnswerDocument objects)
2. save answer (possibly including moving to another document)
3. reattach by recreating AnswerDocuments

So my original note is wrong, even if the code looks like it.

@sliverc please confirm if my assumption is correct.

In any case, I think it may be worth it to only do the detach-reatach circus if actually needed, just to improve performance a bit. Especially since we won't be able to do bulk updates anymore when #522 lands

[sliverc]

@winged
As far as I remember the main reason was that the user interface can already start pre-saving answers before it actually has to add a row to the table. Otherwise it would have to add a table row already before it has all answers which might lead to ghost rows for the user.

I think @anehx and I designed this together and he might have some more feedback on it. If it causes problems this can be rethought though.

[czosel]

👍 That's also how we ended up implementing it - thanks for the affirmation 🙂

[sliverc]

@open-dynaMIX Can this be closed or is there still something unclear?