Bug 11175 - Show the parent record's component parts in the detailed views
Summary: Show the parent record's component parts in the detailed views
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Joonas Kylmälä
QA Contact: Josef Moravec
URL:
Keywords:
Depends on:
Blocks: 12625 21484 27507
  Show dependency treegraph
 
Reported: 2013-10-30 18:30 UTC by Olli-Antti Kivilahti
Modified: 2021-04-27 09:51 UTC (History)
18 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11175 - Show the parent records child records (aka. component parts) in the detailed views. (12.87 KB, patch)
2013-10-30 18:33 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Show the parent records child records (aka. component parts) in the detailed views. (16.00 KB, patch)
2013-10-31 14:49 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11175 - Show the parent records child records (aka. component parts) in the detailed views. (16.15 KB, patch)
2013-11-20 03:49 UTC, Garming Sam
Details | Diff | Splinter Review
Bug 11175 - Show the parent record's component parts in the detailed views. Business layer. (10.49 KB, patch)
2014-01-28 10:32 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Show the parent record's component parts in the detailed views. Presentation for prog-theme. (9.25 KB, patch)
2014-01-28 10:32 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Show the parent record's component parts in the detailed views. Presentation for bootstrap-theme. (3.85 KB, patch)
2014-01-28 10:33 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Show the parent record's component parts in the detailed views. Presentation for bootstrap-theme. (4.67 KB, patch)
2014-01-28 11:46 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Show the parent record's component parts in the detailed views. Business layer. (10.49 KB, patch)
2014-03-17 11:40 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Show the parent record's component parts in the detailed views. Presentation for prog-theme. (9.20 KB, patch)
2014-03-17 11:40 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Show the parent record's component parts in the detailed views. Presentation for bootstrap-theme. (5.33 KB, patch)
2014-03-17 11:40 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Fixing component part injection for non-default XSL-templates (1.82 KB, patch)
2014-03-17 11:41 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Rebased to parse XML result from SimpleSearch instead of the ISO2709-version which it used to be. (1.90 KB, patch)
2014-04-02 14:49 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175 - Show the parent record's component parts in the detailed views. Presentation for bootstrap-theme. (86.33 KB, patch)
2014-04-02 14:55 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (15.08 KB, patch)
2018-08-21 06:26 UTC, paxed
Details | Diff | Splinter Review
Example marc data with host record and component parts (204.00 KB, application/xml)
2018-08-21 06:27 UTC, paxed
Details
Screenshot (95.87 KB, image/png)
2018-08-21 06:28 UTC, paxed
Details
Bug 11175: Show record component parts in the detail view (15.09 KB, patch)
2018-10-01 05:28 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (24.32 KB, patch)
2018-10-02 05:20 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (24.19 KB, patch)
2018-10-02 08:43 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (24.25 KB, patch)
2018-10-02 09:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (21.95 KB, patch)
2020-06-08 10:21 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (972 bytes, patch)
2020-06-08 12:17 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (972 bytes, patch)
2020-06-10 17:24 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.75 KB, patch)
2020-06-10 17:27 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.10 KB, patch)
2020-06-10 17:27 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (2.99 KB, patch)
2020-07-09 09:07 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (22.01 KB, patch)
2020-11-02 14:40 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (972 bytes, patch)
2020-11-02 14:40 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.75 KB, patch)
2020-11-02 14:40 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (2.99 KB, patch)
2020-11-02 14:40 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Use uppercase AND and OR words in search query (1.16 KB, patch)
2020-11-02 15:32 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.56 KB, patch)
2020-11-02 15:32 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.38 KB, patch)
2020-11-02 16:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (22.06 KB, patch)
2020-11-02 16:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.00 KB, patch)
2020-11-02 16:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.81 KB, patch)
2020-11-02 16:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.05 KB, patch)
2020-11-02 16:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.61 KB, patch)
2020-11-02 16:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.43 KB, patch)
2020-11-02 16:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.01 KB, patch)
2020-11-16 08:43 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (9.84 KB, patch)
2020-11-16 08:43 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (1.57 KB, patch)
2020-11-24 17:28 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.08 KB, patch)
2020-11-24 17:36 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (22.04 KB, patch)
2020-12-11 11:11 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.06 KB, patch)
2020-12-11 11:11 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.86 KB, patch)
2020-12-11 11:11 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.10 KB, patch)
2020-12-11 11:11 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.65 KB, patch)
2020-12-11 11:11 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.49 KB, patch)
2020-12-11 11:11 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.07 KB, patch)
2020-12-11 11:11 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (9.87 KB, patch)
2020-12-11 11:11 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.14 KB, patch)
2020-12-11 11:11 UTC, paxed
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (22.19 KB, patch)
2021-02-12 12:06 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.06 KB, patch)
2021-02-12 12:06 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.86 KB, patch)
2021-02-12 12:06 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.11 KB, patch)
2021-02-12 12:06 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.74 KB, patch)
2021-02-12 12:06 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.49 KB, patch)
2021-02-12 12:06 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.07 KB, patch)
2021-02-12 12:06 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (9.90 KB, patch)
2021-02-12 12:06 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.14 KB, patch)
2021-02-12 12:06 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.29 KB, patch)
2021-02-12 12:07 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (5.57 KB, patch)
2021-04-13 05:07 UTC, Andrew Nugged
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.11 KB, patch)
2021-04-13 07:42 UTC, Andrew Nugged
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.52 KB, patch)
2021-04-27 09:51 UTC, Peter Vashchuk
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Olli-Antti Kivilahti 2013-10-30 18:30:10 UTC
TERMINOLOGY:
"Child record" or "Component part": A Bibliographic record connected to it's host record via $773w -> 001 or via other Record-control-number-index relation.
"Host record": A bibliographic record which contains child records. Host record doesn't have a link to the child record.

USEFUL:
For music cataloguers whose jobs depend on these.
For browsing the contents of a musical record.
Provides the basis for a high quality music library service.

Because Koha has the building blocks to create these child records out of box, it is odd that they can't be displayed out of box.


This patch adds a system preference AddChildRecordsToDetailedViews.
This controls if child records data is appended after the bibliographic data for XSLT parsing.
Also makes sure that child records are not pulled needlessly for non-detailed views as this would cripple performance.

Modifies the default XSLT templates:
 MARC21slim2intranetDetail.xsl and
 MARC21slim2OPACDetail.xsl
to display the child record data as HTML.
Modifies .css files as well.
Comment 1 Olli-Antti Kivilahti 2013-10-30 18:30:44 UTC
Sponsored by the Joensuu Regional Library
Comment 2 Olli-Antti Kivilahti 2013-10-30 18:33:24 UTC Comment hidden (obsolete)
Comment 3 Olli-Antti Kivilahti 2013-10-31 14:49:58 UTC Comment hidden (obsolete)
Comment 4 Garming Sam 2013-11-20 03:49:21 UTC Comment hidden (obsolete)
Comment 5 Liz Rea 2013-11-20 07:34:04 UTC
Quick note on this - the functionality is only available in the Prog theme - it
will need to be added to the new default bootstrap theming.

Cheers,
Liz
Comment 6 mathieu saby 2013-11-20 09:49:49 UTC
Note : when it is pushed, a followup will be needed for unimarc

