|
Description
Fridolin Somers
2020-09-25 13:15:04 UTC
Created attachment 110756 [details] [review] Bug 26537: Fix search when no term in value_builder/unimarc_field_4XX.pl Cataloguing plugin value_builder/unimarc_field_4XX.pl has an itemtype filter. It does not work when used without a search term. Test plan : 1) Be sure search on itemtype return some results 2) Use cataloguing plugin unimarc_field_4XX.pl 3) Enter no search term and select an itemtype 4) Check you get correct results 5) Enter a search term and dont select an itemtype 6) Check you get correct results 7) Enter a search term and select an itemtype 8) Check you get correct results @frido: 7) and 8) don't works Searching a term + item type raises a software error. When searching term and item type, the ES query looks like this: "query":"(:jean and itemtype:LIVR)" My opinion is that we should use QueryBuilder to make the query. Counter patch is coming. Created attachment 111049 [details] [review] Bug 26537: use QueryBuilder in unimarc_field_4XX plugin to make query @frido, can you test the last patch ? Maybe rename $es_query into $search_query, it's not specific to ES - $to = $from + $resultsperpage ; + $to = $startfrom * $resultsperpage + $resultsperpage ; I dont understand this change. (In reply to Fridolin Somers from comment #7) > - $to = $from + $resultsperpage ; > + $to = $startfrom * $resultsperpage + $resultsperpage ; > I dont understand this change. This patch breaks paging of results. Created attachment 128930 [details] [review] Bug 26537: use QueryBuilder in unimarc_field_4XX plugin to make query Created attachment 128931 [details] [review] Bug 26537: use QueryBuilder in unimarc_field_4XX plugin to make query Created attachment 128932 [details] [review] Bug 26537: use QueryBuilder in unimarc_field_4XX plugin to make query Bug 26537: (follow-up) fix paging of results Created attachment 128933 [details] [review] Bug 26537: use QueryBuilder in unimarc_field_4XX plugin to make query Created attachment 128934 [details] [review] Bug 26537: (follow-up) fix paging of results Created attachment 128937 [details] [review] Bug 26537 : (follow-up) fix paging of results Created attachment 128938 [details] [review] Bug 26537 : (follow-up) fix paging of results Created attachment 129607 [details] [review] Bug 26537 : (follow-up) fix paging of results Also I renamed "es_query" variable for better understanding (this script works with Elastic search but also Zebra) Created attachment 129646 [details] [review] Bug 26537: use QueryBuilder in unimarc_field_4XX plugin to make query Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 129647 [details] [review] Bug 26537: (follow-up) fix paging of results Also I renamed "es_query" variable for better understanding (this script works with Elastic search but also Zebra) Signed-off-by: Owen Leonard <oleonard@myacpl.org> We should also create a bug report for value builder marc21_linking_section.pl no ? In fact also does not work because of lowercase boolean, see Bug 25616 The patch doesn't apply, I believe the fix is probably simple but don't want to mess with it: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 26537: use QueryBuilder in unimarc_field_4XX plugin to make query Using index info to reconstruct a base tree... M cataloguing/value_builder/unimarc_field_4XX.pl Falling back to patching base and 3-way merge... Auto-merging cataloguing/value_builder/unimarc_field_4XX.pl CONFLICT (content): Merge conflict in cataloguing/value_builder/unimarc_field_4XX.pl error: Failed to merge in the changes. Patch failed at 0001 Bug 26537: use QueryBuilder in unimarc_field_4XX plugin to make query hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-26537-use-QueryBuilder-in-unimarcfield4XX-plug-sBlXav.patch Created attachment 144858 [details] [review] Bug 26537: use QueryBuilder in unimarc_field_4XX plugin to make query Signed-off-by: Owen Leonard <oleonard@myacpl.org> Rebased on master: Thibaud Guillot <thibaud.guillot@biblibre.com> Created attachment 144859 [details] [review] Bug 26537: (follow-up) fix paging of results Also I renamed "es_query" variable for better understanding (this script works with Elastic search but also Zebra) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Test plan : 0) Use ES 1) Add a new record 2) Use cataloguing plugin unimarc_field_4XX.pl 3) Enter no search term and select an itemtype 4) Expect internal server error 5) Apply patch 6) retry 7) Expected no error but there was Did we do well testing this way and can this be added to the patch ? |