Skip to content

Catch conditional variable refs and flag duplicate block ids#42

Open
rajeswari1301 wants to merge 3 commits into
mainfrom
check-attachment-refs
Open

Catch conditional variable refs and flag duplicate block ids#42
rajeswari1301 wants to merge 3 commits into
mainfrom
check-attachment-refs

Conversation

@rajeswari1301
Copy link
Copy Markdown
Contributor

Extends conditional variable checking to catch more places where interviews can get stuck at runtime. Previously the checker only looked at interview-order code blocks. This adds the same check to question:, subquestion:, and attachment:/attachments: content blocks -if a variable is only conditionally asked (show if/hide if) but unconditionally referenced in any of these, the checker now flags it.

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

Also adds duplicate id: detection - if two blocks share the same id, DA silently uses the later one which is almost always a mistake. The checker now flags this as a real error with the line number of the first use.

Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Collaborator

@BryceStevenWilley BryceStevenWilley left a comment

Choose a reason for hiding this comment

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

For the duplicate id: +1, I like it.

For the extensions to conditional vars: it's a good idea in theory, but I'm less sure it's useful in practice.

An example error that this branch flags:

mandatory: True
code: |
    ...
    if has_evidence:
       review_exhibits
     ocr_task  
    ...
---
question: |
  You are required to submit copies of relevant Trial Court documents. 
fields:
  - Do you have any documents to share with the judge?: has_evidence
    datatype: yesnoradio
  - Choose all of the documents you want to upload: exhibits
    datatype: files
    show if: has_evidence
    ....
---
id: review exhibits
question: |
  Review your exhibits
subquestion: |
  Look at the exhibits below. If you want to change the order, use the arrows
  to move an exhibit up or down in the list. Click "delete" or "add another"
  to change which exhibits you send to the court.
  
  ${ exhibits.table }
  
  ${exhibits.add_action() } 

There are no errors with this code, and it's fairly standard for conditional questions to do things like this. It is assumed that review_exhibits is connected to exhibits existing as an object, sure, but I can't think of a more clear or concise way of writing this code. Idk.

Added @nonprofittechy as a reviewer, I feel like he'll have some good thoughts on if this is useful for folks or not.

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