OpenClinica / enketo-express-oc

A fork of Enketo Smart Paper for OpenClinica
Apache License 2.0
5 stars 23 forks source link

Read-only form prompts user to leave without saving changes #358

Closed pbowen-oc closed 4 years ago

pbowen-oc commented 4 years ago

Steps:

  1. Open an existing record in read-only mode
  2. Close the form

Expected: The form closes without prompting the user.

Actual: The user is prompted to leave the form with unsaved data.

Annotation 2020-07-07 105717 Annotation 2020-07-07 105742

I think this is due to the unnecessary max-size call that fails when the form is first opened.

MartijnR commented 4 years ago

To the top of the list?

pbowen-oc commented 4 years ago

@MartijnR - Yes, thanks.

Also, I noticed that the form has the "Failed to save changes due to form loading error" message, which I assume is also due to the max-size call.

MartijnR commented 4 years ago
curl --user enketorules: -d "\
server_url=https://enketo-aggregate.appspot.com&\
form_id=vitals&\
instance=<demo-vitals-grid-v9><page2><media_yn>1</media_yn><pic>img2.jpg</pic><pic_comment>{\"queries\":[], \"logs\":[{\"type\":\"audit\", \"comment\":\"Value changed from \\\"img1.jpg\\\" to \\\"img2.jpg\\\"\", \"date_time\":\"2019-03-02 04:00:00 +06:00\"}]}</pic_comment></page2></demo-vitals-grid-v9>&\
instance_id=b&\
instance_attachments[img1.jpg]=https://github.com/enketo/enketo-express/raw/master/public/images/icon_180x180.png&\
instance_attachments[img2.jpg]=https://github.com/enketo/enketo-express/raw/master/public/images/offline-enabled.png&\
ecid=a\
" \
http://localhost:8005/oc/api/v1/instance/view
MartijnR commented 4 years ago

I cannot reproduce the issue.

@pbowen-oc, can you share the form and the instance (record) you are loading please?

Also can you confirm that this view doesn't have notes enabled (pure readonly)?