M. Saby
Comment 7 Katrin Fischer 2013-12-06 15:21:49 UTC
Hi, 

starting with a code review: 

- Small thing: Please break your commit messages into shorter lines so 
  they are easier to read in a terminal window.
  
- Please provide a follow-up for the Bootstrap theme. As prog and
  CCSR are going to be deprecated this is mandatory now.
  
- Database update adding the new system preference is missing.

- The way this is coded it's a MARC21 specific feature. The rcn index
  and 001 > $w linking are not MARC agnostic. Please check for the
  marcflavor in your code and include a note
  about the limitation to MARC21 in the syspref description.
  
- Some of the comments look like TODOs - maybe something you want to 
  take another look at?

- Creating our 'own' XML in a MARCXML record doesn't look right to me. I
  would much prefer if that could be expressed in normal MARC21 fields
  and subfields.
  
- With the heading "Child records" you assume that it will be always
  a component part, but that's not true. 001 > $w linkings are used
  to express a lot of different relationships between records. I think
  putting a bit more thought into the various relationships could be
  good here.

Also the QA script points out some issues:

 FAIL	C4/XSLT.pm
   FAIL	  pod
		*** ERROR: Unknown command 'head'  in file C4/XSLT.pm
		*** ERROR: Spurious text after =cut  in file C4/XSLT.pm
   OK	  forbidden patterns
   OK	  valid
   FAIL	  critic
		# Variables::ProhibitConditionalDeclarations: Got 1 violation(s).
Comment 8 Olli-Antti Kivilahti 2013-12-09 15:02:59 UTC
Thanks for the QA everyone, thanks for the uncovered bugs!
I'll try to get back at this sometime next year :)
Comment 9 Olli-Antti Kivilahti 2014-01-28 10:32:28 UTC Comment hidden (obsolete)
Comment 10 Olli-Antti Kivilahti 2014-01-28 10:32:51 UTC Comment hidden (obsolete)
Comment 11 Olli-Antti Kivilahti 2014-01-28 10:33:08 UTC Comment hidden (obsolete)
Comment 12 Olli-Antti Kivilahti 2014-01-28 10:46:52 UTC Comment hidden (obsolete)
Comment 13 Olli-Antti Kivilahti 2014-01-28 11:46:06 UTC Comment hidden (obsolete)
Comment 14 Olli-Antti Kivilahti 2014-02-05 16:37:53 UTC Comment hidden (obsolete)
Comment 15 Olli-Antti Kivilahti 2014-03-17 11:40:19 UTC Comment hidden (obsolete)
Comment 16 Olli-Antti Kivilahti 2014-03-17 11:40:44 UTC Comment hidden (obsolete)
Comment 17 Olli-Antti Kivilahti 2014-03-17 11:40:56 UTC Comment hidden (obsolete)
Comment 18 Olli-Antti Kivilahti 2014-03-17 11:41:18 UTC Comment hidden (obsolete)
Comment 19 David Cook 2014-03-26 07:15:49 UTC
I'm quite interested in linking records, so I might have to take a look at this one.

I haven't reviewed the code or applied the patch yet, but how might this fare with a high number of child records? In the case of a journal with article component parts, you might have dozens, hundreds, or more child records for each parent.

I wonder if this would be handled better by an AJAX call and using the relational database instead of XML records.

In any case, I'll take a look later, but just a thought...
Comment 20 Olli-Antti Kivilahti 2014-03-26 08:39:24 UTC
Thank you for your comment!
AJAX might be a better option. You are free to submit any changes.
The complexity of such goes way over what we can develop given our timeframe.
For us this feature works relatively well since we only have a couple of hundred records in our database with over 100 component parts.

I can totally see the caching issues, but returning only a subset of a search query from Zebra is beyond my competence.
Comment 21 Olli-Antti Kivilahti 2014-04-02 14:49:35 UTC Comment hidden (obsolete)
Comment 22 Olli-Antti Kivilahti 2014-04-02 14:55:15 UTC Comment hidden (obsolete)
Comment 23 Bernardo Gonzalez Kriegel 2015-06-02 17:24:07 UTC
Nice feature to test, could you rebase it?
Comment 24 Katrin Fischer 2015-06-06 11:07:55 UTC
I'd be interested in this as well :)
Comment 25 paxed 2018-08-21 06:26:22 UTC
Created attachment 78022 [details] [review]
Bug 11175: Show record component parts in the detail view

Shows the component records of a host on the host intranet detail view,
with clickable links to the component records.
Host has no linking entries to the components, but the components link
to the host record via 773$w.

Adds a new search index, Control-number-identifier (aka cni), which
indexes the 003 controlfield.

When looking up the component part records, the code searches for
records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'.

Does not use easyanalytics.

Only for MARC21 biblios - MARC21 holdings and UNIMARC indexdefs
haven't been updated.

Only for the intranet detail page.

Staff-client.css has not been recreated, so you need to sass that
Comment 26 paxed 2018-08-21 06:27:53 UTC
Created attachment 78023 [details]
Example marc data with host record and component parts
Comment 27 paxed 2018-08-21 06:28:25 UTC
Created attachment 78024 [details]
Screenshot
Comment 28 Joonas Kylmälä 2018-09-26 11:40:51 UTC
I cannot test this:

Using index info to reconstruct a base tree...
M	C4/Search.pm
M	koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
M	koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl
Auto-merging koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
Auto-merging C4/Search.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 11175: Show record component parts in the detail view
The copy of the patch that failed is found in: .git/rebase-apply/patch

Also would it be better to make adding Control-number-identifier to it's own patch and how is this handled with Elasticsearch now that it is also supported by koha?
Comment 29 Katrin Fischer 2018-09-27 21:19:04 UTC
I really like the idea of this, but it won't work for some of our libraries with hundreds of analytics on serial records. Could we truncate the list at some (configurable) point with a 'see more' link at the end?
Comment 30 paxed 2018-09-28 03:41:51 UTC
(In reply to Katrin Fischer from comment #29)
> I really like the idea of this, but it won't work for some of our libraries
> with hundreds of analytics on serial records. Could we truncate the list at
> some (configurable) point with a 'see more' link at the end?

Your serial parts are component records? So there's a serial "host record", of which each published magazine (or whatever) is a child record?
Comment 31 Katrin Fischer 2018-09-28 05:41:28 UTC
Worse. The serial is the host, the children are the articles. The issue level is missing. This is because of the structure in our union catalog, but also fits Koha's subscription management. So this feature would have to be at least optional, nicer if we could use it of course.
Comment 32 paxed 2018-10-01 05:28:06 UTC
Created attachment 79687 [details] [review]
Bug 11175: Show record component parts in the detail view
Comment 33 paxed 2018-10-01 05:57:21 UTC
After talking with Cait, I'll make the feature depend on a syspref.
Comment 34 paxed 2018-10-02 05:20:52 UTC
Created attachment 79803 [details] [review]
Bug 11175: Show record component parts in the detail view

Shows the component records of a host, on the host detail view in
staff client or OPAC, with clickable links to the component records.
Host has no linking entries to the components, but the components link
to the host record via 773$w.

Adds a new search index, Control-number-identifier (aka cni), which
indexes the 003 controlfield.

Adds a Yet Another System Preference, ShowComponentRecords, which can
be used to turn this feature on or off in staff client and/or OPAC,
and defaults to off.

When looking up the component part records, the code searches for
records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'.
Except if UseControlNumber syspref is on, then it looks only for the
Host field 001.

Does not use easyanalytics.

Only for MARC21 biblios - UNIMARC has not been updated.

staff-global.css and opac.css have not been recreated, so you need
to use sass to recreate those from staff-global.scss and opac.scss

Test plan:

0) Apply patch
1) perl bulkmarcimport -file /tmp/easypiano.mrc -m MARCXML
   (This file is an attachment on the bug)
