XLSForm / pyxform

A Python package to create XForms for ODK Collect.
BSD 2-Clause "Simplified" License
77 stars 134 forks source link

Warn when there's a calculate on a visible, user-editable field #546

Open lognaturel opened 2 years ago

lognaturel commented 2 years ago

Software and hardware versions

All versions

Problem description

Form designers often put calculates on user-visible, user-editable fields as an attempt to have a dynamic default. As far as I know, this is never desirable because the calculation will be re-run on form save, etc, meaning that if the user changes the value, the user-specified value will be lost. The one exception I can think of is using once or coalesce/if to only modify the value if it is blank. The documented way to do dynamic defaults is using the trigger column: docs.

Steps to reproduce the problem

| survey |                    |         |         |                |
|        | type               | name    | label   | calculation    |
|        | select_one city    | select  | City    |                |
|        | select_one country | select  | Country | france         |
| choices|                    |         |         |                |
|        | list_name          | name    | label   |                |
|        | city               | paris   | Paris   |                |
|        | city               | peridot | Péridot |                |
|        | country            | france  | France  |                |
|        | country            | usa     | USA     |                |

Expected behavior

For each field with:

The user should see a warning with the field name, the line number and a message like This question can be edited by users but its value will be replaced by the calculation on form save. If you want to display the calculated value, make the question readonly. To define a dynamic default, see https://docs.getodk.org/form-logic/#dynamic-defaults-from-form-data

lognaturel commented 2 years ago

Any objection to doing this? It's a pretty common issue/misconception so it would be relatively high value. I didn't put it on the forum because I doubt most folks will understand the implications but perhaps I should for consistency of process.

I could even be convinced to make this an error. I don't believe it ever makes sense.

Pinging @MartijnR in case you want to give it some thought.

MartijnR commented 2 years ago

I think this would be a very useful addition. Thanks for creating the issue, and checking the existing of the wrapper (once, coalesce, if) probably takes care of all possible objections.

Making it an error would make sense to me.

IsmaelInRedlands commented 2 years ago

There is a significant large number of Survey123 forms that use calculations for the purpose of dynamic defaults. Flagging this as an error would be devastating. For context, the Survey123 implementation does not trigger the calculation expression on values that the user has previously changed. Not even when the form is saved.