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

Put setvalue action for repeats in the body #431

Merged
merged 4 commits into from
Feb 17, 2020

Conversation

lognaturel
Copy link
Contributor

@lognaturel lognaturel commented Feb 13, 2020

Closes #427

This fixes the regression introduced by 266ef19. The regression was because I didn't understand what the in_binding parameter did and because there was no test coverage for the desired behavior so I added tests and did some refactoring.

While doing this, I realized that dynamic defaults in groups or repeats nested within repeats weren't being handled as expected. c24e42c adds test coverage and a fix for this.

@lognaturel lognaturel changed the title Issue 427 Put setvalue action for repeats in the body Feb 13, 2020
@codecov-io
Copy link

codecov-io commented Feb 13, 2020

Codecov Report

Merging #431 into master will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #431      +/-   ##
==========================================
+ Coverage   82.72%   82.81%   +0.09%     
==========================================
  Files          23       23              
  Lines        3386     3399      +13     
  Branches      786      790       +4     
==========================================
+ Hits         2801     2815      +14     
  Misses        439      439              
+ Partials      146      145       -1
Impacted Files Coverage Δ
pyxform/section.py 96.55% <100%> (+0.47%) ⬆️
pyxform/survey_element.py 90.08% <100%> (+0.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8dfd800...c24e42c. Read the comment docs.

result.append(dynamic_default)

# dynamic defaults for repeats go in the body. All other dynamic defaults (setvalue actions) go in the model
if (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was beautiful before black got its hands on it.

| | date | date | Date | now() |
| | integer | foo | Foo | |
| | begin repeat | inner | | |
| | integer | bar | Bar | ${foo} |
Copy link
Contributor Author

@lognaturel lognaturel Feb 14, 2020

Choose a reason for hiding this comment

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

This illustrates the problem with always having a first instance of a repeat that I described at #182 (comment)

@yanokwa yanokwa merged commit 5eddece into XLSForm:master Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setvalue action for dynamic default in repeat should be in repeat form control
3 participants