2) rebuild the zebra biblio index
3) Search for "easy piano" in staff client, and go to
   the biblio detail page. You should not see anything different
   in the record detail page.
4) Do the same on OPAC.
5) Change the ShowComponentRecords syspref appropriately and check
   the record detail page in staff client and OPAC.
   You should see a list of component part records.
Comment 35 paxed 2018-10-02 08:43:22 UTC
Created attachment 79809 [details] [review]
Bug 11175: Show record component parts in the detail view

Shows the component records of a host, on the host detail view in
staff client or OPAC, with clickable links to the component records.
Host has no linking entries to the components, but the components link
to the host record via 773$w.

Adds a new search index, Control-number-identifier (aka cni), which
indexes the 003 controlfield.

Adds a Yet Another System Preference, ShowComponentRecords, which can
be used to turn this feature on or off in staff client and/or OPAC,
and defaults to off.

When looking up the component part records, the code searches for
records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'.

Does not use easyanalytics.

Only for MARC21 biblios - UNIMARC has not been updated.

staff-global.css and opac.css have not been recreated, so you need
to use sass to recreate those from staff-global.scss and opac.scss

Test plan:

0) Apply patch
1) perl bulkmarcimport -file /tmp/easypiano.mrc -m MARCXML
   (This file is an attachment on the bug)
2) rebuild the zebra biblio index
3) Search for "easy piano" in staff client, and go to
   the biblio detail page. You should not see anything different
   in the record detail page.
4) Do the same on OPAC.
5) Change the ShowComponentRecords syspref appropriately and check
   the record detail page in staff client and OPAC.
   You should see a list of component part records.
Comment 36 Martin Renvoize 2018-10-02 09:17:29 UTC
Created attachment 79810 [details] [review]
Bug 11175: Show record component parts in the detail view

Shows the component records of a host, on the hosts detail view in
staff client or OPAC, with clickable links to the component records.

The host does not require linking entries to the components, but
components do require a link to the host record via 773$w.

Adds a new search index, Control-number-identifier (aka cni), which
indexes the 003 controlfield.

Adds 'Yet Another System Preference', ShowComponentRecords, which can
be used to turn this feature on or off in staff client and/or OPAC,
and defaults to off.

When looking up the component part records, the code searches for
records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'
or, if the 003 is not defined in the Host, 773$w=Host001.

Does not use easyanalytics or useControlNumber.

Only for MARC21 biblios - UNIMARC has not been updated.

staff-global.css and opac.css have not been recreated, so you need
to use sass to recreate those from staff-global.scss and opac.scss

Test plan:

0) Apply patch
1) perl bulkmarcimport -file /tmp/easypiano.mrc -m MARCXML
   (This file is an attachment on the bug)
2) rebuild the zebra biblio index
3) Search for "easy piano" in staff client, and go to
   the biblio detail page. You should not see anything different
   in the record detail page.
4) Do the same on OPAC.
5) Change the ShowComponentRecords syspref appropriately and check
   the record detail page in staff client and OPAC.
   You should see a list of component part records.

Signed-off-by: Pasi Kallinen <pasi.kallinen@joensuu.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Martin Renvoize 2018-10-02 09:18:21 UTC
Works as expected for me and it looks like all QA comments to date have been resolved.

Adding my SO as a second signoff.
Comment 38 Josef Moravec 2018-10-03 05:53:51 UTC
Comment on attachment 79810 [details] [review]
Bug 11175: Show record component parts in the detail view

Review of attachment 79810 [details] [review]:
-----------------------------------------------------------------

::: Koha/Biblio.pm
@@ +388,4 @@
>      return $self->{_biblioitem};
>  }
>  
> +=head3 components

Please, provide tests for this method

