From e470f4569dfc55d934edae11a03c7bdf98b4dde6 Mon Sep 17 00:00:00 2001 From: jstet Date: Wed, 4 Nov 2026 20:57:26 +0100 Subject: [PATCH] fix(lstsv): an optional select_multiple with a lower bound can be left empty (#217) LimeSurvey enforces min_answers as `count(...) >= n` also on 0 ticks (em_manager_helper.php), while ODK skips the constraint on an empty answer. On a question that is not required the lower bound now goes into the guarded validation equation (`count(...) == 0 or (count(...) >= n)`) instead of min_answers; a bound of 1 or less only excludes the empty answer and is dropped. max_answers stays an attribute. A required question keeps min_answers. lstsv2xlsform drops the guard and reads the equation back as the same constraint. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 ++- registry/conventions/logicMapping.jsonld | 4 +- src/generated/conventions.json | 4 +- src/generated/conventions.ts | 4 +- src/pipelines/xlsform2lstsv/index.ts | 21 +++++-- tests/ts/unit/countSelected.test.ts | 78 +++++++++++++++++++++--- 6 files changed, 98 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 8c15ffd..dd016ce 100644 --- a/README.md +++ b/README.md @@ -383,8 +383,12 @@ becomes `is_empty(self) or (self >= 0 and self <= 100)` (on a `select_multiple`, `count() == 0 or (…)`), and an empty answer passes on both platforms. A required question's equation is left as it is. `lstsv2xlsform` drops the guard again (`convention:logicMapping`, -`emptyAnswer`). `min_answers` is not guarded: LimeSurvey also enforces it on -an empty multiple choice. +`emptyAnswer`). LimeSurvey also enforces `min_answers` on an empty multiple +choice, so on an optional `select_multiple` the lower bound is not written as +`min_answers` but into the guarded equation +(`count(…) == 0 or (count(…) >= 2)`); `max_answers` stays an attribute. A +lower bound of 1 or less is dropped there, since it only excludes the empty +answer. The reverse paths read the equation back as the same constraint. `coalesce(${a}, 0)` (the answer, or `0` when there is none) becomes `if(is_empty(a), 0, a)`, and back. diff --git a/registry/conventions/logicMapping.jsonld b/registry/conventions/logicMapping.jsonld index 35da75d..c82547c 100644 --- a/registry/conventions/logicMapping.jsonld +++ b/registry/conventions/logicMapping.jsonld @@ -144,14 +144,14 @@ "constraintBounds": { "minAttribute": "min_answers", "maxAttribute": "max_answers", - "rule": "On a select_multiple, each top-level `and` term of the constraint that compares count-selected(.) with a number becomes a question attribute instead of validation: `<= n` max_answers=n, `< n` max_answers=n-1, `>= n` min_answers=n, `> n` min_answers=n+1, `= n` both (the number on the left mirrors the operator). The other terms stay the validation equation. A count-selected(.) anywhere else (inside `or`, `not()`) stays in the equation as count().", + "rule": "On a select_multiple, each top-level `and` term of the constraint that compares count-selected(.) with a number becomes a question attribute instead of validation: `<= n` max_answers=n, `< n` max_answers=n-1, `>= n` min_answers=n, `> n` min_answers=n+1, `= n` both (the number on the left mirrors the operator). The other terms stay the validation equation. On a question that is not required the lower bound is the exception (see emptyAnswer): LimeSurvey enforces min_answers on an empty answer too, so it stays in the guarded validation equation as `count(…) >= n` (a bound of 1 or less, which only excludes the empty answer, is dropped). A count-selected(.) anywhere else (inside `or`, `not()`) stays in the equation as count().", "reverse": "A LimeSurvey multiple choice's min_answers/max_answers come back as `count-selected(.) >= n` / `count-selected(.) <= n`, ANDed before the reversed validation equation." } }, "emptyAnswer": { "guard": "is_empty(self)", "selectMultipleGuard": "count-selected(.) = 0", - "rule": "ODK evaluates a constraint only on an answered question; LimeSurvey evaluates em_validation_q on an empty answer too, so an optional question with a constraint could not be left empty. On a question that is not required, the converted validation equation is therefore written as `is_empty(self) or ()`. On a select_multiple, `self` stands for all its subquestions, so the guard is selectMultipleGuard, i.e. `count(_.NAOK, …) == 0 or ()`. A required question is mandatory in LimeSurvey, which rejects an empty answer before the equation matters, so its equation is not guarded. The guard covers the equation only: LimeSurvey enforces min_answers on an empty multiple choice too (see selectionCount).", + "rule": "ODK evaluates a constraint only on an answered question; LimeSurvey evaluates em_validation_q on an empty answer too, so an optional question with a constraint could not be left empty. On a question that is not required, the converted validation equation is therefore written as `is_empty(self) or ()`. On a select_multiple, `self` stands for all its subquestions, so the guard is selectMultipleGuard, i.e. `count(_.NAOK, …) == 0 or ()`. A required question is mandatory in LimeSurvey, which rejects an empty answer before the equation matters, so its equation is not guarded. LimeSurvey's min_answers is not guarded (its equation is `count(…) >= n`, also on 0 ticks), so on an optional select_multiple the lower bound is written into the guarded equation instead of min_answers (see selectionCount); max_answers stays an attribute, as 0 ticks satisfy it.", "reverse": "lstsv2xlsform drops a top-level ` or (…)` and reads back only the equation inside it. Under ODK's rule the guard makes no difference, so XLSForm → LimeSurvey → XLSForm keeps the constraint as authored." }, "unsupportedFunctionBehavior": "error — the transpiler throws on any XPath function outside supportedXPathFunctions", diff --git a/src/generated/conventions.json b/src/generated/conventions.json index 275f8f2..0c426dc 100644 --- a/src/generated/conventions.json +++ b/src/generated/conventions.json @@ -772,14 +772,14 @@ "constraintBounds": { "minAttribute": "min_answers", "maxAttribute": "max_answers", - "rule": "On a select_multiple, each top-level `and` term of the constraint that compares count-selected(.) with a number becomes a question attribute instead of validation: `<= n` max_answers=n, `< n` max_answers=n-1, `>= n` min_answers=n, `> n` min_answers=n+1, `= n` both (the number on the left mirrors the operator). The other terms stay the validation equation. A count-selected(.) anywhere else (inside `or`, `not()`) stays in the equation as count().", + "rule": "On a select_multiple, each top-level `and` term of the constraint that compares count-selected(.) with a number becomes a question attribute instead of validation: `<= n` max_answers=n, `< n` max_answers=n-1, `>= n` min_answers=n, `> n` min_answers=n+1, `= n` both (the number on the left mirrors the operator). The other terms stay the validation equation. On a question that is not required the lower bound is the exception (see emptyAnswer): LimeSurvey enforces min_answers on an empty answer too, so it stays in the guarded validation equation as `count(…) >= n` (a bound of 1 or less, which only excludes the empty answer, is dropped). A count-selected(.) anywhere else (inside `or`, `not()`) stays in the equation as count().", "reverse": "A LimeSurvey multiple choice's min_answers/max_answers come back as `count-selected(.) >= n` / `count-selected(.) <= n`, ANDed before the reversed validation equation." } }, "emptyAnswer": { "guard": "is_empty(self)", "selectMultipleGuard": "count-selected(.) = 0", - "rule": "ODK evaluates a constraint only on an answered question; LimeSurvey evaluates em_validation_q on an empty answer too, so an optional question with a constraint could not be left empty. On a question that is not required, the converted validation equation is therefore written as `is_empty(self) or ()`. On a select_multiple, `self` stands for all its subquestions, so the guard is selectMultipleGuard, i.e. `count(_.NAOK, …) == 0 or ()`. A required question is mandatory in LimeSurvey, which rejects an empty answer before the equation matters, so its equation is not guarded. The guard covers the equation only: LimeSurvey enforces min_answers on an empty multiple choice too (see selectionCount).", + "rule": "ODK evaluates a constraint only on an answered question; LimeSurvey evaluates em_validation_q on an empty answer too, so an optional question with a constraint could not be left empty. On a question that is not required, the converted validation equation is therefore written as `is_empty(self) or ()`. On a select_multiple, `self` stands for all its subquestions, so the guard is selectMultipleGuard, i.e. `count(_.NAOK, …) == 0 or ()`. A required question is mandatory in LimeSurvey, which rejects an empty answer before the equation matters, so its equation is not guarded. LimeSurvey's min_answers is not guarded (its equation is `count(…) >= n`, also on 0 ticks), so on an optional select_multiple the lower bound is written into the guarded equation instead of min_answers (see selectionCount); max_answers stays an attribute, as 0 ticks satisfy it.", "reverse": "lstsv2xlsform drops a top-level ` or (…)` and reads back only the equation inside it. Under ODK's rule the guard makes no difference, so XLSForm → LimeSurvey → XLSForm keeps the constraint as authored." }, "unsupportedFunctionBehavior": "error — the transpiler throws on any XPath function outside supportedXPathFunctions", diff --git a/src/generated/conventions.ts b/src/generated/conventions.ts index a1cafc2..053c1af 100644 --- a/src/generated/conventions.ts +++ b/src/generated/conventions.ts @@ -781,14 +781,14 @@ const conventions = { "constraintBounds": { "minAttribute": "min_answers", "maxAttribute": "max_answers", - "rule": "On a select_multiple, each top-level `and` term of the constraint that compares count-selected(.) with a number becomes a question attribute instead of validation: `<= n` max_answers=n, `< n` max_answers=n-1, `>= n` min_answers=n, `> n` min_answers=n+1, `= n` both (the number on the left mirrors the operator). The other terms stay the validation equation. A count-selected(.) anywhere else (inside `or`, `not()`) stays in the equation as count().", + "rule": "On a select_multiple, each top-level `and` term of the constraint that compares count-selected(.) with a number becomes a question attribute instead of validation: `<= n` max_answers=n, `< n` max_answers=n-1, `>= n` min_answers=n, `> n` min_answers=n+1, `= n` both (the number on the left mirrors the operator). The other terms stay the validation equation. On a question that is not required the lower bound is the exception (see emptyAnswer): LimeSurvey enforces min_answers on an empty answer too, so it stays in the guarded validation equation as `count(…) >= n` (a bound of 1 or less, which only excludes the empty answer, is dropped). A count-selected(.) anywhere else (inside `or`, `not()`) stays in the equation as count().", "reverse": "A LimeSurvey multiple choice's min_answers/max_answers come back as `count-selected(.) >= n` / `count-selected(.) <= n`, ANDed before the reversed validation equation." } }, "emptyAnswer": { "guard": "is_empty(self)", "selectMultipleGuard": "count-selected(.) = 0", - "rule": "ODK evaluates a constraint only on an answered question; LimeSurvey evaluates em_validation_q on an empty answer too, so an optional question with a constraint could not be left empty. On a question that is not required, the converted validation equation is therefore written as `is_empty(self) or ()`. On a select_multiple, `self` stands for all its subquestions, so the guard is selectMultipleGuard, i.e. `count(_.NAOK, …) == 0 or ()`. A required question is mandatory in LimeSurvey, which rejects an empty answer before the equation matters, so its equation is not guarded. The guard covers the equation only: LimeSurvey enforces min_answers on an empty multiple choice too (see selectionCount).", + "rule": "ODK evaluates a constraint only on an answered question; LimeSurvey evaluates em_validation_q on an empty answer too, so an optional question with a constraint could not be left empty. On a question that is not required, the converted validation equation is therefore written as `is_empty(self) or ()`. On a select_multiple, `self` stands for all its subquestions, so the guard is selectMultipleGuard, i.e. `count(_.NAOK, …) == 0 or ()`. A required question is mandatory in LimeSurvey, which rejects an empty answer before the equation matters, so its equation is not guarded. LimeSurvey's min_answers is not guarded (its equation is `count(…) >= n`, also on 0 ticks), so on an optional select_multiple the lower bound is written into the guarded equation instead of min_answers (see selectionCount); max_answers stays an attribute, as 0 ticks satisfy it.", "reverse": "lstsv2xlsform drops a top-level ` or (…)` and reads back only the equation inside it. Under ODK's rule the guard makes no difference, so XLSForm → LimeSurvey → XLSForm keeps the constraint as authored." }, "unsupportedFunctionBehavior": "error — the transpiler throws on any XPath function outside supportedXPathFunctions", diff --git a/src/pipelines/xlsform2lstsv/index.ts b/src/pipelines/xlsform2lstsv/index.ts index e48c574..db3d2c7 100644 --- a/src/pipelines/xlsform2lstsv/index.ts +++ b/src/pipelines/xlsform2lstsv/index.ts @@ -779,19 +779,32 @@ class Conversion { } const { validation, min, max } = this.transpilerHelper.convertSelectionConstraint(q.constraint, self); + // LimeSurvey enforces min_answers on an empty multiple choice too (its + // equation is `count(...) >= n`, unguarded), so an optional question keeps + // its lower bound in the guarded validation equation instead (a bound of + // 1 or less excludes nothing but the empty answer, so it is dropped). + const lowerInEquation = !q.required && min !== undefined && min > 1; + let equation = validation; + if (lowerInEquation) { + const lower = this.transpilerHelper.convertConstraint( + `count-selected(.) >= ${min}`, + self, + ); + equation = validation ? `${lower} and (${validation})` : lower; + } return { emValidation: - q.required || !validation - ? validation + q.required || !equation + ? equation : guardEmpty( - validation, + equation, this.transpilerHelper.convertConstraint( SELECT_MULTIPLE_EMPTY_GUARD, self, ), ), answerBounds: { - ...(min !== undefined && min > 0 + ...(q.required && min !== undefined && min > 0 ? { [MIN_ANSWERS_ATTRIBUTE]: String(min) } : {}), ...(max !== undefined ? { [MAX_ANSWERS_ATTRIBUTE]: String(max) } : {}), diff --git a/tests/ts/unit/countSelected.test.ts b/tests/ts/unit/countSelected.test.ts index 6493537..20438c1 100644 --- a/tests/ts/unit/countSelected.test.ts +++ b/tests/ts/unit/countSelected.test.ts @@ -2,7 +2,9 @@ * convention:logicMapping `selectionCount`: `count-selected()` (#185, #186). * In an expression it counts a select_multiple's ticked subquestions; a * constraint bounding it on the question itself becomes `min_answers` / - * `max_answers`. + * `max_answers` (#217: on an optional question the lower bound stays in the + * guarded validation equation, because LimeSurvey enforces `min_answers` on an + * empty answer too). */ import { describe, expect, test } from 'vitest'; import fc from 'fast-check'; @@ -19,8 +21,14 @@ const choices = [ ]; /** "Tick all that apply, then pick the most important" (AES DIFFTYPE/DIFFMAIN). */ -const pickMain = (constraint = '') => [ - { type: 'select_multiple m', name: 'k04', label: 'Which?', constraint }, +const pickMain = (constraint = '', required = '') => [ + { + type: 'select_multiple m', + name: 'k04', + label: 'Which?', + constraint, + required, + }, { type: 'select_one m', name: 'k04haupt', @@ -110,7 +118,7 @@ describe('count-selected(.) constraint → min_answers/max_answers (#186)', () = ['count-selected(.) >= 1 and count-selected(.) <= 3', '1', '3'], ['count-selected(${k04}) <= 2', undefined, '2'], ])('%s', async (constraint, min, max) => { - const { rows, warnings } = await convert(pickMain(constraint)); + const { rows, warnings } = await convert(pickMain(constraint, 'yes')); const row = q(rows, 'k04'); expect(row.min_answers || undefined).toBe(min); expect(row.max_answers || undefined).toBe(max); @@ -118,6 +126,40 @@ describe('count-selected(.) constraint → min_answers/max_answers (#186)', () = expect(warnings.filter((w) => w.code === 'constraint-dropped')).toEqual([]); }); + const COUNT = 'count(k04_a.NAOK, k04_b.NAOK, k04_c.NAOK)'; + + test.each([ + ['count-selected(.) >= 2', undefined, `${COUNT} == 0 or (${COUNT} >= 2)`], + ['count-selected(.) > 1', undefined, `${COUNT} == 0 or (${COUNT} >= 2)`], + ['count-selected(.) = 2', '2', `${COUNT} == 0 or (${COUNT} >= 2)`], + [ + 'count-selected(.) >= 2 and count-selected(.) <= 3', + '3', + `${COUNT} == 0 or (${COUNT} >= 2)`, + ], + // at least one tick only excludes the empty answer, which stays allowed + ['count-selected(.) >= 1', undefined, ''], + ['count-selected(.) <= 3', '3', ''], + ])( + 'optional: %s keeps the lower bound out of min_answers (#217)', + async (constraint, max, validation) => { + const { rows } = await convert(pickMain(constraint)); + const row = q(rows, 'k04'); + expect(row.min_answers ?? '').toBe(''); + expect(row.max_answers || undefined).toBe(max); + expect(row.em_validation_q).toBe(validation); + }, + ); + + test('optional: a lower bound and another term share the guard (#217)', async () => { + const { rows } = await convert( + pickMain("count-selected(.) >= 2 and not(selected(., 'c'))"), + ); + expect(q(rows, 'k04').em_validation_q).toBe( + `${COUNT} == 0 or (${COUNT} >= 2 and (!((self=="c"))))`, + ); + }); + test('the other terms stay the validation equation', async () => { const { rows } = await convert( pickMain("count-selected(.) <= 2 and not(selected(., 'c'))"), @@ -169,9 +211,24 @@ describe('the way back (lstsv2xlsform)', () => { expect(k04?.constraint).toBe('not(count-selected(.) < 2)'); }); + test('optional: the lower bound in the guarded equation comes back as the constraint (#217)', async () => { + const back = await roundTrip( + pickMain('count-selected(.) >= 2 and count-selected(.) <= 3'), + ); + const k04 = back.find((r) => r.name === 'k04'); + expect(k04?.constraint).toBe( + 'count-selected(.) <= 3 and count-selected(.) >= 2', + ); + // ...and converts to the same TSV again + const again = await roundTrip(pickMain(String(k04?.constraint))); + expect(again.find((r) => r.name === 'k04')?.constraint).toBe( + k04?.constraint, + ); + }); + test('min_answers/max_answers come back as a count-selected(.) constraint', async () => { const back = await roundTrip( - pickMain('count-selected(.) >= 1 and count-selected(.) <= 3'), + pickMain('count-selected(.) >= 1 and count-selected(.) <= 3', 'yes'), ); const k04 = back.find((r) => r.name === 'k04'); expect(k04?.constraint).toBe( @@ -216,6 +273,7 @@ describe('property: count bounds survive XLSForm → LimeSurvey → XLSForm', () fc.asyncProperty( fc.array(term, { minLength: 1, maxLength: 3 }), async (terms) => { + // required: the bounds are LimeSurvey attributes const constraint = terms .map(({ op, n, flipped }) => flipped @@ -223,19 +281,20 @@ describe('property: count bounds survive XLSForm → LimeSurvey → XLSForm', () : `count-selected(.) ${op} ${n}`, ) .join(' and '); - const { rows } = await convert(pickMain(constraint)); + const { rows } = await convert(pickMain(constraint, 'yes')); const row = q(rows, 'k04'); expect(row.em_validation_q).toBe(''); const min = Number(row.min_answers || 0); const max = row.max_answers ? Number(row.max_answers) : Infinity; - // An empty answer (0 ticks) is the "not answered" case for both ODK - // and LimeSurvey; compare the counts a respondent can tick. + // Required, so 0 ticks is rejected by mandatory on both platforms; + // compare the counts a respondent can tick. (On an optional question + // LimeSurvey would enforce min_answers on 0 ticks too: #217.) for (let k = 1; k <= 4; k++) { expect(k >= min && k <= max).toBe(allows(terms, k)); } // ...and the way back gives the same bounds again. const tsv = await new XLSFormToTSVConverter().convert( - pickMain(constraint), + pickMain(constraint, 'yes'), choices, [], ); @@ -243,6 +302,7 @@ describe('property: count bounds survive XLSForm → LimeSurvey → XLSForm', () const again = await convert( pickMain( String(back.find((r) => r.name === 'k04')?.constraint ?? ''), + 'yes', ), ); expect(q(again.rows, 'k04').min_answers ?? '').toBe(