feat(#833): error when forms reference CSV external datasets - #839
Open
yasumorishima wants to merge 1 commit into
Open
yasumorishima wants to merge 1 commit into
yasumorishima wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a form validation that fails when a form references an external dataset stored in a CSV file.
Since
select_*_from_filesupport landed (medic/cht-core#10971), only XML datasets are supported. A form whose secondary instance usessrc="jr://file-csv/..."uploads and loads without any error, but the question simply has no data. This catches that before the form reaches the server.src/lib/validation/form/no-csv-external-datasets.js: errors for everymodel/instancenode whosesrcstarts withjr://file-csv/.requiresInstance: falseandskipFurtherValidation: false, as suggested in the issue. All offending sources are listed in a single message, with a link to the docs.test/lib/validation/form/no-csv-external-datasets.spec.js: 7 unit tests (no external data, XML source, one CSV source, several CSV sources, mixed XML/CSV, ajr://file-csv-backup/...near match, and a whitespace paddedsrc).test/lib/validate-forms.spec.js: added the new validation to the expected load order.No documentation change looked necessary: the docs already state that only XML files are supported. Note this is a behaviour change for existing configs that reference a CSV dataset - they will now fail validation instead of uploading a form that quietly shows no choices.
AI Disclosure: this PR was written primarily with Claude Code. It explored the codebase, drafted the validation following the existing ones and wrote the tests. I reviewed the result, verified the error message against the docs, and confirmed lint, unit tests (Node 20/22/24) and the e2e suite pass.
#833
Code review items
License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.