@@ +399,5 @@
> +
> +sub components {
> +    my ($self) = @_;
> +
> +    return undef if (C4::Context->preference('marcflavour') ne 'MARC21');

Do not return explicit undef
Comment 39 Josef Moravec 2018-10-03 06:07:11 UTC
And please add support for elasticsearch.
Comment 40 paxed 2018-10-03 06:14:01 UTC
Someone else can take this bug now, I have no time to work on it.
Comment 41 Olli-Antti Kivilahti 2018-10-03 06:26:33 UTC
(In reply to Josef Moravec from comment #38)
> Do not return explicit undef

Could you elaborate a bit why returning undef is bad?
Comment 42 Josef Moravec 2018-10-03 07:49:09 UTC
(In reply to Olli-Antti Kivilahti from comment #41)
> (In reply to Josef Moravec from comment #38)
> > Do not return explicit undef
> 
> Could you elaborate a bit why returning undef is bad?

It could lead to unexpected results, see:

https://metacpan.org/pod/Perl::Critic::Policy::Subroutines::ProhibitExplicitReturnUndef

https://perlmaven.com/how-to-return-undef-from-a-function

We even have it in our guidelines (this policy is part of perlcritic severity 5):
https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL4:_Perlcritic
Comment 43 Josef Moravec 2018-10-03 07:51:45 UTC
(In reply to Olli-Antti Kivilahti from comment #41)

Are you going to adopt this? It would be great!
Comment 44 Katrin Fischer 2018-10-03 14:22:43 UTC
(In reply to Josef Moravec from comment #39)
> And please add support for elasticsearch.

I don't think we should make this a requirement at the moment until Elasticsearch has left experimental status and this has been added to the coding guidelines as a requirement. For now, I think, not breaking Elasticsearch functionality with new code would be enough. Other things can be dealt with later.

This is something we have been asked about for an awful long time, it would be sad to see the first steps abandoned.
Comment 45 Olli-Antti Kivilahti 2018-10-03 14:30:39 UTC
I think this feature should be generalized to work with all the 77X linking fields as instructed by the MARC21 manual. That might not be such a big step up.
Adding pagination is more complicated and requires this feature to be ported to the REST API, which would be superb anyway.
Comment 46 Katrin Fischer 2018-10-03 20:35:36 UTC
I think making it work with other 7xx could be a next step on a separate bug report. Also for the other 7xx there is more information in the record already about the linked items. From a set you often don't see much if anything about the children, so this appears to be the most urgent.
Comment 47 Josef Moravec 2018-10-04 06:07:25 UTC
(In reply to Katrin Fischer from comment #44)
> (In reply to Josef Moravec from comment #39)
> > And please add support for elasticsearch.
> 
> I don't think we should make this a requirement at the moment until
> Elasticsearch has left experimental status and this has been added to the
> coding guidelines as a requirement. For now, I think, not breaking
> Elasticsearch functionality with new code would be enough. Other things can
> be dealt with later.

Ok, I could live without elastic for now, but I have filled a follow-up bug report - bug 21484 to address this.

> 
> This is something we have been asked about for an awful long time, it would
> be sad to see the first steps abandoned.
Comment 48 Johanna Räisä 2018-11-02 07:37:35 UTC
As we have used this in Finland for some years now, my opinion is that using the index for linking the records is not very efficient. You are always depended on it and if you want to make REST endpoint for the host and its component parts the callback probably isn't always solid.

Also we have had issues with removing records. If you remove the host record the children are not going to be removed at the same time. And finding which children can be removed afterwards is difficult. On the other hand cleaning the database is getting very annoying and slow.

My proposal is to move this linking to database level. Having a linking table which has host's biblionumber and child's biblionumber would make it solid and easier to handle. This could be a solution that we don't have to implement always to new search index platform.
Comment 49 Olli-Antti Kivilahti 2018-11-02 08:07:22 UTC
DB link table is a good idea.
Zebra searches for the link relations were blazingly fast tho.
Comment 50 Martin Renvoize 2018-11-02 09:07:40 UTC
Add DB link tables moves this bug into entirely different territory in my opinion... lets get the basics working well with zebra first as we're very nearly there here already.
Comment 51 Katrin Fischer 2018-11-02 09:13:09 UTC
So I think the only thing left fixing are these 2 points right now, correct?

BTW I am not sure about a linking table for links between records, but I would love to see a linking table for the EasyAnalytics feature that happens on item level and 'spills over' into the record with non-standard 77x entries. 

In reply to Josef Moravec from comment #38)
> Comment on attachment 79810 [details] [review] [review]
> Bug 11175: Show record component parts in the detail view
> 
> Review of attachment 79810 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/Biblio.pm
> @@ +388,4 @@
> >      return $self->{_biblioitem};
> >  }
> >  
> > +=head3 components
> 
> Please, provide tests for this method
> 
> @@ +399,5 @@
> > +
> > +sub components {
> > +    my ($self) = @_;
> > +
> > +    return undef if (C4::Context->preference('marcflavour') ne 'MARC21');
> 
> Do not return explicit undef
Comment 52 Olli-Antti Kivilahti 2018-11-02 09:17:44 UTC
We are currently discussing how we solve this problem properly for both the Finnish academic and public library sectors.
This includes more than 773 linking.

I have 0% interest to keep fiddling with this miniscule minimum product which no longer satisfies our extended requirements.

I understand the need for small steps, but thanks to the small steps, we still have to restart the SIP2-server due to memcached.
Comment 53 Katrin Fischer 2018-11-02 09:20:24 UTC
So you are going to let this die here?
Comment 54 Olli-Antti Kivilahti 2018-11-02 09:27:02 UTC
We can always open a new bug ticket :)
Comment 55 Olli-Antti Kivilahti 2018-11-02 09:28:22 UTC
BTW Zebra doesn't always return the correct search results. For ex we have a REST API endpoint which returns all component parts for a biblio. It is awkward that we know some biblios have component parts, and still the Zebra searches don't return those results.
That makes for rather bad API service.

Using a DB link-table adds reliability.
Comment 56 Olli-Antti Kivilahti 2018-11-02 09:34:35 UTC
BTW.
First Finnish academic library HAMK went live last tuesday.

Using ES in production.

What we are trying to decide is, should these links be only in the ES index, only in the DB as a link-table with different link types separated, or should we implement both.
Comment 57 Johanna Räisä 2018-11-02 10:55:08 UTC
Maybe we should open new ticket for it. The linking table could be generic and used more versatile for different types of links.
Comment 58 Joonas Kylmälä 2020-06-08 10:21:51 UTC
Created attachment 105631 [details] [review]
Bug 11175: Show record component parts in the detail view

Shows the component records of a host, on the hosts detail view in
staff client or OPAC, with clickable links to the component records.

The host does not require linking entries to the components, but
components do require a link to the host record via 773$w.

Adds a new search index, Control-number-identifier (aka cni), which
indexes the 003 controlfield.

Adds 'Yet Another System Preference', ShowComponentRecords, which can
be used to turn this feature on or off in staff client and/or OPAC,
and defaults to off.

When looking up the component part records, the code searches for
records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'
or, if the 003 is not defined in the Host, 773$w=Host001.

Does not use easyanalytics or useControlNumber.

Only for MARC21 biblios - UNIMARC has not been updated.

staff-global.css and opac.css have not been recreated, so you need
to use sass to recreate those from staff-global.scss and opac.scss

Test plan:

0) Apply patch
1) perl bulkmarcimport -file /tmp/easypiano.mrc -m MARCXML
   (This file is an attachment on the bug)
2) rebuild the zebra biblio index
3) Search for "easy piano" in staff client, and go to
   the biblio detail page. You should not see anything different
   in the record detail page.
4) Do the same on OPAC.
5) Change the ShowComponentRecords syspref appropriately and check
   the record detail page in staff client and OPAC.
   You should see a list of component part records.

Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 59 Joonas Kylmälä 2020-06-08 10:26:36 UTC
I rebased the earlier patch and will now start working on follow-ups for the two problems raised by Josef in comment #38:

1. Please, provide tests for this method
2. Do not return explicit undef

I think it's is not bad idea to finish this implementation, the refactoring to DB table based implementation can be done later, this already adds a nice UI.
Comment 60 Joonas Kylmälä 2020-06-08 12:17:30 UTC
Created attachment 105641 [details] [review]
Bug 11175: (follow-up) Don't return explicitly undef

This would create in list context an list of one element that is
undef, which we don't want, we want empty list if there are no
components.
Comment 61 Joonas Kylmälä 2020-06-10 17:24:29 UTC
Created attachment 105703 [details] [review]
Bug 11175: (follow-up) Don't return explicitly undef

This would create in list context an list of one element that is
undef, which we don't want, we want empty list if there are no
components.
Comment 62 Joonas Kylmälä 2020-06-10 17:27:13 UTC
Created attachment 105706 [details] [review]
Bug 11175: (follow-up) Return empty array if no components
Comment 63 Joonas Kylmälä 2020-06-10 17:27:33 UTC
Created attachment 105707 [details] [review]
Bug 11175: (follow-up) Add tests
Comment 64 Joonas Kylmälä 2020-06-10 17:31:35 UTC
I added the missing tests and addressed the undef problem. In my kohadevbox setup however the test works only on the first run after running "reset_all". For some reason the Koha::Biblio->find accessor stops working, it could be something related to my setup, or there might be a bug in the test?
Comment 65 Joonas Kylmälä 2020-06-10 17:34:14 UTC
I will do some more testing with the unit test issue to make sure its not a bug in the test code, so changing the status to "In discussion" until that is resolved.
Comment 66 Joonas Kylmälä 2020-07-09 09:07:49 UTC
Created attachment 106692 [details] [review]
Bug 11175: (follow-up) Add tests
Comment 67 Joonas Kylmälä 2020-07-09 09:11:19 UTC
(In reply to Joonas Kylmälä from comment #65)
> I will do some more testing with the unit test issue to make sure its not a
> bug in the test code, so changing the status to "In discussion" until that
> is resolved.

The issue was with the test's handling of C4::AddBiblio() results: it was missing parentheses around $host_bibnum so it was treated as an array. I have fixed this now. The patches are ready for someone to sign-off!
Comment 68 David Cook 2020-09-08 07:29:13 UTC
I have concerns about these patches.

Firstly, they're only targeting Zebra and not Elasticsearch. While I'm not sure if there is any formal rules about that yet, I think we want to be including Elasticsearch for all search related changes.

Secondly, I don't really like the idea of stuffing more XML into the search result XSLT. I think that's already one of the slowest parts of Koha, and this is likely to just slow it down further. It would be nicer to load related data asynchronously using AJAX.

But the more I read and re-read the code... maybe it's not a big drama. 

I feel like this is adding to technical debt, but I'm not going to block it.
Comment 69 Martin Renvoize 2020-09-08 10:42:31 UTC
I've poked Nick C to ask how easy it might be to add a followup for ES support here.

Otherwise, I'm generally happy with the code as it stands.. yes, it might be nice to eventually make it asynchronous, and perhaps we could make it clearer in the various sysprefs how EasyAnalytics and UseControlNumber are not involved in this one.  But generally I think the approach is sound and doesn't add too much additional maintenance burden at this time.

Awaiting Nicks reply before a final SO/QA run.
Comment 70 Katrin Fischer 2020-09-13 19:53:36 UTC
Hi Joonas,

finally found some time to take a closer look:

1) System preference
- I think staff interface is not a good tab choise, I'd put it under "Cataloguing" or "Search".
- As this currently doesn't work for UNIMARC or Elasticsearch, we should add a hint about this in the pref so people know what to expect. 
- And maybe "does not require UseControlNumber".?
- Change staff client to staff interface (see terminology list)

2) Use of 003
Looking at the patch and the test plan I am quite happy with the proposed search strategy. This should work really well for our libraries!

