Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing to add iteration to top-level repeated section containing a section #4032

Closed
ebruchez opened this issue Apr 19, 2019 · 5 comments

Comments

1 participant
@ebruchez
Copy link
Collaborator

commented Apr 19, 2019

This works if the top-level repeated section only contains a grid. With a nested section, this doesn't work in a simple case.

The server throws an error:

|----------------------------------------------------------------------------------------------------------------------|
|Exception: java.lang.AssertionError                                                                                   |
|----------------------------------------------------------------------------------------------------------------------|
|scala.Predef$                                      |assert                        |Predef.scala                  | 156|
|rg.orbeon.oxf.xforms.control.XFormsComponentControl|createNestedContainer         |XFormsComponentControl.scala  | 114|
|rg.orbeon.oxf.xforms.control.XFormsComponentControl|recreateNestedContainer       |XFormsComponentControl.scala  | 134|
|org$orbeon$oxf$xforms$control$Controls$$buildTree$1|apply                         |Controls.scala                | 157|
|org$orbeon$oxf$xforms$control$Controls$$buildTree$1|apply                         |Controls.scala                | 138|
|scala.Option                                       |map                           |Option.scala                  | 146|
|org.orbeon.oxf.xforms.control.Controls$            |org$orbeon$oxf$xforms$control$|Controls.scala                | 137|
|orms$control$Controls$$buildTree$1$$anonfun$apply$3|apply                         |Controls.scala                | 164|
|orms$control$Controls$$buildTree$1$$anonfun$apply$3|apply                         |Controls.scala                | 164|
|xf.xforms.control.Controls$$anonfun$buildChildren$1|apply                         |Controls.scala                | 181|
|xf.xforms.control.Controls$$anonfun$buildChildren$1|apply                         |Controls.scala                | 180|

It produces the following result:

Screen Shot 2019-04-19 at 9 50 53 AM

See also #4018, which is probably a different issue.

@ebruchez ebruchez self-assigned this Apr 19, 2019

@ebruchez ebruchez added this to To do in Orbeon Forms 2018.2.3 via automation Apr 19, 2019

@ebruchez ebruchez added this to To review in Orbeon Forms 2019.1 via automation Apr 19, 2019

@ebruchez

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 19, 2019

Interesting that the server throws an error yet there is an Ajax response produced.

@ebruchez

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 22, 2019

We are getting a server error first with assert(_nestedContainerOpt.isEmpty) in:

updateBindings()
createRepeatIterationTree()
recreateNestedContainer()
createNestedContainer()

The condition to call recreateNestedContainer() is probably incorrect:

// Control got created relevant and, in the process, created the static `ConcreteBinding`.
componentControl.isRelevant &&
componentControl.staticControl.hasLazyBinding &&
componentControl.staticControl.bindingOpt.isDefined

The comment mirrors the following 2 comments in BindingUpdater:

// Control just got relevant and, in the process, created the static `ConcreteBinding`.
// Control just got non-relevant and, in the process, deleted the static `ConcreteBinding`.

In this case, we just created the XFormsComponentControl with the control factory. When that happens, createNestedContainer() runs at construction. So the container is there, and calling recreateNestedContainer() will raise the assert.

Now the question is: do we need to remove recreateNestedContainer() altogether, or just add destroyNestedContainer() first? Neither seem to make sense.

@ebruchez

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 22, 2019

Understanding:

  • ConcreteBinding got created during iteration 1
  • repeat iteration 2 and associated subtree of concrete controls is created
  • component with lazy binding
    • new concrete control in iteration 2 is built through the factory
    • createNestedContainer() runs at construction
    • control binding is evaluated
    • onCreate() is called
    • ConcreteBinding is created if needed
      • in this case, it doesn't need to be created since it was created during iteration 1

Now, if there was no iteration, and ConcreteBinding has just been created, do we need recreateNestedContainer(), or is the original createNestedContainer() sufficient?

The answer is that the call to createNestedContainer() in the constructor actually does nothing when the static binding is lazy and empty! So we do need to create the nested container after the static binding got created. Further, onCreate() got called, but since there was no nested container, it didn't create the nested models.

@ebruchez

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 22, 2019

Moving that spaghetti code to XFormsComponentControl to make things clearer.

ebruchez added a commit that referenced this issue Apr 22, 2019

@ebruchez ebruchez closed this in 7647b91 Apr 22, 2019

Orbeon Forms 2019.1 automation moved this from To review to Done Apr 22, 2019

Orbeon Forms 2018.2.3 automation moved this from To do to Done Apr 22, 2019

@ebruchez ebruchez added this to Todo in Orbeon Forms 2018.1.4 via automation Apr 22, 2019

@ebruchez ebruchez moved this from Todo to Done in Orbeon Forms 2018.1.4 Apr 22, 2019

@ebruchez

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 22, 2019

For reference, found this issue while working on #4011.

ebruchez added a commit that referenced this issue Apr 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.