If we're going to add it for elasticsearch, it ought to also be there for zebra ;)
Created attachment 61581 [details] [review] Add a facet for ccode fields to Zebra
This patch also fixes a couple minor bugs (id of collections facet title and dom element for facet titles) from the aforementioned elastic bug ;)
(In reply to Martin Renvoize from comment #0) > If we're going to add it for elasticsearch, it ought to also be there for > zebra ;) Hope that will not become a rule...
This is missing the index definition on the zebra files.
(In reply to Tomás Cohen Arazi from comment #4) > This is missing the index definition on the zebra files. And the UNIMARC part of it (if UNIMARC even has that field mapped)
(In reply to Jonathan Druart from comment #3) > (In reply to Martin Renvoize from comment #0) > > If we're going to add it for elasticsearch, it ought to also be there for > > zebra ;) > > Hope that will not become a rule... As is happens we were about to submit this patch anyway when the elastic version got into master. I'd prefer to see the configurable facets system in elastic; having this sort of thing hard coded is not very nice, but if we are hard coding then I feel the two search systems should stay roughly in sync to prevent a maintenance and support nightmare.
(In reply to Tomás Cohen Arazi from comment #4) > This is missing the index definition on the zebra files. The definition is already present in the zebra files.
Created attachment 61597 [details] [review] [QA Followup] Add a facet for ccode fields to Zebra (UNIMARC)
Added UNIMARC defs as requested.
Created attachment 61612 [details] [review] Furher UNIMarc Followup
I expected to see something like: <facet tag="952" subfields="8"> <target_index>ccode:0</target_index> </facet> in biblio-koha-indexdefs.xml for MARC21 and NORMARC (and the corresponding field/subfield for UNIMARC). And then have the biblio-zebra-indexdefs.xsl file get re-generated.
Created attachment 61643 [details] [review] Add configuration for zebra based faceting
Thanks for that pointer Tomas.. zebra facets vs non-zebra facets.. We had all our test systems set to non-zebra
Magnus, I wanted to check whether you also wanted this in the NORMARC configs?
(In reply to Martin Renvoize from comment #14) > Magnus, I wanted to check whether you also wanted this in the NORMARC > configs? Sure, no objections from me!
Created attachment 61657 [details] [review] Add a facet for ccode fields to Zebra This patch adds the index definitions for zebra faceting of ccode in koha for marc21, normarc and unimarc. We also add lines to the templates to expose the new facet and enable non-zebra faceting for ccode too.
Squashed patches. This patch now covers adding the ccode facet for marc21, normarc and unimarc in zebra and non-zebra faceting.
Test plan, ensure you have some collection codes defined in koha, and assigned to some materials. 1) Set zebra facets to 0 and do a search for a material with said facets. You should see the 'Collections' facet appear (both opac and staff are catered for in this patch). 2) Set zebra facets to 1 and reindex, then search for a material with said facets. You should see the 'Collections' facet appear (both opac and staff are catered for in this patch).
*** Bug 19435 has been marked as a duplicate of this bug. ***
Created attachment 67823 [details] [review] Add a facet for ccode fields to Zebra This patch adds the index definitions for zebra faceting of ccode in koha for marc21, normarc and unimarc. We also add lines to the templates to expose the new facet and enable non-zebra faceting for ccode too. https://bugs.koha-community.org/show_bug.cgi?id=18322 Signed-off-by: David Cook <dcook@prosentient.com.au>
Awesome job, Martin. I've been meaning to write this patch for a while, so it's great to see that you've already done all the hard work!
I was staring at it for a quite a while to figure it out, but we are missing the generated .xsl files here. We have the .xml files, but not the ones generated by running the xsltproc process. Please complete the patch! xsltproc /etc/koha/zebradb/xsl/koha-indexdefs-to-zebra.xsl /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-koha-indexdefs.xml >/etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl
Created attachment 75997 [details] [review] Bug 18322: (followup) Add generated xsl
Created attachment 76007 [details] [review] Bug 18322: Add a facet for ccode fields to Zebra This patch adds the index definitions for zebra faceting of ccode in koha for marc21, normarc and unimarc. We also add lines to the templates to expose the new facet and enable non-zebra faceting for ccode too. Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 76008 [details] [review] Bug 18322: (followup) Add generated xsl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 76009 [details] [review] Bug 18322: Update xslt for NORMARC and UNIMARC Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Almost there. The result looks funny because it is showing codes instead of descriptions. Take a look at C4::Search:570 and you will understand.
C4::Search:584 is a closer example.
Created attachment 76021 [details] [review] Bug 18322: (followup) Use authorized value labels Rather than using the raw ccode's use the labels as defined in the authorized values.
Good spot, thanks Tomás!
Created attachment 76023 [details] [review] Bug 18322: Add a facet for ccode fields to Zebra This patch adds the index definitions for zebra faceting of ccode in koha for marc21, normarc and unimarc. We also add lines to the templates to expose the new facet and enable non-zebra faceting for ccode too. Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 76024 [details] [review] Bug 18322: (follow-up) Add generated xsl Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 76025 [details] [review] Bug 18322: Update xslt for NORMARC and UNIMARC Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 76026 [details] [review] Bug 18322: (follow-up) Use authorized value labels Rather than using the raw ccode's use the labels as defined in the authorized values. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Awesome work all! Pushed to master for 18.11
Enhancement, will not be backported to 18.05.x series.