Description
Nick Clemens
2021-10-20 12:46:03 UTC
Created attachment 126583 [details] [review] Bug 29284: Don't die on analytics searchign error This patch adds an eval around the call to search for analytic records It pases a value to the template on the staff side, but logs the warning on the opac This seems similar to 'decoding_error' which is noted on staff side, but absent on OPAC The eval follows the patter used during searching To test: 1 - Add a title to catalog, with 245a: Digger does it all (not really!) 2 - Set searchEngine preference to: Elasticsearch 3 - The record does not load 4 - Apply patch 5 - The record loads, there is a note about analytics at the top fo the record 6 - View record in opac, no note 7 - Check logs on intranet and opac, searching error is logged Created attachment 126584 [details] [review] Bug 29284: Handle the case of an exclamation point in parentheses This expands the regex to handle this specific case To test: 1 - Load record created for last patch 2 - Note analytics error 3 - Apply patch 4 - Restart and reload 5 - No more errorm also no Analytics link 6 - Add a 773$t to a record with title used before: Digger does it all (not really!) 7 - reload the initial record 8 - See 'Show analytics' link 9 - Click the link 10 - You should end up on the record you added the 773 to *** Bug 28682 has been marked as a duplicate of this bug. *** Created attachment 126586 [details] [review] Bug 29284: Don't die on analytics searchign error This patch adds an eval around the call to search for analytic records It pases a value to the template on the staff side, but logs the warning on the opac This seems similar to 'decoding_error' which is noted on staff side, but absent on OPAC The eval follows the patter used during searching To test: 1 - Add a title to catalog, with 245a: Digger does it all (not really!) 2 - Set searchEngine preference to: Elasticsearch 3 - The record does not load 4 - Apply patch 5 - The record loads, there is a note about analytics at the top fo the record 6 - View record in opac, no note 7 - Check logs on intranet and opac, searching error is logged Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126587 [details] [review] Bug 29284: Handle the case of an exclamation point in parentheses This expands the regex to handle this specific case To test: 1 - Load record created for last patch 2 - Note analytics error 3 - Apply patch 4 - Restart and reload 5 - No more errorm also no Analytics link 6 - Add a 773$t to a record with title used before: Digger does it all (not really!) 7 - reload the initial record 8 - See 'Show analytics' link 9 - Click the link 10 - You should end up on the record you added the 773 to Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Clear and concise patch that does what it says on the tin. QA scripts are happy and testing shows it all works as expected. Going straight for QA on this one as it's a major with a minimal complexity fix. PQA 1. Please improve the error handling [2021/10/21 08:08:21] [WARN] Use of uninitialized value $err in concatenation (.) or string at /kohadevbox/koha/catalogue/detail.pl line 150. + if ($err || $@){ + warn "Warning from simple_search_compat: $err.$@"; 2. Second patch needs tests. Created attachment 126656 [details] [review] Bug 29284: Unit test Created attachment 126657 [details] [review] Bug 29284: (follow-up) Improve error logging to prevent warns Created attachment 126658 [details] [review] Bug 29284: (follow-up) Fix code that I copied from too Pushed to master for 21.11, thanks to everybody involved! Pushed to 21.05.x for 21.05.05 Doesn't 20.11 need this too? Backported: Pushed to 20.11.x branch for 20.11.14 |