(P.S. I did see the failing max-size request and fixed that, but that's not related).

pbowen-oc commented 4 years ago

@MartijnR - I opened the form in the following mode - /view/fs/i/

I noticed that since the record was incomplete, there were several items that showed required or constraint errors if I clicked Close and then chose Cancel in that confirmation popup. Perhaps it is trying to add autoqueries and failing (though there is no sign of them in the network tab)?

readonlyform.xml.txt

Annotation 2020-07-07 162711

MartijnR commented 4 years ago

Thanks @pbowen-oc. Could you provide the record as well? (response from the GET /submission request in network tab)

MartijnR commented 4 years ago

Suspect that a similar issue could be caused by the static default value submissions for empty forms. Looks like they could add submissions, though those submissions would never be attempted to be submitted. This is not the case here though.

I think we also have a setvalue loophole that can change values in readonly views

pbowen-oc commented 4 years ago

@MartijnR - Here is the instance for that form: readonlyform.instance.txt

MartijnR commented 4 years ago

Thanks, I can reproduce it now.

curl --user enketorules: -d "\
server_url=http://localhost:3000&\
form_id=readonlyform&\
instance=<data xmlns:oc=\"http://openclinica.org/xforms\"  xmlns:enk=\"http://enketo.org/xforms\" ><page1><rndm>6</rndm><cohort>E</cohort><temp_yn>2</temp_yn><temp_calc></temp_calc><temp_group><temp></temp><temp_units></temp_units><temp_unit_des></temp_unit_des><temp_entry></temp_entry><temp_entry_hidden></temp_entry_hidden><temp_units_comment>{\"queries\":[],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"2\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:39:48.958 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"2\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:39:56.339 Z\"}]}</temp_units_comment><temp_unit_des_comment></temp_unit_des_comment><temp_entry_hidden_comment></temp_entry_hidden_comment><temp_comment>{\"queries\":[],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"98.5\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:39:48.747 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"98.5\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:39:59.271 Z\"}]}</temp_comment><temp_entry_comment>{\"queries\":[],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"temperature not entered\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:29:50.102 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"temperature not entered\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:29:58.789 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"temperature not entered\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:30:00.826 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"temperature not entered\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:30:21.383 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"temperature not entered\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:38:39.545 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"temperature not entered\\\" to \\\"98.5 ⁰F\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:39:49.369 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"98.5 ⁰F\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:39:54.605 Z\"}]}</temp_entry_comment></temp_group><wh_y>1</wh_y><wh_units>1</wh_units><weight>100</weight><weight_unit_des>100 kilograms</weight_unit_des><height>201</height><height_unit_des>201 centimeters</height_unit_des><bmi>24.75</bmi><bmi_hidden></bmi_hidden><bmi_calc>24.75</bmi_calc><bmi_rad></bmi_rad><resp_rate></resp_rate><pulse></pulse><dob></dob><age></age><url_item>https://openclinica.com</url_item><format1></format1><format2></format2><format3></format3><format4></format4><bmi_hidden_comment></bmi_hidden_comment><format2_comment></format2_comment><dob_comment></dob_comment><height_unit_des_comment></height_unit_des_comment><temp_yn_comment>{\"queries\":[],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"2\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-09 06:14:48.205 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"2\\\" to \\\"1\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:29:49.433 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"1\\\" to \\\"2\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:29:58.376 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"2\\\" to \\\"1\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:30:00.405 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"1\\\" to \\\"2\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:30:20.957 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"2\\\" to \\\"1\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:38:38.890 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"1\\\" to \\\"2\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:39:54.148 Z\"}]}</temp_yn_comment><rndm_comment></rndm_comment><format3_comment></format3_comment><url_item_comment>{\"queries\":[],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"https://openclinica.com\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-09 06:13:40.742 Z\"}]}</url_item_comment><temp_calc_comment></temp_calc_comment><wh_units_comment>{\"queries\":[],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"1\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:33:01.735 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"1\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:35:00.312 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"1\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:35:04.521 Z\"}]}</wh_units_comment><format4_comment></format4_comment><pulse_comment></pulse_comment><cohort_comment></cohort_comment><age_comment></age_comment><resp_rate_comment></resp_rate_comment><weight_comment>{\"queries\":[{\"type\":\"comment\",\"id\":\"1\",\"date_time\":\"2020-07-07 14:48:07.396 Z\",\"comment\":\"closing\",\"status\":\"closed\",\"assigned_to\":\"pbowenadm\",\"notify\":false,\"thread_id\":\"c9597ea1-5989-4f68-8b4d-7af15a095c72\",\"visible_thread_id\":\"10\",\"user\":\"pbowenadm\"},{\"type\":\"comment\",\"id\":\"2\",\"date_time\":\"2020-07-07 14:48:01.793 Z\",\"comment\":\"test1\",\"status\":\"new\",\"assigned_to\":\"pbowenadm\",\"notify\":false,\"thread_id\":\"c9597ea1-5989-4f68-8b4d-7af15a095c72\",\"visible_thread_id\":\"10\",\"user\":\"pbowenadm\"}],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"100\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:33:06.525 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"100\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:34:57.363 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"100\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:35:08.390 Z\"}]}</weight_comment><height_comment>{\"queries\":[{\"type\":\"comment\",\"id\":\"1\",\"date_time\":\"2020-07-07 14:48:28.664 Z\",\"comment\":\"Data changed after query was closed\",\"status\":\"closed-modified\",\"notify\":false,\"thread_id\":\"86b42d59-b286-40ef-998d-71b07314ac9f\",\"visible_thread_id\":\"11\",\"user\":\"root\"},{\"type\":\"comment\",\"id\":\"2\",\"date_time\":\"2020-07-07 14:48:20.404 Z\",\"comment\":\"closing\",\"status\":\"closed\",\"assigned_to\":\"pbowenadm\",\"notify\":false,\"thread_id\":\"86b42d59-b286-40ef-998d-71b07314ac9f\",\"visible_thread_id\":\"11\",\"user\":\"pbowenadm\"},{\"type\":\"comment\",\"id\":\"3\",\"date_time\":\"2020-07-07 14:48:15.182 Z\",\"comment\":\"test\",\"status\":\"new\",\"assigned_to\":\"pbowenadm\",\"notify\":false,\"thread_id\":\"86b42d59-b286-40ef-998d-71b07314ac9f\",\"visible_thread_id\":\"11\",\"user\":\"pbowenadm\"}],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"200\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:33:09.728 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"200\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:34:55.552 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"100\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:35:11.292 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"100\\\" to \\\"200\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-07-07 14:47:10.778 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"200\\\" to \\\"201\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-07-07 14:48:27.650 Z\"}]}</height_comment><format1_comment></format1_comment><wh_y_comment>{\"queries\":[],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"1\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:32:55.484 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"1\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:34:26.879 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"1\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:35:02.117 Z\"}]}</wh_y_comment><bmi_comment>{\"queries\":[{\"type\":\"comment\",\"id\":\"1\",\"date_time\":\"2020-06-15 18:35:27.491 Z\",\"comment\":\"The item has been removed, this Query has been Closed.\",\"status\":\"closed-modified\",\"assigned_to\":\"pgopalan\",\"notify\":false,\"thread_id\":\"f61511be-679c-4d2e-950a-c06fa70e5ade\",\"visible_thread_id\":\"1\",\"user\":\"pgopalan\"},{\"type\":\"comment\",\"id\":\"2\",\"date_time\":\"2020-06-10 19:35:25.116 Z\",\"comment\":\"Automatic query for: BMI should not be greater than 50.\",\"status\":\"new\",\"assigned_to\":\"pbowenadm\",\"notify\":false,\"thread_id\":\"f61511be-679c-4d2e-950a-c06fa70e5ade\",\"visible_thread_id\":\"1\",\"user\":\"root\"}],\"logs\":[{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"0\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:32:55.690 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"0\\\" to \\\"25\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:33:09.938 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"25\\\" to \\\"\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:34:27.085 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"\\\" to \\\"0\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:35:02.341 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"0\\\" to \\\"100\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-06-10 19:35:11.600 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"100\\\" to \\\"25\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-07-07 14:47:11.055 Z\"},{\"type\":\"audit\",\"message\":\"Value changed from \\\"25\\\" to \\\"24.75\\\"\",\"user\":\"pbowenadm\",\"date_time\":\"2020-07-07 14:48:27.886 Z\"}]}</bmi_comment><weight_unit_des_comment></weight_unit_des_comment><bmi_calc_comment></bmi_calc_comment><bmi_rad_comment></bmi_rad_comment></page1><page2><bp_yn></bp_yn><bp_rg enk:last-used-ordinal=\"1\"  enk:ordinal=\"1\" ><bp_rg_num_init></bp_rg_num_init><bp_rg_num></bp_rg_num><bp_row></bp_row><bp_pos></bp_pos><bp_sys></bp_sys><bp_sys_val></bp_sys_val><bp_sys_count></bp_sys_count><bp_dia></bp_dia><bp_dia_val></bp_dia_val><bp_dia_count></bp_dia_count><bp_ratio></bp_ratio><bp_ratio_hidden></bp_ratio_hidden><bp_dt_prec></bp_dt_prec><bp_dt_ymd></bp_dt_ymd><bp_dt_ym></bp_dt_ym><bp_dt_y></bp_dt_y><bp_dt></bp_dt><bp_dia_count_comment></bp_dia_count_comment><bp_row_comment></bp_row_comment><bp_dt_ym_comment></bp_dt_ym_comment><bp_dt_prec_comment></bp_dt_prec_comment><bp_sys_comment></bp_sys_comment><bp_ratio_hidden_comment></bp_ratio_hidden_comment><bp_sys_val_comment></bp_sys_val_comment><bp_dt_y_comment></bp_dt_y_comment><bp_rg_num_comment></bp_rg_num_comment><bp_rg_num_init_comment></bp_rg_num_init_comment><bp_pos_comment></bp_pos_comment><bp_dia_val_comment></bp_dia_val_comment><bp_ratio_comment></bp_ratio_comment><bp_dt_comment></bp_dt_comment><bp_sys_count_comment></bp_sys_count_comment><bp_dt_ymd_comment></bp_dt_ymd_comment><bp_dia_comment></bp_dia_comment></bp_rg><bp_sys_mean></bp_sys_mean><bp_sys_max></bp_sys_max><bp_sys_min></bp_sys_min><bp_sys_countfn></bp_sys_countfn><bp_sys_countnefn></bp_sys_countnefn><bp_dia_mean></bp_dia_mean><bp_dia_max></bp_dia_max><bp_dia_min></bp_dia_min><bp_dia_countfn></bp_dia_countfn><bp_dia_countnefn></bp_dia_countnefn><bp_sys_string></bp_sys_string><bp_dia_string></bp_dia_string><bp_number></bp_number><bp_ratio_lookup></bp_ratio_lookup><bp_rg_review_count></bp_rg_review_count><bp_rg_review enk:last-used-ordinal=\"1\"  enk:ordinal=\"1\" ><bp_review_pos></bp_review_pos><bp_review_sys></bp_review_sys><bp_review_dia></bp_review_dia><bp_review_dia_comment></bp_review_dia_comment><bp_review_sys_comment></bp_review_sys_comment><bp_review_pos_comment></bp_review_pos_comment></bp_rg_review><likert_group><systolic_agreement></systolic_agreement><diastolic_agreement></diastolic_agreement><diastolic_agreement_comment></diastolic_agreement_comment><systolic_agreement_comment></systolic_agreement_comment></likert_group><newselectitem></newselectitem><newdecimal></newdecimal><pain_level></pain_level><vas_question></vas_question><media_yn></media_yn><pic></pic><sound></sound><vid></vid><afile_pdf></afile_pdf><afile_any></afile_any><pic_ann></pic_ann><pic_draw></pic_draw><pic_sig></pic_sig><ecg_picture></ecg_picture><cascade1></cascade1><cascade2></cascade2><cascade3></cascade3><us_map></us_map><us_map_name></us_map_name><add_notes></add_notes><pic_comment></pic_comment><bp_number_comment></bp_number_comment><bp_yn_comment></bp_yn_comment><cascade1_comment></cascade1_comment><bp_sys_mean_comment></bp_sys_mean_comment><bp_sys_countfn_comment></bp_sys_countfn_comment><bp_sys_string_comment></bp_sys_string_comment><bp_dia_countfn_comment></bp_dia_countfn_comment><pic_ann_comment></pic_ann_comment><bp_sys_max_comment></bp_sys_max_comment><bp_rg_review_count_comment></bp_rg_review_count_comment><media_yn_comment></media_yn_comment><bp_sys_countnefn_comment></bp_sys_countnefn_comment><newdecimal_comment></newdecimal_comment><afile_pdf_comment></afile_pdf_comment><pain_level_comment></pain_level_comment><bp_dia_string_comment></bp_dia_string_comment><pic_sig_comment></pic_sig_comment><vid_comment></vid_comment><bp_dia_max_comment></bp_dia_max_comment><bp_ratio_lookup_comment></bp_ratio_lookup_comment><sound_comment></sound_comment><us_map_comment></us_map_comment><add_notes_comment></add_notes_comment><bp_sys_min_comment></bp_sys_min_comment><newselectitem_comment></newselectitem_comment><bp_dia_mean_comment></bp_dia_mean_comment><bp_dia_min_comment></bp_dia_min_comment><bp_dia_countnefn_comment></bp_dia_countnefn_comment><cascade3_comment></cascade3_comment><ecg_picture_comment></ecg_picture_comment><afile_any_comment></afile_any_comment><pic_draw_comment></pic_draw_comment><us_map_name_comment></us_map_name_comment><cascade2_comment></cascade2_comment><vas_question_comment></vas_question_comment></page2><meta><instanceID>uuid:1234</instanceID><instanceID_comment></instanceID_comment></meta></data>&\
instance_id=b&\
ecid=a\
" \
http://localhost:8005/oc/api/v1/instance/view
MartijnR commented 4 years ago

We don't have a distinction in saving feedback message (and logic!) between a disabled form that is readonly or a form that is disabled due to loading errors. For now I've changed:

"disabled": "Failed to save changes due to form loading error."

to a generic:

"disabled": "Saving is disabled."

Actually changing the logic, to display 2 different messages (or an empty string and a message) would be more work (but could be done of course).

Screen Shot 2020-07-09 at 2 58 06 PM
pbowen-oc commented 4 years ago

Looks good after testing.