Skip to content

Add check for conditional variables in attachment content (EG416)#48

Merged
nonprofittechy merged 6 commits into
mainfrom
check-attachment-refs-v2
Jun 4, 2026
Merged

Add check for conditional variables in attachment content (EG416)#48
nonprofittechy merged 6 commits into
mainfrom
check-attachment-refs-v2

Conversation

@rajeswari1301
Copy link
Copy Markdown
Contributor

Adds a check for attachment content blocks that reference variables which are only conditionally asked (show if/hide if). If the condition isn't met at runtime, docassemble tries to render the variable, can't find it, and the interview gets stuck.

Respects Mako % if guards so correctly guarded references don't get flagged.

rajeswari1301 and others added 3 commits June 3, 2026 00:59
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Member

@nonprofittechy nonprofittechy left a comment

Choose a reason for hiding this comment

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

Really close, but our template interviews all use the attachment-specific skip undefined modifier instead of the global feature, so we should support both in this rule

@nonprofittechy
Copy link
Copy Markdown
Member

Also can't quite tell what the test error is. Is it just an inverted assertion?

@rajeswari1301
Copy link
Copy Markdown
Contributor Author

Also can't quite tell what the test error is. Is it just an inverted assertion?

the 3 failing tests are in test_yaml_structure_cli.py. They were already broken on main before this PR, looks like the tests weren't updated after some recent output format and exit code changes.

@nonprofittechy nonprofittechy merged commit d41dd96 into main Jun 4, 2026
4 checks passed
@nonprofittechy nonprofittechy deleted the check-attachment-refs-v2 branch June 4, 2026 14:23
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.

2 participants