OpenClinica/enketo-express-oc

When loading forms in View mode, use repeating groups from instance unless repeat_count is higher

Closed this issue · 5 comments

pbowen-oc commented

Is your feature request related to a problem? Please describe.
When an existing form record is opened in one of our view modes (query-only mode or read-only mode), the instance sometimes does not have a repeat_count value that is consistent with the repeating groups in the instance. In some cases, repeat_count may be undefined and in other cases it may be defined but lower than that the number of repeats in the instance. In those cases, Enketo attempts to delete excess repeating groups above the repeat_count value. These calls fail (since that action is not permitted in this modes) and the user is presented with a message for 'Failed to save changes" which causes confusion.

Describe the solution you'd like
When a form is opened in query-only mode or read-only mode, for any repeating group present on the form, use the greater of a) the repeat_count value for the repeating group or b) the repeating groups present in the instance. In this way, if repeat_count were lower than the repeats passed in through the instance, repeat_count would be effectively ignored. This would prevent Enketo from attempting to delete excess repeating groups.

For forms in edit mode, repeat_count functionality should remain unchanged (i.e., it should still set a limit for how many repeats are allowed regardless of the instance).

MartijnR commented
  • look into whether turning repeatModule.remove into a no-op does what is desired. - Not working for empty repeat count but works for lower repeat count
  • find out why an empty repeat count doesn't create any repeat instances in the view - it does actually create them correctly, but they are not visible.
  • find out why repeats are not visible in previous point - because it still has the pre-init class, something to do this the relevancy customization in OC's fork. - This seems to only occur if the repeat contains only readonly items (in the actual form definition) so that may be ok.

readonly view, repeat count 2, but 4 in record:

curl --user enketorules: -d "server_url=http://localhost:3000&form_id=repeatcount1&ecid=1&instance_id=aa&ecid=b&instance=\
    <data xmlns:OpenClinica=\"http://openclinica.com/odm\" xmlns:enk=\"http://enketo.org/xforms\" xmlns:jr=\"http://openrosa.org/javarosa\" xmlns:oc=\"http://openclinica.org/xforms\" xmlns:orx=\"http://openrosa.org/xforms\" id=\"repeatcount1\" version=\"1\">\n<num1>2</num1>\n<repeat1_count>2</repeat1_count>\n\n<repeat1 enk:last-used-ordinal=\"4\" enk:ordinal=\"1\">\n<int1>1</int1>\n<int2>2</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"2\">\n<int1>2</int1>\n<int2>4</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"3\">\n<int1>3</int1>\n<int2>6</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"4\">\n<int1>4</int1>\n<int2>8</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1>\n<meta>\n<instanceID>uuid:8310256f-173e-43cc-972e-9abc48ad03ef</instanceID>\n</meta>\n<num1_comment oc:queryParent=\"num1\"/>\n</data>"  \
    http://localhost:8005/oc/api/v1/instance/view/

query view, repeat count 2, but 4 in record:

curl --user enketorules: -d "server_url=http://localhost:3000&form_id=repeatcount1&ecid=1&instance_id=a&ecid=b&instance=\
    <data xmlns:OpenClinica=\"http://openclinica.com/odm\" xmlns:enk=\"http://enketo.org/xforms\" xmlns:jr=\"http://openrosa.org/javarosa\" xmlns:oc=\"http://openclinica.org/xforms\" xmlns:orx=\"http://openrosa.org/xforms\" id=\"repeatcount1\" version=\"1\">\n<num1>2</num1>\n<repeat1_count>2</repeat1_count>\n\n<repeat1 enk:last-used-ordinal=\"4\" enk:ordinal=\"1\">\n<int1>1</int1>\n<int2>2</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"2\">\n<int1>2</int1>\n<int2>4</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"3\">\n<int1>3</int1>\n<int2>6</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"4\">\n<int1>4</int1>\n<int2>8</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1>\n<meta>\n<instanceID>uuid:8310256f-173e-43cc-972e-9abc48ad03ef</instanceID>\n</meta>\n<num1_comment oc:queryParent=\"num1\"/>\n</data>"  \
    http://localhost:8005/oc/api/v1/instance/note/

readonly view, repeat count empty, but 4 in record:

curl --user enketorules: -d "server_url=http://localhost:3000&form_id=repeatcount1&ecid=1&instance_id=b&ecid=b&instance=\
    <data xmlns:OpenClinica=\"http://openclinica.com/odm\" xmlns:enk=\"http://enketo.org/xforms\" xmlns:jr=\"http://openrosa.org/javarosa\" xmlns:oc=\"http://openclinica.org/xforms\" xmlns:orx=\"http://openrosa.org/xforms\" id=\"repeatcount1\" version=\"1\">\n<num1></num1>\n<repeat1_count></repeat1_count>\n\n<repeat1 enk:last-used-ordinal=\"4\" enk:ordinal=\"1\">\n<int1>1</int1>\n<int2>2</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"2\">\n<int1>2</int1>\n<int2>4</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"3\">\n<int1>3</int1>\n<int2>6</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"4\">\n<int1>4</int1>\n<int2>8</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1>\n<meta>\n<instanceID>uuid:8310256f-173e-43cc-972e-9abc48ad03ef</instanceID>\n</meta>\n<num1_comment oc:queryParent=\"num1\"/>\n</data>"  \
    http://localhost:8005/oc/api/v1/instance/view/

query view, repeat count empty, but 4 in record:

curl --user enketorules: -d "server_url=http://localhost:3000&form_id=repeatcount1&ecid=1&instance_id=b&ecid=b&instance=\
    <data xmlns:OpenClinica=\"http://openclinica.com/odm\" xmlns:enk=\"http://enketo.org/xforms\" xmlns:jr=\"http://openrosa.org/javarosa\" xmlns:oc=\"http://openclinica.org/xforms\" xmlns:orx=\"http://openrosa.org/xforms\" id=\"repeatcount1\" version=\"1\">\n<num1></num1>\n<repeat1_count></repeat1_count>\n\n<repeat1 enk:last-used-ordinal=\"4\" enk:ordinal=\"1\">\n<int1>1</int1>\n<int2>2</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"2\">\n<int1>2</int1>\n<int2>4</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"3\">\n<int1>3</int1>\n<int2>6</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"4\">\n<int1>4</int1>\n<int2>8</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1>\n<meta>\n<instanceID>uuid:8310256f-173e-43cc-972e-9abc48ad03ef</instanceID>\n</meta>\n<num1_comment oc:queryParent=\"num1\"/>\n</data>"  \
    http://localhost:8005/oc/api/v1/instance/note/

regular edit view, repeat count 2 but 4 in record:

curl --user enketorules: -d "server_url=http://localhost:3000&form_id=repeatcount1&ecid=1&instance_id=c&ecid=b&instance=\
    <data xmlns:OpenClinica=\"http://openclinica.com/odm\" xmlns:enk=\"http://enketo.org/xforms\" xmlns:jr=\"http://openrosa.org/javarosa\" xmlns:oc=\"http://openclinica.org/xforms\" xmlns:orx=\"http://openrosa.org/xforms\" id=\"repeatcount1\" version=\"1\">\n<num1>2</num1>\n<repeat1_count>2</repeat1_count>\n\n<repeat1 enk:last-used-ordinal=\"4\" enk:ordinal=\"1\">\n<int1>1</int1>\n<int2>2</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"2\">\n<int1>2</int1>\n<int2>4</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"3\">\n<int1>3</int1>\n<int2>6</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1><repeat1 enk:ordinal=\"4\">\n<int1>4</int1>\n<int2>8</int2>\n<int1_comment oc:queryParent=\"int1\"/>\n<int2_comment oc:queryParent=\"int2\"/>\n</repeat1>\n<meta>\n<instanceID>uuid:8310256f-173e-43cc-972e-9abc48ad03ef</instanceID>\n</meta>\n<num1_comment oc:queryParent=\"num1\"/>\n</data>"  \
    http://localhost:8005/oc/api/v1/instance/edit/
pbowen-oc commented

@MartijnR - Did this solution happen to include PDF mode as well? I didn't call it out, but I see now that something quite similar is needed. PDF mode doesn't try to delete repeats (as far as I know), but a mismatched number of repeats coming in from the instance and as repeat count causes the PDF to display data incorrectly.

In the PDF case, we also want it to use the higher of repeat_count and actual repeat instances. Right now, we pass in deleted repeats for PDF mode (to show as part of the audit log) and this can cause the number of repeats passed in to exceed repeat_count. Right now, that means the number of repeats displayed for PDF mode is truncated at repeat_count. So, we need PDF mode to use the higher value for display purposes like with view mode.

If this is covered now and you want a separate ticket logged for it, let me know.

pbowen-oc commented

@MartijnR - I tested this in PDF view using master and it seems like it is already working as I requested. Can you confirm that it is implemented for the PDF case?

MartijnR commented

@pbowen-oc, I checked and can confirm this.

pbowen-oc commented

Looks good in testing