3) I am still slightly worried about what will happen when there are several hundred of analytical records found. This would be very commonly the case for our law and special libraries that do extensive analytical cataloguing for serials. And all the analyticals are linked to the serials (there are no volume or issue records as per the rules of our union catalog) so it gets quite a lot. Could you think of a strategy for those?

Maybe we could define a number for a "cutoff" and display a link for "Show more". Ideal would be to show the newest x linked records.

I am going to sign-off as this is an optional feature that works well as is. I hope we can figure out something for 1) (maybe we will add Elasticsearch support after consulting Nick?) and possibly 3) in the QA step.
Comment 71 Katrin Fischer 2020-09-13 19:54:59 UTC
Hm, sorry, found a bigger issue last minute: In the OPAC the display is not nice, probably due to the bootstrap patch being pushed in between. All the components show above the title of the record now.
Comment 72 Joonas Kylmälä 2020-11-02 14:40:14 UTC
Created attachment 112838 [details] [review]
Bug 11175: Show record component parts in the detail view

Shows the component records of a host, on the hosts detail view in
staff client or OPAC, with clickable links to the component records.

The host does not require linking entries to the components, but
components do require a link to the host record via 773$w.

Adds a new search index, Control-number-identifier (aka cni), which
indexes the 003 controlfield.

Adds 'Yet Another System Preference', ShowComponentRecords, which can
be used to turn this feature on or off in staff client and/or OPAC,
and defaults to off.

When looking up the component part records, the code searches for
records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'
or, if the 003 is not defined in the Host, 773$w=Host001.

Does not use easyanalytics or useControlNumber.

Only for MARC21 biblios - UNIMARC has not been updated.

staff-global.css and opac.css have not been recreated, so you need
to use sass to recreate those from staff-global.scss and opac.scss

Test plan:

0) Apply patch
1) perl bulkmarcimport -file /tmp/easypiano.mrc -m MARCXML
   (This file is an attachment on the bug)
2) rebuild the zebra biblio index
3) Search for "easy piano" in staff client, and go to
   the biblio detail page. You should not see anything different
   in the record detail page.
4) Do the same on OPAC.
5) Change the ShowComponentRecords syspref appropriately and check
   the record detail page in staff client and OPAC.
   You should see a list of component part records.

Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 73 Joonas Kylmälä 2020-11-02 14:40:20 UTC
Created attachment 112839 [details] [review]
Bug 11175: (follow-up) Don't return explicitly undef

This would create in list context an list of one element that is
undef, which we don't want, we want empty list if there are no
components.
Comment 74 Joonas Kylmälä 2020-11-02 14:40:25 UTC
Created attachment 112840 [details] [review]
Bug 11175: (follow-up) Return empty array if no components
Comment 75 Joonas Kylmälä 2020-11-02 14:40:30 UTC
Created attachment 112841 [details] [review]
Bug 11175: (follow-up) Add tests
Comment 76 Joonas Kylmälä 2020-11-02 14:40:49 UTC
Rebased.
Comment 77 Joonas Kylmälä 2020-11-02 15:32:50 UTC
Created attachment 112846 [details] [review]
Bug 11175: (follow-up) Use uppercase AND and OR words in search query

Elasticsearch only understands uppercase query operators.
Comment 78 Joonas Kylmälä 2020-11-02 15:32:55 UTC
Created attachment 112847 [details] [review]
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description

The syspref is not only affecting staff interface so moving it to
Cataloguing section. Added also notes about UNIMARC and Elasticsearch
not being supported.
Comment 79 Joonas Kylmälä 2020-11-02 15:33:48 UTC
(In reply to Katrin Fischer from comment #71)
> Hm, sorry, found a bigger issue last minute: In the OPAC the display is not
> nice, probably due to the bootstrap patch being pushed in between. All the
> components show above the title of the record now.

To have it show correctly one needs to rebuild the CSS files: https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_interface#Compiling_Sass_files
Comment 80 Joonas Kylmälä 2020-11-02 15:51:17 UTC
(In reply to Katrin Fischer from comment #70)
> Hi Joonas,
> 
> finally found some time to take a closer look:
> 
> 1) System preference
> - I think staff interface is not a good tab choise, I'd put it under
> "Cataloguing" or "Search".

I went with Cataloguing.

> - As this currently doesn't work for UNIMARC or Elasticsearch, we should add
> a hint about this in the pref so people know what to expect. 

Added this info to description.

> - And maybe "does not require UseControlNumber".?

I don't understand what you mean here.

> - Change staff client to staff interface (see terminology list)

Done.

> 3) I am still slightly worried about what will happen when there are several
> hundred of analytical records found. This would be very commonly the case
> for our law and special libraries that do extensive analytical cataloguing
> for serials. And all the analyticals are linked to the serials (there are no
> volume or issue records as per the rules of our union catalog) so it gets
> quite a lot. Could you think of a strategy for those?
> 
> Maybe we could define a number for a "cutoff" and display a link for "Show
> more". Ideal would be to show the newest x linked records.

I added 325 component part records and it loaded as far as I could tell the same speed as without any component part records, so there is no noticeable slow down here, the algorithm for this looks linear. The current approach seems OK to me, no need for paging because the HTML links generated here are very light weight.

