Let the request form capture every objectType it offers - #115
Merged
Merged
Conversation
The dropdown lists eight kinds; the form could complete four. The schema requires a field per kind and the form asked for two of them: MultiValuedDataElement itemType missing MeasurementUnit symbol missing MeasurementUnit crossReferences.ucumCode missing Quantity dimension missing Value value missing So a requester could pick MeasurementUnit, fill in everything the form asked for, and still have produced a request that cannot be drafted. The form's own header promises it mirrors the envelope "so triage can generate the draft YAML mechanically"; for half the kinds it could not. ucumCode is the sharpest case, since check 2 validates its syntax — a guess fails the publish PR rather than degrading quietly. Added as one group behind a note saying only the row matching your objectType applies. All stay optional: GitHub issue forms cannot express "required when that dropdown says X", so the condition lives in the label and description. Left out uneceCommonCode, qudtUnit and conversions. Real unit entries carry them, but they are registry lookups a maintainer does better than a requester, and none is schema-enforced. Found while checking whether legalBasis had reached the form. It had, in #78's own commit — these had not, which is the second time the form has trailed the schema, so it becomes a check rather than a thing to remember. The test walks the schema's allOf conditionals and asserts each required property has an input to capture it, and separately that no input names a field the envelope lacks. Verified by renaming ucum-code in the form and watching both directions fail with the drift named. Closes #114
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.
Summary
The
dictionary-requestdropdown offers eightobjectTypes. The form could complete four.schema/dictionary-entry.schema.jsonrequires a field per kind; the form asked for two of them:SingleValuedDataElement/MultiLanguageDataElement/RelatedResourcevalueDataTypeDataElementCollectionelementsMultiValuedDataElementitemTypeMeasurementUnitsymbolMeasurementUnitcrossReferences.ucumCodeQuantitydimensionValuevalueA requester could pick
MeasurementUnit, fill in everything asked of them, and still submit a request that cannot be drafted. The form's header promises it mirrors the envelope "so triage can generate the draft YAML mechanically" — for half the kinds it could not.ucumCodeis the sharpest case: check 2 validates its syntax, so a guess fails the publish PR rather than degrading quietly.Change
The five missing inputs, as one group behind a note saying only the row matching your
objectTypeapplies. All stayrequired: false— GitHub issue forms cannot express "required when that dropdown says X" — so the condition lives in each label and description.ucumCode's help text warns that UCUM is case-sensitive and spells some units unexpectedly (Cel,[psi],mm2), and tells the requester to leave it blank rather than guess.Left out
uneceCommonCode,qudtUnitandconversions. Real unit entries carry them, but they are registry lookups a maintainer does better than a requester, and none is schema-enforced. Drafting fills them.Ratchet
This is the second time the form has trailed the schema, so it becomes a check rather than a thing to remember.
test/request-form.test.tswalks the schema'sallOfconditionals and asserts every conditionally-required property has an input to capture it — and, in reverse, that no input names a field the envelope lacks, since that produces a value with nowhere to go.Adding a kind-specific requirement to the schema now fails the suite until the form can ask for it.
Closes #114
Test plan
npm test— 82/82ucum-codein the form and confirmed both directions fail, naming the gap —MeasurementUnit requires crossReferences.ucumCode — no form input "ucum-code"— then restored and re-ran green. A guard that has only ever passed proves nothing.crossReferencesis a$refinto$defs, so the reverse check was reading an empty property set. Now resolves the ref, with an assertion that it did.new-entryskill's field list updated to match, so drafting knows to look for the new blockNote
Found while answering "did the form adopt the legal reference?" — it had, in #78's own commit (
d10ca65), with an ELI placeholder and wording that separates it fromdefinitionStandard/testStandard. These five had not.