Skip to content
This repository has been archived by the owner on Nov 29, 2023. It is now read-only.

[Fix] Ensure nested repeats are shown when relevance is true #807

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eyelidlessness
Copy link
Contributor

Fixes enketo/enketo#103. This is branched off of master, unlike #806 which we should likely close.

Some of these tests are likely redundant, but it was hard for me to be sure, so I felt more comfortable adding them.

@MartijnR
Copy link
Member

Thanks for tackling a nested repeat issue! I am committing to doing a review.

@MartijnR MartijnR self-requested a review August 31, 2021 13:50
const repeatInfo = getChild( node, `.or-repeat-info[data-name="${p.path}"]` );

if ( repeatInfo != null && !getChild( node, `.or-repeat[name="${p.path}"]` ) ) {
const count = this.form.repeats.updateViewInstancesFromModel( repeatInfo );
Copy link
Member

@MartijnR MartijnR Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the underlying issue is that the first instance of the nested repeat is not created when the top-level repeat (no. 2) is created (after the user clicks the plus button).

The relevant module should not be responsible for fixing that, as it's the responsibility of the repeat module. It's odd that something so elementary is not done correctly. I wonder if this bug was introduced when we started allowing 0 repeat instances. There seems to be some (still mysterious) special condition in the test form, because in e.g. http://localhost:8005/?xform=nested_repeats.xml the bug doesn't surface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hesitant to apply this fix here too, I'm glad you agree. To be honest I chose this path because I struggled to understand the repeat initialization flow and wasn't sure how to address it there. But I'll take another look because I agree it would be better solved there.

@lognaturel lognaturel marked this pull request as draft April 21, 2022 17:25
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.

No relevance evaluated on 2nd iteration of a nested repeat
2 participants