fix issue with removeLinkStorages when specificaly querying the link field

[vparpoil]

This commit fixes this issue #386
Tests are still passing locally

[theodorDiaconu]

I think the solution is in the graph preperation phase, not in the delivery.

[vparpoil]

Can you indicate me which file/function you have in mind ?

I have been searching for the scheduledForDeletion param in the preparation of the graph but in the case referenced in the issue, the field authorId is never marked as scheduledForDeletion

As you can see in the code modified in this PR, the code removing the field is not testing for the scheduledForDeletion field anyway, but only removing collectionNode.linkStorageFields.
I don't see which part of the graph I should update in the preparation to avoid the issue (I am pretty sure that unsetting collectionNode.linkStorageFields is not an option here because it will break other logic)

[bhunjadi]

I think you should check createGraph.js for preparation.
There is also add function in collectionNode.js which sets conditionaly shouldCleanStorage.

It seems that order matters but it should not. _shouldCleanStorage depends on fields that were added and when authorId is after author field it does not catch it as added and therefore it is cleaned.

Solution could be to first separate db fields, linkers and reducers and add the db fields first and then other types of fields.

[vparpoil]

Thanks for the insight !
indeed, _shouldCleanStorage is called after each linker node is added. Since the auhtorId has not been yet added to the CollectionNode, it is marked with the shouldCleanStorage attribute.
I will try to propose an alternative fix sorting root.body in the createNodes function

[vparpoil]

@theodorDiaconu Here is an updated version modifying the body keys order to add the fields first to the CollectionNode. Let me know if you think it is not the good way to handle this.

[theodorDiaconu]

Not sure if sorting is the right idea here. It may fail for nested links. I think the solution is to not mark the field as scheduled for deletion, or use it above the link.

[bhunjadi]

Yeah, with that sorting function we are doing kind of a guesswork. Why not make this process 2-pass? First, we determine what are props, fields, reducers and links. Second, we add them. Props and fields first, then links and reducers.

[vparpoil]

Thank you for your time reviewing
Since the fields are not yet added in _shouldCleanStorage, the hasField method cannot see that the field is supposed to be there.
We could update this line and change it with

result = !this.hasField(node.linkStorageField, true) && !(node.linkStorageField in node.parent.body)

Would that be a better solution @theodorDiaconu ?

[StorytellerCZ]

@theodorDiaconu @vparpoil tests are now passing, so I want to inquire how you two feel about merging this.

[vparpoil]

Hi @StorytellerCZ , I don't feel very confident myself because I was expecting some approval from @theodorDiaconu
I don't know the codebase enough and this was a long time ago. I could work on another proposition for this fix but I need some hints on how / where to fix the issue.
Thanks for bumping this PR !