Bug 18322

Summary: Add facets for ccode to zebra
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: SearchingAssignee: Martin Renvoize <martin.renvoize>
Status: RESOLVED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: brendan, claire_gravely, dcook, fiona.borthwick, jonathan.druart, katrin.fischer, magnus, marjorie.barry-vila, nick, rkuiper, severine.queune, testopia, tomascohen
Version: master   
Hardware: All   
OS: All   
Whiteboard:
Change sponsored?: Sponsored Patch complexity: ---
Bot Control: --- When did the bot last check this:
Who signed the patch off: Text to go in the release notes:
Bug Depends on: 17169, 18068    
Bug Blocks: 21095    
Attachments: Add a facet for ccode fields to Zebra
[QA Followup] Add a facet for ccode fields to Zebra (UNIMARC)
Furher UNIMarc Followup
Add configuration for zebra based faceting
Add a facet for ccode fields to Zebra
Add a facet for ccode fields to Zebra
Bug 18322: (followup) Add generated xsl
Bug 18322: Add a facet for ccode fields to Zebra
Bug 18322: (followup) Add generated xsl
Bug 18322: Update xslt for NORMARC and UNIMARC
Bug 18322: (followup) Use authorized value labels
Bug 18322: Add a facet for ccode fields to Zebra
Bug 18322: (follow-up) Add generated xsl
Bug 18322: Update xslt for NORMARC and UNIMARC
Bug 18322: (follow-up) Use authorized value labels

Description Martin Renvoize 2017-03-23 16:17:14 UTC
If we're going to add it for elasticsearch, it ought to also be there for zebra ;)
Comment 1 Martin Renvoize 2017-03-23 16:18:08 UTC
Created attachment 61581 [details] [review]
Add a facet for ccode fields to Zebra
Comment 2 Martin Renvoize 2017-03-23 16:21:35 UTC
This patch also fixes a couple minor bugs (id of collections facet title and dom element for facet titles) from the aforementioned elastic bug ;)
Comment 3 Jonathan Druart 2017-03-23 16:22:44 UTC
(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...
Comment 4 Tomás Cohen Arazi 2017-03-23 20:15:17 UTC
This is missing the index definition on the zebra files.
Comment 5 Tomás Cohen Arazi 2017-03-23 20:16:14 UTC
(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)
Comment 6 Martin Renvoize 2017-03-24 09:01:34 UTC
(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.
Comment 7 Martin Renvoize 2017-03-24 09:02:06 UTC
(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.
Comment 8 Martin Renvoize 2017-03-24 09:05:21 UTC
Created attachment 61597 [details] [review]
[QA Followup] Add a facet for ccode fields to Zebra (UNIMARC)
Comment 9 Martin Renvoize 2017-03-24 09:07:26 UTC
Added UNIMARC defs as requested.
Comment 10 Martin Renvoize 2017-03-24 15:02:13 UTC
Created attachment 61612 [details] [review]
Furher UNIMarc Followup
Comment 11 Tomás Cohen Arazi 2017-03-24 15:10:26 UTC
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.
Comment 12 Martin Renvoize 2017-03-27 15:42:04 UTC
Created attachment 61643 [details] [review]
Add configuration for zebra based faceting
Comment 13 Martin Renvoize 2017-03-27 15:42:57 UTC
Thanks for that pointer Tomas.. zebra facets vs non-zebra facets.. We had all our test systems set to non-zebra
Comment 14 Martin Renvoize 2017-03-27 15:44:29 UTC
Magnus, I wanted to check whether you also wanted this in the NORMARC configs?
Comment 15 Magnus Enger 2017-03-28 06:11:13 UTC
(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!
Comment 16 Martin Renvoize 2017-03-28 08:09:53 UTC
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.
Comment 17 Martin Renvoize 2017-03-28 08:13:05 UTC
Squashed patches.

This patch now covers adding the ccode facet for marc21, normarc and unimarc in zebra and non-zebra faceting.
Comment 18 Martin Renvoize 2017-03-28 08:15:13 UTC
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).
Comment 19 David Cook 2017-10-09 00:22:40 UTC
*** Bug 19435 has been marked as a duplicate of this bug. ***
Comment 20 David Cook 2017-10-09 00:29:02 UTC
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>
Comment 21 David Cook 2017-10-09 00:30:00 UTC
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!
Comment 22 Katrin Fischer 2018-03-12 13:08:37 UTC
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
Comment 23 Martin Renvoize 2018-06-12 14:40:10 UTC
Created attachment 75997 [details] [review]
Bug 18322: (followup) Add generated xsl
Comment 24 Tomás Cohen Arazi 2018-06-12 18:00:35 UTC Comment hidden (obsolete)
Comment 25 Tomás Cohen Arazi 2018-06-12 18:00:42 UTC Comment hidden (obsolete)
Comment 26 Tomás Cohen Arazi 2018-06-12 18:00:50 UTC Comment hidden (obsolete)
Comment 27 Tomás Cohen Arazi 2018-06-12 18:02:13 UTC
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.
Comment 28 Tomás Cohen Arazi 2018-06-12 18:03:13 UTC
C4::Search:584 is a closer example.
Comment 29 Martin Renvoize 2018-06-13 09:31:08 UTC
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.
Comment 30 Martin Renvoize 2018-06-13 09:32:21 UTC
Good spot, thanks Tomás!
Comment 31 Tomás Cohen Arazi 2018-06-13 13:14:38 UTC
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>
Comment 32 Tomás Cohen Arazi 2018-06-13 13:14:47 UTC
Created attachment 76024 [details] [review]
Bug 18322: (follow-up) Add generated xsl

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 33 Tomás Cohen Arazi 2018-06-13 13:14:54 UTC
Created attachment 76025 [details] [review]
Bug 18322: Update xslt for NORMARC and UNIMARC

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 34 Tomás Cohen Arazi 2018-06-13 13:15:05 UTC
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>
Comment 35 Nick Clemens 2018-07-18 19:14:44 UTC
Awesome work all!

Pushed to master for 18.11
Comment 36 Martin Renvoize 2018-08-02 12:22:04 UTC
Enhancement, will not be backported to 18.05.x series.