I'm thinking this is again ready for sign-off.
Comment 81 Nick Clemens 2020-11-02 16:03:40 UTC
Created attachment 112851 [details] [review]
Bug 11175: Add Elasticsearch support
Comment 82 Nick Clemens 2020-11-02 16:04:57 UTC
(In reply to Joonas Kylmälä from comment #77)
> Created attachment 112846 [details] [review] [review]
> Bug 11175: (follow-up) Use uppercase AND and OR words in search query
> 
> Elasticsearch only understands uppercase query operators.

Hi Joonas,

I obsoleted yours and added a new patch to fully add ES support here, let me know if that works for you
Comment 83 Nick Clemens 2020-11-02 16:07:39 UTC
Created attachment 112852 [details] [review]
Bug 11175: Show record component parts in the detail view

Shows the component records of a host, on the hosts detail view in
staff client or OPAC, with clickable links to the component records.

The host does not require linking entries to the components, but
components do require a link to the host record via 773$w.

Adds a new search index, Control-number-identifier (aka cni), which
indexes the 003 controlfield.

Adds 'Yet Another System Preference', ShowComponentRecords, which can
be used to turn this feature on or off in staff client and/or OPAC,
and defaults to off.

When looking up the component part records, the code searches for
records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'
or, if the 003 is not defined in the Host, 773$w=Host001.

Does not use easyanalytics or useControlNumber.

Only for MARC21 biblios - UNIMARC has not been updated.

staff-global.css and opac.css have not been recreated, so you need
to use sass to recreate those from staff-global.scss and opac.scss

Test plan:

0) Apply patch
1) perl bulkmarcimport -file /tmp/easypiano.mrc -m MARCXML
   (This file is an attachment on the bug)
2) rebuild the zebra biblio index
3) Search for "easy piano" in staff client, and go to
   the biblio detail page. You should not see anything different
   in the record detail page.
4) Do the same on OPAC.
5) Change the ShowComponentRecords syspref appropriately and check
   the record detail page in staff client and OPAC.
   You should see a list of component part records.

Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 84 Nick Clemens 2020-11-02 16:07:43 UTC
Created attachment 112853 [details] [review]
Bug 11175: (follow-up) Don't return explicitly undef

This would create in list context an list of one element that is
undef, which we don't want, we want empty list if there are no
components.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 85 Nick Clemens 2020-11-02 16:07:48 UTC
Created attachment 112854 [details] [review]
Bug 11175: (follow-up) Return empty array if no components

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 86 Nick Clemens 2020-11-02 16:07:52 UTC
Created attachment 112855 [details] [review]
Bug 11175: (follow-up) Add tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 87 Nick Clemens 2020-11-02 16:07:57 UTC
Created attachment 112856 [details] [review]
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description

The syspref is not only affecting staff interface so moving it to
Cataloguing section. Added also notes about UNIMARC and Elasticsearch
not being supported.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 88 Nick Clemens 2020-11-02 16:08:02 UTC
Created attachment 112857 [details] [review]
Bug 11175: Add Elasticsearch support

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 89 Joonas Kylmälä 2020-11-02 16:12:23 UTC
(In reply to Nick Clemens from comment #82)
> Hi Joonas,
> 
> I obsoleted yours and added a new patch to fully add ES support here, let me
> know if that works for you

Of course it works for me! You're awesome, thanks :)

I tested the Elasticsearch implementation and I noticed slight difference there: with Elasticsearch only the first 100 component part records are returned meanwhile with Zebra all the component part records are returned. Maybe a follow-up for this would be needed?
Comment 90 Nick Clemens 2020-11-02 16:30:57 UTC
(In reply to Joonas Kylmälä from comment #89)
> (In reply to Nick Clemens from comment #82)
> > Hi Joonas,
> > 
> > I obsoleted yours and added a new patch to fully add ES support here, let me
> > know if that works for you
> 
> Of course it works for me! You're awesome, thanks :)
> 
> I tested the Elasticsearch implementation and I noticed slight difference
> there: with Elasticsearch only the first 100 component part records are
> returned meanwhile with Zebra all the component part records are returned.
> Maybe a follow-up for this would be needed?

Koha/SearchEngine/Elasticsearch/Search.pm 
298 =item C<$max_results>
299 
300 The max number of results to return. The default is 100 (because unlimited
301 is a pretty terrible thing to do.)

Probably this should be a syspref? For Zebra we don't hardcode a limit, but maybe we should not return everything and it would be better to have one syspref to control both>
Comment 91 Joonas Kylmälä 2020-11-03 09:23:08 UTC
(In reply to Nick Clemens from comment #90)
> Koha/SearchEngine/Elasticsearch/Search.pm 
> 298 =item C<$max_results>
> 299 
> 300 The max number of results to return. The default is 100 (because
> unlimited
> 301 is a pretty terrible thing to do.)
> 
> Probably this should be a syspref? For Zebra we don't hardcode a limit, but
> maybe we should not return everything and it would be better to have one
> syspref to control both>

I don't think a global max results syspref would be a good idea. We could probably identify all calls of simple_search_compat() and then those that require more than 100 results, like this feature, should handle this case by making more calls using offset or different max results value. So it seems like we are now back to the question whether implement AJAX as was discussed earlier but it would be useful to have a rest api endpoint for component parts in that case. Or we could also say we are fine with 100 results at this point in time. What are your opinions on this?
Comment 92 paxed 2020-11-03 10:47:19 UTC
Just as an example, from one of our larger consortiums, the single biblio with the most components they have is 1000 - from "THE FOLKSONG FAKE BOOK : A COLLECTION OF OVER 1000 FOLKSONGS FROM AROUND THE WORLD"

Average number of components is 18.5, and there are about 56000 component parts in total.

If the max number of components is set to something like 300, it would cover more than 99% of the cases, which IMO would be good enough for now - this could be changed after the feature is in.
Comment 93 Katrin Fischer 2020-11-03 12:12:58 UTC
If the performance is ok, I would be happy with 100. Maybe we could add a note that it will show up to 100 if 100 results are returned or so? Just a hint that there might be more.
Comment 94 Nick Clemens 2020-11-03 12:28:58 UTC
(In reply to Katrin Fischer from comment #93)
> If the performance is ok, I would be happy with 100. Maybe we could add a
> note that it will show up to 100 if 100 results are returned or so? Just a
> hint that there might be more.

As the page here uses a search to find these, why not just add a link to perform that search as a regular search and provide the results to the user?
Comment 95 Joonas Kylmälä 2020-11-03 12:32:06 UTC
(In reply to Nick Clemens from comment #94)
> (In reply to Katrin Fischer from comment #93)
> > If the performance is ok, I would be happy with 100. Maybe we could add a
> > note that it will show up to 100 if 100 results are returned or so? Just a
> > hint that there might be more.
> 
> As the page here uses a search to find these, why not just add a link to
> perform that search as a regular search and provide the results to the user?

Great idea! I will start working on a follow-up. I will also fix the wording in the syspref page regarding ES not being supported.
Comment 96 Joonas Kylmälä 2020-11-16 08:43:34 UTC
Created attachment 113649 [details] [review]
Bug 11175: Keep component part record list items inside the box

Without this the number denoting the component part item in the list
will overflow and is partly hidden.

To test:
 1) Create 100 component parts
 2) Notice without this patch the number next to the component part is
    not fully shown
Comment 97 Joonas Kylmälä 2020-11-16 08:43:38 UTC
Created attachment 113650 [details] [review]
Bug 11175: Limit the amount of component parts returned

There was already before this a limit of 100 results when using
Elasticsearch but then the list of component parts was truncated
silently. This change now limits the amount to hard coded limit of 300
which is still fast to render. Also when the 300 component part record
limit is reached there is a link in the list now to list all the
records via the cataloging search.

To test:
   1) Create 300+ component part records and see if the link to list
      all the component part records shows up
   2) Make sure prove t/Koha/Util/Search.t passes
Comment 98 Joonas Kylmälä 2020-11-24 17:28:30 UTC Comment hidden (obsolete)
Comment 99 Joonas Kylmälä 2020-11-24 17:32:41 UTC
If anybody knows why the double quotes are needed with Elasticsearch I would be happy to know :)
Comment 100 Joonas Kylmälä 2020-11-24 17:36:03 UTC
Created attachment 113964 [details] [review]
Bug 11175: Search using double quotes to support Elasticsearch

With Elasticsearch if you would use the search string here without the
change to double quotes, i.e.

(rcn='1234' AND cni='FI-XXX') OR rcn='FI-XXX 1234'

then the search would somehow manage to return a record with 001 =
1234 and 003 = FI-XXX. Using double quotes prevents that. Also using
parenthesis around the latter part of the OR seems to work:

(rcn='1234' AND cni='FI-XXX') OR (rcn='FI-XXX 1234')
Comment 101 paxed 2020-12-11 11:11:01 UTC
Created attachment 114326 [details] [review]
Bug 11175: Show record component parts in the detail view

Shows the component records of a host, on the hosts detail view in
staff client or OPAC, with clickable links to the component records.

The host does not require linking entries to the components, but
components do require a link to the host record via 773$w.

Adds a new search index, Control-number-identifier (aka cni), which
indexes the 003 controlfield.

Adds 'Yet Another System Preference', ShowComponentRecords, which can
be used to turn this feature on or off in staff client and/or OPAC,
and defaults to off.

When looking up the component part records, the code searches for
records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'
or, if the 003 is not defined in the Host, 773$w=Host001.

Does not use easyanalytics or useControlNumber.

Only for MARC21 biblios - UNIMARC has not been updated.

staff-global.css and opac.css have not been recreated, so you need
to use sass to recreate those from staff-global.scss and opac.scss

Test plan:

0) Apply patch
1) perl bulkmarcimport -file /tmp/easypiano.mrc -m MARCXML
   (This file is an attachment on the bug)
2) rebuild the zebra biblio index
3) Search for "easy piano" in staff client, and go to
   the biblio detail page. You should not see anything different
   in the record detail page.
4) Do the same on OPAC.
5) Change the ShowComponentRecords syspref appropriately and check
   the record detail page in staff client and OPAC.
   You should see a list of component part records.

Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 102 paxed 2020-12-11 11:11:06 UTC
Created attachment 114327 [details] [review]
Bug 11175: (follow-up) Don't return explicitly undef

This would create in list context an list of one element that is
undef, which we don't want, we want empty list if there are no
components.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 103 paxed 2020-12-11 11:11:11 UTC
Created attachment 114328 [details] [review]
Bug 11175: (follow-up) Return empty array if no components

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 104 paxed 2020-12-11 11:11:16 UTC
Created attachment 114329 [details] [review]
Bug 11175: (follow-up) Add tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 105 paxed 2020-12-11 11:11:21 UTC
Created attachment 114330 [details] [review]
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description

The syspref is not only affecting staff interface so moving it to
Cataloguing section. Added also notes about UNIMARC and Elasticsearch
not being supported.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 106 paxed 2020-12-11 11:11:26 UTC
Created attachment 114331 [details] [review]
Bug 11175: Add Elasticsearch support

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 107 paxed 2020-12-11 11:11:32 UTC
Created attachment 114332 [details] [review]
Bug 11175: Keep component part record list items inside the box

Without this the number denoting the component part item in the list
will overflow and is partly hidden.

To test:
 1) Create 100 component parts
 2) Notice without this patch the number next to the component part is
    not fully shown

Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 108 paxed 2020-12-11 11:11:37 UTC
Created attachment 114333 [details] [review]
Bug 11175: Limit the amount of component parts returned

There was already before this a limit of 100 results when using
Elasticsearch but then the list of component parts was truncated
silently. This change now limits the amount to hard coded limit of 300
which is still fast to render. Also when the 300 component part record
limit is reached there is a link in the list now to list all the
records via the cataloging search.

To test:
   1) Create 300+ component part records and see if the link to list
      all the component part records shows up
   2) Make sure prove t/Koha/Util/Search.t passes

Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 109 paxed 2020-12-11 11:11:42 UTC
Created attachment 114334 [details] [review]
Bug 11175: Search using double quotes to support Elasticsearch

With Elasticsearch if you would use the search string here without the
change to double quotes, i.e.

(rcn='1234' AND cni='FI-XXX') OR rcn='FI-XXX 1234'

then the search would somehow manage to return a record with 001 =
1234 and 003 = FI-XXX. Using double quotes prevents that. Also using
parenthesis around the latter part of the OR seems to work:

(rcn='1234' AND cni='FI-XXX') OR (rcn='FI-XXX 1234')

Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 110 Joonas Kylmälä 2021-01-12 14:29:10 UTC
Elasticsearch from the syspref text "Elasticsearch and UNIMARC are not supported" needs to be removed.
Comment 111 Joonas Kylmälä 2021-02-12 12:06:17 UTC
Created attachment 116819 [details] [review]
Bug 11175: Show record component parts in the detail view

Shows the component records of a host, on the hosts detail view in
staff client or OPAC, with clickable links to the component records.

The host does not require linking entries to the components, but
components do require a link to the host record via 773$w.

Adds a new search index, Control-number-identifier (aka cni), which
indexes the 003 controlfield.

Adds 'Yet Another System Preference', ShowComponentRecords, which can
be used to turn this feature on or off in staff client and/or OPAC,
and defaults to off.

When looking up the component part records, the code searches for
records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'
or, if the 003 is not defined in the Host, 773$w=Host001.

Does not use easyanalytics or useControlNumber.

Only for MARC21 biblios - UNIMARC has not been updated.

staff-global.css and opac.css have not been recreated, so you need
to use sass to recreate those from staff-global.scss and opac.scss

Test plan:

0) Apply patch
1) perl bulkmarcimport -file /tmp/easypiano.mrc -m MARCXML
   (This file is an attachment on the bug)
2) rebuild the zebra biblio index
3) Search for "easy piano" in staff client, and go to
   the biblio detail page. You should not see anything different
   in the record detail page.
4) Do the same on OPAC.
5) Change the ShowComponentRecords syspref appropriately and check
   the record detail page in staff client and OPAC.
   You should see a list of component part records.

Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 112 Joonas Kylmälä 2021-02-12 12:06:23 UTC
Created attachment 116820 [details] [review]
Bug 11175: (follow-up) Don't return explicitly undef

This would create in list context an list of one element that is
undef, which we don't want, we want empty list if there are no
components.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 113 Joonas Kylmälä 2021-02-12 12:06:27 UTC
Created attachment 116821 [details] [review]
Bug 11175: (follow-up) Return empty array if no components

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 114 Joonas Kylmälä 2021-02-12 12:06:33 UTC
Created attachment 116822 [details] [review]
Bug 11175: (follow-up) Add tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 115 Joonas Kylmälä 2021-02-12 12:06:38 UTC
Created attachment 116823 [details] [review]
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description

