diff --git a/src/pipelines/ddi2xlsform/README.md b/src/pipelines/ddi2xlsform/README.md index 0f0f9b1..c5f6a4f 100644 --- a/src/pipelines/ddi2xlsform/README.md +++ b/src/pipelines/ddi2xlsform/README.md @@ -67,6 +67,9 @@ out): `begin group` / `end group` as `begin_group` / `end_group`, a boolean setting as its text (`true`), a setting's `{ lang: text }` cell as one `::` column per language. +3. **A repair**: a `guidance_hint=` inside `parameters` + (qwacback's old export wrote it) comes back as the `guidance_hint` column, + since pyxform rejects it there. One without spaces stays where it was. ## Tests diff --git a/src/utils/parameters.ts b/src/utils/parameters.ts index ac7c535..e7d2c56 100644 --- a/src/utils/parameters.ts +++ b/src/utils/parameters.ts @@ -11,3 +11,24 @@ export function parseParameters(cell: unknown): Record { } return out; } + +/** + * `parameters` without a `guidance_hint=` pyxform rejects: one whose + * text has whitespace, which qwacback's old DDI → XLSForm export wrote + * (`guidance_hint=Only once`). pyxform reads the cell as space-separated + * `key=value` pairs, so the hint belongs in the `guidance_hint` column. + * `moved` says whether one was taken out. + */ +export function withoutSpacedGuidance(parameters: string): { + parameters: string; + moved: boolean; +} { + const parts = parameters.split(';'); + const kept = parts.filter((part) => { + const m = /^\s*guidance_hint\s*=(.*)$/.exec(part); + return !m || !/\s/.test(m[1].trim()); + }); + return kept.length === parts.length + ? { parameters, moved: false } + : { parameters: kept.join(';').trim(), moved: true }; +} diff --git a/src/xlsform/fromInstrument.ts b/src/xlsform/fromInstrument.ts index 07b1948..5804cf9 100644 --- a/src/xlsform/fromInstrument.ts +++ b/src/xlsform/fromInstrument.ts @@ -31,6 +31,7 @@ import type { } from '../instrument/types.js'; import { htmlToMarkdown } from '../utils/markdownRenderer.js'; import { languageTagOf } from '../utils/languageUtils.js'; +import { withoutSpacedGuidance } from '../utils/parameters.js'; type Row = Record; @@ -270,10 +271,13 @@ function questionRow(q: QuestionItem, ctx: EmitCtx): SurveyRow { name: q.name, label: htmlLabel(ctx.label(q.label)), }; + // A guidance_hint inside `parameters` stays there, unless pyxform would + // reject it (text with spaces): then it is the guidance_hint column. + const { parameters, moved } = withoutSpacedGuidance(q.parameters); + const inParameters = !moved && /(^|;)\s*guidance_hint\s*=/.test(parameters); const texts: Array<[string, Text]> = [ ['hint', q.hint], - // guidance_hint kept inside `parameters` stays there. - ...(/(^|;)\s*guidance_hint\s*=/.test(q.parameters) + ...(inParameters ? [] : [['guidance_hint', q.guidanceHint] as [string, Text]]), ['constraint_message', q.constraintMessage], @@ -286,7 +290,7 @@ function questionRow(q: QuestionItem, ctx: EmitCtx): SurveyRow { ['required', q.required ? requiredCell(q) : ''], ['default', q.default], ['appearance', appearanceCell(q)], - ['parameters', q.parameters], + ['parameters', parameters], ['relevant', q.relevant], ['constraint', q.constraint], ]; diff --git a/tests/fixtures/surveys/hints_survey/ddi2xlsform.json b/tests/fixtures/surveys/hints_survey/ddi2xlsform.json index 2ddeac7..099fec9 100644 --- a/tests/fixtures/surveys/hints_survey/ddi2xlsform.json +++ b/tests/fixtures/surveys/hints_survey/ddi2xlsform.json @@ -22,7 +22,7 @@ "type": "select_one janein", "name": "ehrenamt", "label": "Sind Sie ehrenamtlich tätig?", - "parameters": "guidance_hint=Auch gelegentliches Engagement zählt." + "guidance_hint": "Auch gelegentliches Engagement zählt." }, { "type": "select_multiple medien", diff --git a/tests/ts/contract/canonicalInstrument.ts b/tests/ts/contract/canonicalInstrument.ts index 8b2dbc4..89df968 100644 --- a/tests/ts/contract/canonicalInstrument.ts +++ b/tests/ts/contract/canonicalInstrument.ts @@ -10,6 +10,7 @@ import { isMetadataType } from '../../../src/conventions/metadata.js'; import { OTHER_CODE } from '../../../src/conventions/other.js'; import { isExclusive } from '../../../src/conventions/exclusive.js'; import { allColumns } from '../../../src/conventions/columns.js'; +import { withoutSpacedGuidance } from '../../../src/utils/parameters.js'; import { languageTagOf } from '../../../src/utils/languageUtils.js'; import type { Instrument, @@ -85,7 +86,8 @@ function question(q: QuestionItem, ctx: Ctx): Canon { required: required(q), default: q.default, appearance: appearance(q), - parameters: q.parameters.trim(), + // A guidance_hint pyxform rejects in parameters is moved to its column. + parameters: withoutSpacedGuidance(q.parameters.trim()).parameters, columns: q.columns ?? {}, }; } diff --git a/tests/ts/contract/ddiRoundtrip.test.ts b/tests/ts/contract/ddiRoundtrip.test.ts index b82fd28..ca09235 100644 --- a/tests/ts/contract/ddiRoundtrip.test.ts +++ b/tests/ts/contract/ddiRoundtrip.test.ts @@ -15,6 +15,7 @@ import { buildDdiXml } from '../../../src/pipelines/xlsform2ddi/index.js'; import { instrumentFromDdi } from '../../../src/instrument/fromDdi.js'; import { instrumentFromXlsform } from '../../../src/instrument/fromXlsform.js'; import { ddiToXlsform } from '../../../src/pipelines/ddi2xlsform/index.js'; +import { withoutSpacedGuidance } from '../../../src/utils/parameters.js'; import { canonical } from './canonicalInstrument.js'; import { cases, ROOT } from './roundtripCases.js'; @@ -54,19 +55,33 @@ describe('XLSForm → DDI → XLSForm sheets', () => { }); }); +/** A form ddi2xlsform repairs: a guidance_hint pyxform rejects in parameters. */ +const repaired = (c: { survey: Record[] }) => + c.survey.some( + (r) => + withoutSpacedGuidance( + typeof r['parameters'] === 'string' ? r['parameters'] : '', + ).moved, + ); + describe('DDI → XLSForm → DDI gives the same codebook', () => { + const again = (xml: string) => { + const sheets = ddiToXlsform(xml); + return buildDdiXml(sheets.survey, sheets.choices, { + prodDate: '2020-01-01', + settings: sheets.settings[0] ?? {}, + }); + }; + test.each(CASES)('$name', (c) => { const xml = buildDdiXml(c.survey, c.choices, { prodDate: '2020-01-01', settings: c.settings[0] ?? {}, }); - const sheets = ddiToXlsform(xml); - expect( - buildDdiXml(sheets.survey, sheets.choices, { - prodDate: '2020-01-01', - settings: sheets.settings[0] ?? {}, - }), - ).toBe(xml); + const once = again(xml); + // A repaired form's codebook changes once, then stays. + expect(once === xml).toBe(!repaired(c)); + expect(again(once)).toBe(once); }); }); diff --git a/tests/ts/unit/ddi/fromDdi.test.ts b/tests/ts/unit/ddi/fromDdi.test.ts index 1321bc3..6f0e3f3 100644 --- a/tests/ts/unit/ddi/fromDdi.test.ts +++ b/tests/ts/unit/ddi/fromDdi.test.ts @@ -290,3 +290,29 @@ describe('the last cells (#160)', () => { }); }); }); + +describe('a guidance_hint in parameters (#160)', () => { + const back = (parameters: string) => + ddiToXlsform( + buildDdiXml([{ type: 'text', name: 't', label: 'T', parameters }], [], { + prodDate: '2020-01-01', + }), + ).survey[0]; + + test('stays there when pyxform accepts it', () => { + expect(back('guidance_hint=Once')).toMatchObject({ + parameters: 'guidance_hint=Once', + }); + expect(back('guidance_hint=Once')).not.toHaveProperty('guidance_hint'); + }); + + test('is its column when its text has spaces, which pyxform rejects', () => { + expect(back('randomize=true; guidance_hint=Only once')).toMatchObject({ + parameters: 'randomize=true', + guidance_hint: 'Only once', + }); + const alone = back('guidance_hint=Only once'); + expect(alone).toMatchObject({ guidance_hint: 'Only once' }); + expect(alone).not.toHaveProperty('parameters'); + }); +}); diff --git a/tests/validation/test_xlsform_pyxform.py b/tests/validation/test_xlsform_pyxform.py index cebfb38..514b37f 100644 --- a/tests/validation/test_xlsform_pyxform.py +++ b/tests/validation/test_xlsform_pyxform.py @@ -137,25 +137,11 @@ def test_survey_fixture_is_valid_xlsform(survey_dir: Path, tmp_path: Path) -> No _BACK = [d for d in _SURVEYS if (d / "ddi2xlsform.json").exists()] -@pytest.mark.parametrize( - "survey_dir", - [ - pytest.param( - d, - marks=pytest.mark.xfail( - strict=True, - reason=f"gives back its source's cells (#160), which pyxform rejects: {_PYXFORM_INVALID[d.name]}", - ), - ) - if d.name in _PYXFORM_INVALID - else d - for d in _BACK - ], - ids=[d.name for d in _BACK], -) +@pytest.mark.parametrize("survey_dir", _BACK, ids=[d.name for d in _BACK]) def test_ddi2xlsform_output_is_valid_xlsform(survey_dir: Path, tmp_path: Path) -> None: """The form ddi2xlsform gives back from each blessed codebook is valid - XLSForm when its source is: qwacback exports it to Kobo.""" + XLSForm, even where its source isn't (a guidance_hint with spaces in + parameters goes to its column): qwacback exports it to Kobo.""" for csv in (REPO_ROOT / "registry" / "vocab").glob("*.csv"): shutil.copy(csv, tmp_path / csv.name) xlsx = _json_xlsx(survey_dir / "ddi2xlsform.json", tmp_path / f"{survey_dir.name}.xlsx")