Skip to content
This repository has been archived by the owner on Oct 17, 2018. It is now read-only.

Bugfix realAllUnder for collection and map with complex field path #56

Merged
merged 1 commit into from
Aug 24, 2016

Conversation

florianlacreuse
Copy link
Member

No description provided.

@yrodiere
Copy link
Contributor

Seems like I did something wrong indeed. I may also have forgot to include a test for this case...

Anyway, while this commit would probably fix the immediate problem, a better course of action might be to add a "parentModel" to each BindableModel (except the root and collection items), and use that parent instead of propertyModelsByImpactingPaths for read event propagation. The readAllUnder method would now simply be a bind(bindingRoot).readAll() and readAll would now include a loop akin to BindableModel parent = parentModel ; while (parent != null) { parent.read() /* Not readAll */; parent = parent.parentModel; }. This would ensure that, when a model was registered as Bindings.a().b().c(), using the model for B and calling readAllUnder(Bindings.b().c()) would still trigger a read on the model for A.

@yrodiere
Copy link
Contributor

Something is still wrong with my previous comment. The "read" loop should trigger reads from the top down, so it should do something like:

void propagateReadToParent() {
  if (parentModel != null) {
    parentModel.propagateReadToParent();
  }
  read();
}

public void readAll() {
  propagateReadToParent();
  // ... do stuff that's already done in the current code
}

@florianlacreuse florianlacreuse force-pushed the bindable-model-bugfix-readallunder branch from 036bc89 to 0d48cf4 Compare August 22, 2016 09:24
@lalmeras lalmeras merged commit 06d81fa into master Aug 24, 2016
@lalmeras
Copy link
Member

This fix needs to be refactored to address Yoann advices.

@lalmeras lalmeras deleted the bindable-model-bugfix-readallunder branch January 23, 2017 14:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants