Bug 36879 - Spurious warnings in QueryBuilder
Summary: Spurious warnings in QueryBuilder
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial
Assignee: Andreas Jonsson
QA Contact: Marcel de Rooy
URL:
Keywords:
: 37433 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-05-16 07:41 UTC by Andreas Jonsson
Modified: 2024-09-11 16:04 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the cause of a warning message in the log files. Changing the sort order for search results in the staff interface (for example, from Relevance to Author (A-Z)) would generate an unnecessary warning message in plack-intranet-error.log: [WARN] Use of uninitialized value $f in hash element at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm line 72 5.
Version(s) released in:
24.11.00,24.05.04
Circulation function:


Attachments
Bug 36879: Spurious warning in QueryBuilder (1.16 KB, patch)
2024-05-16 07:53 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 36879: Spurious warning in QueryBuilder (1.20 KB, patch)
2024-08-01 20:13 UTC, David Nind
Details | Diff | Splinter Review
Bug 36879: Spurious warning in QueryBuilder (1.30 KB, patch)
2024-08-02 09:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Jonsson 2024-05-16 07:41:16 UTC
Spurious warning about undefined variables when processing sort order.
Comment 1 Andreas Jonsson 2024-05-16 07:53:31 UTC
Created attachment 166807 [details] [review]
Bug 36879: Spurious warning in QueryBuilder

Test plan:

With ElasticSearch enabled,
* Perform a search using the default  sort order
  (i.e. 'relevance').
* Verify that no warnings are generated in
  plack-intranet-error.log
Comment 2 David Nind 2024-08-01 14:29:08 UTC
I think this bug should have the "Needs Signoff" status (I think it was accidentally set as "Needs documenting").

However, I can't seem to change it 8-(...

I tested it, but couldn't change the status that way either with a git bz attach -e 36879 HEAD

Testing notes (using KTD):
1. Start up KTD with Elasticsearch: ktd --es8 up
2. Go to the staff interface and search the catalog - I searched for perl
3. Change the sort order on the results page from Relevance to Author(A-Z)
4. Change the sort order on the results page from Author (A-Z) to relevance
5. Note that you have these warnings in the /var/log/koha/kohadev/plack-intranet-error.log logfile:
    [2024/08/01 14:15:24] [WARN] Use of uninitialized value $f in hash element at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm line 72    5.
    [2024/08/01 14:15:24] [WARN] Use of uninitialized value $d in hash element at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm line 72    5.
6. Apply the patch.
7. Restart everything: restart_all
8. Repeat steps 2-4:
   Result: No warnings in the log file!
Comment 3 David Nind 2024-08-01 20:13:08 UTC
Created attachment 169964 [details] [review]
Bug 36879: Spurious warning in QueryBuilder

Test plan:

With ElasticSearch enabled,
* Perform a search using the default  sort order
  (i.e. 'relevance').
* Verify that no warnings are generated in
  plack-intranet-error.log

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-08-01 20:21:39 UTC
I updated the status from "Needs documenting" to "Needs Signoff" by changing the status a few times:
1. RESOLVED FIXED
2. REOPENED
3. Needs Signoff

(Thanks to the tips in the Community Chat from Katrin!)
Comment 5 Marcel de Rooy 2024-08-02 09:37:27 UTC
Created attachment 169996 [details] [review]
Bug 36879: Spurious warning in QueryBuilder

Test plan:

With ElasticSearch enabled,
* Perform a search using the default  sort order
  (i.e. 'relevance').
* Verify that no warnings are generated in
  plack-intranet-error.log

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Katrin Fischer 2024-08-02 15:44:03 UTC
Please don't forget to tidy your code. Amended patch.
Comment 7 Marcel de Rooy 2024-08-02 16:00:07 UTC
(In reply to Katrin Fischer from comment #6)
> Please don't forget to tidy your code. Amended patch.

Why does my qa tools no longer pick that up ?
Comment 8 Katrin Fischer 2024-08-02 16:02:43 UTC
(In reply to Marcel de Rooy from comment #7)
> (In reply to Katrin Fischer from comment #6)
> > Please don't forget to tidy your code. Amended patch.
> 
> Why does my qa tools no longer pick that up ?

I am not sure, I reset my ktd yesterday, so it should all be current. It showed 3 untidy lines for me here and was OK after I perltidied the section.
Comment 9 Katrin Fischer 2024-08-02 17:01:23 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 10 Janusz Kaczmarek 2024-09-09 12:55:50 UTC
*** Bug 37433 has been marked as a duplicate of this bug. ***
Comment 11 Lucas Gass 2024-09-11 16:04:05 UTC
Backported to 24.05.x for upcoming 24.05.04