Bug 37953 - Incorrect handling of DisplayLibraryFacets in previous database update 23.12.000.36
Summary: Incorrect handling of DisplayLibraryFacets in previous database update 23.12....
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords: rel_24_05_candidate
Depends on: 35138
Blocks:
  Show dependency treegraph
 
Reported: 2024-09-18 08:46 UTC by Jonathan Druart
Modified: 2024-10-11 09:15 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 37953: (bug 35138 follow-up) Fix DisplayLibraryFacets - facet order (3.00 KB, patch)
2024-09-18 08:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37953: (bug 35138 follow-up) Fix DisplayLibraryFacets - facet order (3.06 KB, patch)
2024-09-19 12:25 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2024-09-18 08:46:39 UTC
As noted on bug 37446 the test on DisplayLibraryFacets is wrong in installer/data/mysql/db_revs/231200036.pl

We need to deal with the facet_order instead of the label of the facet.
Comment 1 Jonathan Druart 2024-09-18 08:53:21 UTC
Created attachment 171656 [details] [review]
Bug 37953: (bug 35138 follow-up) Fix DisplayLibraryFacets - facet order

Before bug 35138:
DisplayLibraryFacets had priority over the settings in the mappings page.
If you had DisplayLibraryFacets set to "holding library" (only) then only the holding library facet was displayed.

On 231200036 we were not adjusting the facet order/appearance, but the
label, which was wrong. The label must be adjusted for both facets and
the order depending on DisplayLibraryFacets.
Note that bug 37446 is adding a new db rev to adjust the label for
installations that have been updated already.

Test plan:
0. checkout c120975b783 (the commit right before 35138 has been pushed)
1. reset_all
2. holding library is set in DisplayLibraryFacets by default
3. checkout main (so without this patch)
4. updatedatabase
=> ES search result display both facets

5. Repeat 0,1,2 and checkout main + this patch, updatedatabase
=> ES search result display only the holding library facet

Feel free to try with another value in DisplayLibraryFacets
Comment 2 Nick Clemens (kidclamp) 2024-09-19 12:25:25 UTC
Created attachment 171716 [details] [review]
Bug 37953: (bug 35138 follow-up) Fix DisplayLibraryFacets - facet order

Before bug 35138:
DisplayLibraryFacets had priority over the settings in the mappings page.
If you had DisplayLibraryFacets set to "holding library" (only) then only the holding library facet was displayed.

On 231200036 we were not adjusting the facet order/appearance, but the
label, which was wrong. The label must be adjusted for both facets and
the order depending on DisplayLibraryFacets.
Note that bug 37446 is adding a new db rev to adjust the label for
installations that have been updated already.

Test plan:
0. checkout c120975b783 (the commit right before 35138 has been pushed)
1. reset_all
2. holding library is set in DisplayLibraryFacets by default
3. checkout main (so without this patch)
4. updatedatabase
=> ES search result display both facets

5. Repeat 0,1,2 and checkout main + this patch, updatedatabase
=> ES search result display only the holding library facet

Feel free to try with another value in DisplayLibraryFacets

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 3 Nick Clemens (kidclamp) 2024-09-19 12:27:40 UTC
I cheated and tested with:
perl -e 'use C4::Installer; my $db= C4::Installer::run_db_rev( "installer/data/mysql/db_revs/231200036.pl" ); warn Data::Dumper::Dumper($db->{output})'

I added both homelibrary and holdinglibrary facets back in after each test. Tested with all settings of DisplayLibraryFacets


For libraries who have already upgraded, they will get both facets, but this will prevent issues for libraries who have not yet upgraded.


Small patch, moving to PQA for RM/RMaint testing
Comment 4 Katrin Fischer 2024-10-11 09:15:38 UTC
Pushed for 24.11!

Well done everyone, thank you!