JSON does not allow real line-breaks. If a field contains them and they are not escaped, a JSON parser will be unable to convert the stringified JSON back into an object. This is clearly exemplified by the guarantor search, where a multi-line note on the guarantor's record will break the ability to select that guarantor.
Created attachment 69235 [details] [review] Bug 19655 - To.json doesn't escape newlines which can create invalid JSON JSON does not allow real line-breaks. If a field contains them and they are not escaped, a JSON parser will be unable to convert the stringified JSON back into an object. This is clearly exemplified by the guarantor search, where a multi-line note on the guarantor's record will break the ability to select that guarantor. Test Plan: 1) Create Patron A with a "Circulation note" that has multiple lines in it 2) Create Patron B 3) Attempt to set Patron A to be the guarantor for Patron B 4) Note selecting the patron does nothing 5) Apply this patch 6) Repeat step 3 7) Selecting the guarantor now works!
Created attachment 69244 [details] [review] Bug 19655 - To.json doesn't escape newlines which can create invalid JSON JSON does not allow real line-breaks. If a field contains them and they are not escaped, a JSON parser will be unable to convert the stringified JSON back into an object. This is clearly exemplified by the guarantor search, where a multi-line note on the guarantor's record will break the ability to select that guarantor. Test Plan: 1) Create Patron A with a "Circulation note" that has multiple lines in it 2) Create Patron B 3) Attempt to set Patron A to be the guarantor for Patron B 4) Note selecting the patron does nothing 5) Apply this patch 6) Repeat step 3 7) Selecting the guarantor now works! Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>
Created attachment 69326 [details] [review] Bug 19655 - To.json doesn't escape newlines which can create invalid JSON JSON does not allow real line-breaks. If a field contains them and they are not escaped, a JSON parser will be unable to convert the stringified JSON back into an object. This is clearly exemplified by the guarantor search, where a multi-line note on the guarantor's record will break the ability to select that guarantor. Test Plan: 1) Create Patron A with a "Circulation note" that has multiple lines in it 2) Create Patron B 3) Attempt to set Patron A to be the guarantor for Patron B 4) Note selecting the patron does nothing 5) Apply this patch 6) Repeat step 3 7) Selecting the guarantor now works! Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 69346 [details] [review] Bug 19655: Add tests
I am not sure it is the best way to fix it. 1. We may encounter issues later using To.json on other values that use escaped strings (ok we will update the code to make it works) 2. Maybe we should replace all \\ with \\\\ I will push as it and adapt later if needed.
Pushed to master for 17.11, thanks to everybody involved!
Poushed to 17.05.x, will be in v17.05.07
Pushed to 16.11.x will be in 16.11.15
(In reply to Chris Cormack from comment #8) > Pushed to 16.11.x will be in 16.11.15 This patch breaks tests: https://jenkins.koha-community.org/job/Koha_16.11_D8/89/ # Failed test 'No tests run for subtest "json"' # at t/db_dependent/Template/Plugin/To.t line 44. Can't locate object method "build_object" via package "t::lib::TestBuilder" at t/db_dependent/Template/Plugin/To.t line 32. # Looks like your test exited with 255 just after 1. [09:41:40] t/db_dependent/Template/Plugin/To.t The build_object method of TestBuilder has been added by bug 18182, which is not in 16.11.x You should use ->build instead.
Created attachment 69732 [details] [review] Bug 19655: Use ->build instead of build_object in 16.11.x The build_object method of TestBuilder has been added by bug 18182, which is not in 16.11.x Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 69806 [details] [review] Bug 19655: Use ->build instead of build_object in 16.11.x The build_object method of TestBuilder has been added by bug 18182, which is not in 16.11.x Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>