ucam-department-of-psychiatry / camcops

Cambridge Cognitive and Psychiatric Test Kit (CamCOPS)
Other
12 stars 8 forks source link

Make IDED-3D settings configurable for a patient's task schedule #319

Closed martinburchell closed 9 months ago

martinburchell commented 10 months ago

Support for IDED-3D settings as JSON in a patient's task schedule.

Also apply a task's settings every time it is edited, not just when the task is first created. This shouldn't actually be possible with IDED-3D but see #318

Fixes #314

RudolfCardinal commented 10 months ago

Thanks -- looks good. Could I just ask (only when convenient!) about your thinking about applying the settings every time, rather than at task creation? Generally, of course, these will be the same. I'm not sure whether it's possible for the settings to change after creation, though, but if they did, might that (in principle) make some tasks behave oddly? I know the ID-ED one is a bad example, since it shouldn't be editable anyway, but if you'd completed 7 stages and then the settings changed to make the maximum stage 5 (for example), that might be a bit unexpected in terms of the resulting data. Is there a situation when changes might be wanted like this?

RudolfCardinal commented 10 months ago

Also -- the settings are accepted, I think, if QuPage::mayProgressIgnoringValidators() returns true. Is there another check on JSON validity? I don't think so -- so, for example, if someone entered an invalid value (e.g. max_stage = 9), should we also check that the validators are happy (and bring up the questionnaire if not)?

RudolfCardinal commented 10 months ago

All else looks great. Thank you!

martinburchell commented 10 months ago

Thanks -- looks good. Could I just ask (only when convenient!) about your thinking about applying the settings every time, rather than at task creation? Generally, of course, these will be the same. I'm not sure whether it's possible for the settings to change after creation, though, but if they did, might that (in principle) make some tasks behave oddly? I know the ID-ED one is a bad example, since it shouldn't be editable anyway, but if you'd completed 7 stages and then the settings changed to make the maximum stage 5 (for example), that might be a bit unexpected in terms of the resulting data. Is there a situation when changes might be wanted like this?

I noticed that if I set up the settings for a patient and then edited an aborted task, the settings would be lost on the re-attempt and I'd just see the defaults. This was before I realised that we don't normally allow patients to edit this task and I raised #318

I could revert this aspect of the change if we were confident that tasks in the future won't need it.

martinburchell commented 10 months ago

Also -- the settings are accepted, I think, if QuPage::mayProgressIgnoringValidators() returns true. Is there another check on JSON validity? I don't think so -- so, for example, if someone entered an invalid value (e.g. max_stage = 9), should we also check that the validators are happy (and bring up the questionnaire if not)?

Ah, good point. I think I need to change validateQuestionnaire() to include the limits that the form fields would be enforcing.

martinburchell commented 10 months ago

I could revert this aspect of the change if we were confident that tasks in the future won't need it.

As discussed in person, I'll revert this change as any (future) editable tasks should be storing the settings along with the database fields for the task. This will prevent problems if the settings are changed mid-task completion.

martinburchell commented 9 months ago

@RudolfCardinal ready for another look. The JSON settings are now validated to make sure they are within the limits. If they fall outside they are removed and the settings form will appear.

RudolfCardinal commented 9 months ago

Looks good! Thank you!