The syspref is not only affecting staff interface so moving it to
Cataloguing section. Added also notes about UNIMARC and Elasticsearch
not being supported.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 116 Joonas Kylmälä 2021-02-12 12:06:43 UTC
Created attachment 116824 [details] [review]
Bug 11175: Add Elasticsearch support

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 117 Joonas Kylmälä 2021-02-12 12:06:48 UTC
Created attachment 116825 [details] [review]
Bug 11175: Keep component part record list items inside the box

Without this the number denoting the component part item in the list
will overflow and is partly hidden.

To test:
 1) Create 100 component parts
 2) Notice without this patch the number next to the component part is
    not fully shown

Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 118 Joonas Kylmälä 2021-02-12 12:06:53 UTC
Created attachment 116826 [details] [review]
Bug 11175: Limit the amount of component parts returned

There was already before this a limit of 100 results when using
Elasticsearch but then the list of component parts was truncated
silently. This change now limits the amount to hard coded limit of 300
which is still fast to render. Also when the 300 component part record
limit is reached there is a link in the list now to list all the
records via the cataloging search.

To test:
   1) Create 300+ component part records and see if the link to list
      all the component part records shows up
   2) Make sure prove t/Koha/Util/Search.t passes

Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 119 Joonas Kylmälä 2021-02-12 12:06:59 UTC
Created attachment 116827 [details] [review]
Bug 11175: Search using double quotes to support Elasticsearch

With Elasticsearch if you would use the search string here without the
change to double quotes, i.e.

(rcn='1234' AND cni='FI-XXX') OR rcn='FI-XXX 1234'

then the search would somehow manage to return a record with 001 =
1234 and 003 = FI-XXX. Using double quotes prevents that. Also using
parenthesis around the latter part of the OR seems to work:

(rcn='1234' AND cni='FI-XXX') OR (rcn='FI-XXX 1234')

Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 120 Joonas Kylmälä 2021-02-12 12:07:03 UTC
Created attachment 116828 [details] [review]
Bug 11175: Remove outdated info about Elasticsearch not being suppported

Support for Elasticsearch was added in commit "Bug 11175: Add
Elasticsearch support".

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 121 Joonas Kylmälä 2021-02-12 12:09:56 UTC
There was a minor rebase conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/staff_interface.pref because a new syspref was introduced so I rebased that. I removed now from the syspref text in "Bug 11175: Remove outdated info about Elasticsearch not being suppported" the info that elasticsearch is not supported. These were super minor changes so I'm keeping the Signed off state. Ready for QA.
Comment 122 Andrew Nugged 2021-04-13 05:07:13 UTC Comment hidden (obsolete)
Comment 123 Andrew Nugged 2021-04-13 05:07:48 UTC
rebased one conflicting patch
Comment 124 Andrew Nugged 2021-04-13 07:24:18 UTC Comment hidden (obsolete)
Comment 125 Andrew Nugged 2021-04-13 07:42:11 UTC
Created attachment 119506 [details] [review]
Bug 11175: (follow-up) Add tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Comment 126 Katrin Fischer 2021-04-18 09:02:08 UTC
I am not able to download the records from Bugzilla. This might be a side effect of some spam protection measure :( Could you make them available somewhere else maybe?

I've also got a question about this:

>When looking up the component part records, the code searches for
>records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'
>or, if the 003 is not defined in the Host, 773$w=Host001.

I am in a hard spot here. Because Koha didn't care about 003 so far, our $w never have the 003, but it's defined in the records. Does this mean this won't work until we would change data or is there a way this could still work?

This does not only apply for our use case. When a library has used the "New child record" option from the detail page for cataloguing, 003 won't have been added to their $w.
Comment 127 Joonas Kylmälä 2021-04-19 13:36:29 UTC
(In reply to Katrin Fischer from comment #126)
> I am not able to download the records from Bugzilla. This might be a side
> effect of some spam protection measure :( Could you make them available
> somewhere else maybe?

https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78023&action=diff&collapsed=&headers=1&format=raw

> 
> I've also got a question about this:
> 
> >When looking up the component part records, the code searches for
> >records with (773$w=Host001 and 003=Host003) or 773$w='Host003 Host001'
> >or, if the 003 is not defined in the Host, 773$w=Host001.
> 
> I am in a hard spot here. Because Koha didn't care about 003 so far, our $w
> never have the 003, but it's defined in the records. Does this mean this
> won't work until we would change data or is there a way this could still
> work?

It will work, it supports both, the 003 will be used only for further disambiguation in case the records have it. See also bug 27507.
Comment 128 Joonas Kylmälä 2021-04-19 13:39:11 UTC
(In reply to Joonas Kylmälä from comment #127)
> It will work, it supports both, the 003 will be used only for further
> disambiguation in case the records have it. See also bug 27507.

Also to further elaborate, the adding of 003 is to fully support the marc format, see https://www.loc.gov/marc/bibliographic/bd773.html for examples of 773$w.
Comment 129 Martin Renvoize 2021-04-20 11:05:24 UTC
(In reply to Joonas Kylmälä from comment #128)
> (In reply to Joonas Kylmälä from comment #127)
> > It will work, it supports both, the 003 will be used only for further
> > disambiguation in case the records have it. See also bug 27507.
> 
> Also to further elaborate, the adding of 003 is to fully support the marc
> format, see https://www.loc.gov/marc/bibliographic/bd773.html for examples
> of 773$w.

I've wanted to add the use of 003 for ages but never got around to it.. Nice one!
Comment 130 Martin Renvoize 2021-04-20 11:06:33 UTC
As an asside, once we push this, it would be great to have another look at the relevant wiki pages an make sure they reflect these updates: https://wiki.koha-community.org/wiki/Multipart_Bibliographics
Comment 131 Katrin Fischer 2021-04-21 11:08:12 UTC
> It will work, it supports both, the 003 will be used only for further
> disambiguation in case the records have it. See also bug 27507.

We do have the 003 set, but not using prefixes in $w (to clarify). Does that change your reply?

I am aware 003 is standard, but as Koha does enforce 003 (the advanced editor at least does), but the plugins we have to crate analytics do not, there is some mismatch I want to check for. 

We should definitely move to be more standard compliant, but maybe we can have fallbacks to make this useful for people with the existing data meanwhile.
Comment 132 Joonas Kylmälä 2021-04-21 11:09:28 UTC
(In reply to Katrin Fischer from comment #131)
> > It will work, it supports both, the 003 will be used only for further
> > disambiguation in case the records have it. See also bug 27507.
> 
> We do have the 003 set, but not using prefixes in $w (to clarify). Does that
> change your reply?

No, it still works.
Comment 133 Katrin Fischer 2021-04-21 12:20:06 UTC
> > We do have the 003 set, but not using prefixes in $w (to clarify). Does that
> > change your reply?
> 
> No, it still works.

Awesome :)
Comment 134 Peter Vashchuk 2021-04-27 09:51:46 UTC
Created attachment 120206 [details] [review]
Bug 11175: (follow-up) make atomic update idempotent

Atomic update tries to insert line into system preferences even if it's
already there. This patch fixes that by adding 'IGNORE' to the query,
making it idempotent.