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: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature (vote)
Assignee: Martin Renvoize
QA Contact: Marcel de Rooy
URL:
Keywords:
: 21484 (view as bug list)
Depends on: 28484 28830
Blocks: 12625 31608 21484 29288 29314 29970 30327 30865 31543
  Show dependency treegraph
 
Reported: 2013-10-30 18:30 UTC by Olli-Antti Kivilahti
Modified: 2023-06-08 22:26 UTC (History)
21 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds the 'ShowComponentParts' system preference. When enabled, a record with analytical records has a new tab below the record detail containing links to the component parts records. The feature requires `MaxComponentRecords` is set to limit the maximum number of attached records to display; if more records are found then a link to the 'Show analytics' search will appear at the bottom of the listed analytics.
Version(s) released in:
21.11.00


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, Andrii Nugged
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.11 KB, patch)
2021-04-13 07:42 UTC, Andrii 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
Bug 11175: Show record component parts in the detail view (22.16 KB, patch)
2021-07-09 11:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.13 KB, patch)
2021-07-09 11:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.93 KB, patch)
2021-07-09 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.78 KB, patch)
2021-07-09 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.55 KB, patch)
2021-07-09 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.13 KB, patch)
2021-07-09 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (9.93 KB, patch)
2021-07-09 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.20 KB, patch)
2021-07-09 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.35 KB, patch)
2021-07-09 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.17 KB, patch)
2021-07-09 11:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.58 KB, patch)
2021-07-09 11:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.54 KB, patch)
2021-07-09 11:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (22.16 KB, patch)
2021-07-23 08:12 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.13 KB, patch)
2021-07-23 08:12 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.93 KB, patch)
2021-07-23 08:12 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.78 KB, patch)
2021-07-23 08:12 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.55 KB, patch)
2021-07-23 08:12 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.13 KB, patch)
2021-07-23 08:12 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.00 KB, patch)
2021-07-23 08:12 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.20 KB, patch)
2021-07-23 08:12 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.35 KB, patch)
2021-07-23 08:13 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.20 KB, patch)
2021-07-23 08:13 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.58 KB, patch)
2021-07-23 08:13 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.54 KB, patch)
2021-07-23 08:13 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc (887 bytes, patch)
2021-07-23 08:13 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.23 KB, patch)
2021-08-03 08:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (974 bytes, patch)
2021-08-03 10:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (22.16 KB, patch)
2021-08-05 16:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.13 KB, patch)
2021-08-05 16:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.93 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.78 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.55 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.13 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.00 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.20 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.35 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.20 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.58 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.54 KB, patch)
2021-08-05 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc (952 bytes, patch)
2021-08-05 16:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.29 KB, patch)
2021-08-05 16:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (1.02 KB, patch)
2021-08-05 16:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (7.75 KB, patch)
2021-08-05 16:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (7.87 KB, patch)
2021-08-05 16:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (9.63 KB, patch)
2021-08-06 07:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (22.16 KB, patch)
2021-08-06 07:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.13 KB, patch)
2021-08-06 07:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.93 KB, patch)
2021-08-06 07:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.78 KB, patch)
2021-08-06 07:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.55 KB, patch)
2021-08-06 07:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.13 KB, patch)
2021-08-06 07:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.00 KB, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.20 KB, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.35 KB, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.20 KB, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.58 KB, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.54 KB, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc (952 bytes, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.29 KB, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (1.02 KB, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (9.63 KB, patch)
2021-08-06 07:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Missed line in previous commit (859 bytes, patch)
2021-08-06 08:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) More Fixes (2.30 KB, patch)
2021-08-06 09:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (10.57 KB, patch)
2021-08-06 11:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (11.49 KB, patch)
2021-08-06 11:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (1.02 KB, patch)
2021-08-06 12:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (10.54 KB, patch)
2021-08-06 12:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (11.91 KB, patch)
2021-08-06 12:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.40 KB, patch)
2021-08-06 12:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (1.63 KB, patch)
2021-08-06 16:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Detail view of record with RTL and LTR script (78.13 KB, image/png)
2021-08-06 16:27 UTC, Katrin Fischer
Details
WIP: Display components in own tab (3.03 KB, patch)
2021-08-09 07:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (WIP) Display components in own tab (2.89 KB, patch)
2021-08-09 07:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (WIP) Display components in own tab (2.89 KB, patch)
2021-08-09 09:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (WIP) Remove special XSLT handling for parts (13.32 KB, patch)
2021-08-09 09:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Alternative display option (210.17 KB, image/png)
2021-08-09 09:38 UTC, Martin Renvoize
Details
Bug 11175: (WIP) OPAC (3.97 KB, patch)
2021-08-09 10:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (19.18 KB, patch)
2021-08-09 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.13 KB, patch)
2021-08-09 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.93 KB, patch)
2021-08-09 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.78 KB, patch)
2021-08-09 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.55 KB, patch)
2021-08-09 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.13 KB, patch)
2021-08-09 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.00 KB, patch)
2021-08-09 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.20 KB, patch)
2021-08-09 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.35 KB, patch)
2021-08-09 11:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.20 KB, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.58 KB, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.54 KB, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc (952 bytes, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.29 KB, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (1.02 KB, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (10.54 KB, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (12.32 KB, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.40 KB, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (1.63 KB, patch)
2021-08-09 11:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (WIP) Display components in own tab (2.89 KB, patch)
2021-08-09 11:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (WIP) Remove special XSLT handling for parts (13.32 KB, patch)
2021-08-09 11:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (WIP) OPAC (3.97 KB, patch)
2021-08-09 11:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (19.18 KB, patch)
2021-08-11 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.13 KB, patch)
2021-08-11 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.93 KB, patch)
2021-08-11 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.78 KB, patch)
2021-08-11 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.00 KB, patch)
2021-08-11 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.13 KB, patch)
2021-08-11 07:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.00 KB, patch)
2021-08-11 07:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.20 KB, patch)
2021-08-11 07:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.35 KB, patch)
2021-08-11 07:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.20 KB, patch)
2021-08-11 07:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.58 KB, patch)
2021-08-11 07:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.54 KB, patch)
2021-08-11 07:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc (952 bytes, patch)
2021-08-11 07:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.29 KB, patch)
2021-08-11 07:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (1.02 KB, patch)
2021-08-11 07:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (10.54 KB, patch)
2021-08-11 07:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (12.32 KB, patch)
2021-08-11 07:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.40 KB, patch)
2021-08-11 07:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (1.63 KB, patch)
2021-08-11 07:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Display components in own tab (19.75 KB, patch)
2021-08-11 07:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Rename back to get_marc_components (6.90 KB, patch)
2021-08-11 07:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (19.22 KB, patch)
2021-08-27 08:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.13 KB, patch)
2021-08-27 08:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.93 KB, patch)
2021-08-27 08:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.78 KB, patch)
2021-08-27 08:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.00 KB, patch)
2021-08-27 08:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.13 KB, patch)
2021-08-27 08:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.00 KB, patch)
2021-08-27 08:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.20 KB, patch)
2021-08-27 08:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.35 KB, patch)
2021-08-27 08:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.20 KB, patch)
2021-08-27 08:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.58 KB, patch)
2021-08-27 08:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.54 KB, patch)
2021-08-27 08:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc (952 bytes, patch)
2021-08-27 08:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.29 KB, patch)
2021-08-27 08:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (1.02 KB, patch)
2021-08-27 08:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (10.70 KB, patch)
2021-08-27 08:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (12.32 KB, patch)
2021-08-27 08:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.40 KB, patch)
2021-08-27 08:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (1.63 KB, patch)
2021-08-27 08:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Display components in own tab (19.78 KB, patch)
2021-08-27 08:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Rename back to get_marc_components (6.90 KB, patch)
2021-08-27 08:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fixes for bug 12561 (3.40 KB, patch)
2021-08-27 08:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (19.22 KB, patch)
2021-09-01 15:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.13 KB, patch)
2021-09-01 15:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.93 KB, patch)
2021-09-01 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.78 KB, patch)
2021-09-01 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.00 KB, patch)
2021-09-01 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.13 KB, patch)
2021-09-01 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.00 KB, patch)
2021-09-01 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.20 KB, patch)
2021-09-01 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.35 KB, patch)
2021-09-01 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.20 KB, patch)
2021-09-01 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.58 KB, patch)
2021-09-01 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.54 KB, patch)
2021-09-01 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc (952 bytes, patch)
2021-09-01 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.29 KB, patch)
2021-09-01 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (1.02 KB, patch)
2021-09-01 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (10.70 KB, patch)
2021-09-01 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (12.32 KB, patch)
2021-09-01 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.40 KB, patch)
2021-09-01 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (1.63 KB, patch)
2021-09-01 15:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Display components in own tab (19.78 KB, patch)
2021-09-01 15:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Rename back to get_marc_components (6.90 KB, patch)
2021-09-01 15:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fixes for bug 12561 (3.40 KB, patch)
2021-09-01 15:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Use clean_search_term (1.17 KB, patch)
2021-09-01 15:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (19.22 KB, patch)
2021-09-06 07:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.13 KB, patch)
2021-09-06 07:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.93 KB, patch)
2021-09-06 07:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.78 KB, patch)
2021-09-06 07:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.00 KB, patch)
2021-09-06 07:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.13 KB, patch)
2021-09-06 07:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.00 KB, patch)
2021-09-06 07:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.20 KB, patch)
2021-09-06 07:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.35 KB, patch)
2021-09-06 07:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.20 KB, patch)
2021-09-06 07:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.58 KB, patch)
2021-09-06 07:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.54 KB, patch)
2021-09-06 07:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc (952 bytes, patch)
2021-09-06 07:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.29 KB, patch)
2021-09-06 07:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (1.02 KB, patch)
2021-09-06 07:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (10.89 KB, patch)
2021-09-06 07:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (12.32 KB, patch)
2021-09-06 07:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.40 KB, patch)
2021-09-06 07:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (1.63 KB, patch)
2021-09-06 07:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Display components in own tab (19.78 KB, patch)
2021-09-06 07:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Rename back to get_marc_components (6.90 KB, patch)
2021-09-06 07:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fixes for bug 12561 (3.40 KB, patch)
2021-09-06 07:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Use clean_search_term (1.17 KB, patch)
2021-09-06 07:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (19.27 KB, patch)
2021-10-12 09:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.17 KB, patch)
2021-10-12 09:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.97 KB, patch)
2021-10-12 09:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.83 KB, patch)
2021-10-12 09:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.05 KB, patch)
2021-10-12 09:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.18 KB, patch)
2021-10-12 09:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.04 KB, patch)
2021-10-12 09:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.25 KB, patch)
2021-10-12 09:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.40 KB, patch)
2021-10-12 09:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.32 KB, patch)
2021-10-12 09:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.63 KB, patch)
2021-10-12 09:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.58 KB, patch)
2021-10-12 09:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc (1000 bytes, patch)
2021-10-12 09:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.34 KB, patch)
2021-10-12 09:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) We lost Encode somewhere (852 bytes, patch)
2021-10-12 09:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (11.13 KB, patch)
2021-10-12 09:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (12.38 KB, patch)
2021-10-12 09:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.45 KB, patch)
2021-10-12 09:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (3.07 KB, patch)
2021-10-12 09:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Display components in own tab (19.91 KB, patch)
2021-10-12 09:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Rename back to get_marc_components (7.02 KB, patch)
2021-10-12 09:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fixes for bug 12561 (3.51 KB, patch)
2021-10-12 09:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (follow-up) Use clean_search_term (1.46 KB, patch)
2021-10-12 09:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Account for difference in simple_search return (3.11 KB, patch)
2021-10-12 09:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Display components tab if no holdings (2.58 KB, patch)
2021-10-12 09:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) OPAC - Display components tab if no holdings (2.88 KB, patch)
2021-10-12 09:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Be more specific with title fields (969 bytes, patch)
2021-10-18 15:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Drop useless use (636 bytes, patch)
2021-10-18 15:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Restore link to more results (3.38 KB, patch)
2021-10-18 15:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit (5.30 KB, patch)
2021-10-22 07:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit (6.28 KB, patch)
2021-10-22 08:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit (7.38 KB, patch)
2021-10-22 08:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Uniformize ComponentParts in template (3.62 KB, patch)
2021-10-22 08:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (19.49 KB, patch)
2021-10-22 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.17 KB, patch)
2021-10-22 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (2.98 KB, patch)
2021-10-22 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.83 KB, patch)
2021-10-22 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.05 KB, patch)
2021-10-22 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.18 KB, patch)
2021-10-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.41 KB, patch)
2021-10-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.25 KB, patch)
2021-10-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.40 KB, patch)
2021-10-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.32 KB, patch)
2021-10-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.63 KB, patch)
2021-10-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.59 KB, patch)
2021-10-22 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.34 KB, patch)
2021-10-22 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (11.20 KB, patch)
2021-10-22 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (10.96 KB, patch)
2021-10-22 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.51 KB, patch)
2021-10-22 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (3.08 KB, patch)
2021-10-22 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Display components in own tab (20.25 KB, patch)
2021-10-22 09:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (follow-up) Use clean_search_term (1.46 KB, patch)
2021-10-22 09:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Account for difference in simple_search return (3.11 KB, patch)
2021-10-22 09:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Display components tab if no holdings (5.33 KB, patch)
2021-10-22 09:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Be more specific with title fields (971 bytes, patch)
2021-10-22 09:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Restore link to more results (3.39 KB, patch)
2021-10-22 09:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit (7.38 KB, patch)
2021-10-22 09:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Uniformize ComponentParts in template (5.33 KB, patch)
2021-10-22 09:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (20.18 KB, patch)
2021-10-22 10:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.27 KB, patch)
2021-10-22 10:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (3.07 KB, patch)
2021-10-22 10:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.92 KB, patch)
2021-10-22 10:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.14 KB, patch)
2021-10-22 10:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.27 KB, patch)
2021-10-22 10:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.49 KB, patch)
2021-10-22 10:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.35 KB, patch)
2021-10-22 10:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.49 KB, patch)
2021-10-22 10:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.42 KB, patch)
2021-10-22 10:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.72 KB, patch)
2021-10-22 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.68 KB, patch)
2021-10-22 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.44 KB, patch)
2021-10-22 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (11.28 KB, patch)
2021-10-22 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (11.05 KB, patch)
2021-10-22 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.60 KB, patch)
2021-10-22 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (3.17 KB, patch)
2021-10-22 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Display components in own tab (20.94 KB, patch)
2021-10-22 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Use clean_search_term (1.55 KB, patch)
2021-10-22 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Account for difference in simple_search return (3.21 KB, patch)
2021-10-22 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Display components tab if no holdings (5.42 KB, patch)
2021-10-22 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Be more specific with title fields (1.04 KB, patch)
2021-10-22 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Restore link to more results (3.47 KB, patch)
2021-10-22 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit (7.46 KB, patch)
2021-10-22 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Uniformize ComponentParts in template (5.42 KB, patch)
2021-10-22 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Simplify block in XSLT (1.72 KB, patch)
2021-10-22 10:08 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Remove caching _components (1.15 KB, patch)
2021-10-22 10:08 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Add tests and correct show_analytics logic (5.18 KB, patch)
2021-10-22 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: Add tests and correct show_analytics logic (5.18 KB, patch)
2021-10-22 10:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Remove caching _components (1.15 KB, patch)
2021-10-25 09:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Do not fetch twice (8.24 KB, patch)
2021-10-25 09:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Show record component parts in the detail view (20.18 KB, patch)
2021-10-26 07:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Don't return explicitly undef (1.27 KB, patch)
2021-10-26 07:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Return empty array if no components (3.07 KB, patch)
2021-10-26 07:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Relocate and reword ShowComponentRecords syspref description (2.92 KB, patch)
2021-10-26 07:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Add Elasticsearch support (3.14 KB, patch)
2021-10-26 07:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Keep component part record list items inside the box (1.27 KB, patch)
2021-10-26 07:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Limit the amount of component parts returned (10.49 KB, patch)
2021-10-26 07:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Search using double quotes to support Elasticsearch (2.35 KB, patch)
2021-10-26 07:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Remove outdated info about Elasticsearch not being suppported (1.49 KB, patch)
2021-10-26 07:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Add tests (3.42 KB, patch)
2021-10-26 07:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) make atomic update idempotent (1.72 KB, patch)
2021-10-26 07:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Rename routine get_marc_components (3.68 KB, patch)
2021-10-26 07:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move preference to 'Display' (2.44 KB, patch)
2021-10-26 07:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Take account of bug 15851 (11.72 KB, patch)
2021-10-26 07:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Move get_component_part_query (11.05 KB, patch)
2021-10-26 07:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Only get biblio once (1.60 KB, patch)
2021-10-26 07:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Fix queries (3.17 KB, patch)
2021-10-26 07:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Display components in own tab (20.94 KB, patch)
2021-10-26 07:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (follow-up) Use clean_search_term (1.55 KB, patch)
2021-10-26 07:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Account for difference in simple_search return (3.21 KB, patch)
2021-10-26 07:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Display components tab if no holdings (5.42 KB, patch)
2021-10-26 07:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Be more specific with title fields (1.04 KB, patch)
2021-10-26 07:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Restore link to more results (3.47 KB, patch)
2021-10-26 07:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit (7.46 KB, patch)
2021-10-26 07:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Uniformize ComponentParts in template (5.42 KB, patch)
2021-10-26 07:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Remove caching _components (1.15 KB, patch)
2021-10-26 07:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Do not fetch twice (8.24 KB, patch)
2021-10-26 07:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Restore bug 29284 (3.16 KB, patch)
2021-10-26 09:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Restore bug 29284 (3.50 KB, patch)
2021-10-26 09:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Restore bug 29284 (3.60 KB, patch)
2021-10-26 11:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Test message contents for analytics error (1.56 KB, patch)
2021-10-26 12:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: (QA follow-up) Test message contents for analytics error (2.01 KB, patch)
2021-10-26 12:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11175: Add tests (3.28 KB, patch)
2021-10-26 13:18 UTC, Jonathan Druart
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 Comment hidden (obsolete)
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 Comment hidden (obsolete)
Comment 35 paxed 2018-10-02 08:43:22 UTC Comment hidden (obsolete)
Comment 36 Martin Renvoize 2018-10-02 09:17:29 UTC Comment hidden (obsolete)
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 Comment hidden (obsolete)
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 Comment hidden (obsolete)
Comment 61 Joonas Kylmälä 2020-06-10 17:24:29 UTC Comment hidden (obsolete)
Comment 62 Joonas Kylmälä 2020-06-10 17:27:13 UTC Comment hidden (obsolete)
Comment 63 Joonas Kylmälä 2020-06-10 17:27:33 UTC Comment hidden (obsolete)
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 Comment hidden (obsolete)
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 Comment hidden (obsolete)
Comment 73 Joonas Kylmälä 2020-11-02 14:40:20 UTC Comment hidden (obsolete)
Comment 74 Joonas Kylmälä 2020-11-02 14:40:25 UTC Comment hidden (obsolete)
Comment 75 Joonas Kylmälä 2020-11-02 14:40:30 UTC Comment hidden (obsolete)
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 Comment hidden (obsolete)
Comment 78 Joonas Kylmälä 2020-11-02 15:32:55 UTC Comment hidden (obsolete)
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 Comment hidden (obsolete)
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 Comment hidden (obsolete)
Comment 84 Nick Clemens 2020-11-02 16:07:43 UTC Comment hidden (obsolete)
Comment 85 Nick Clemens 2020-11-02 16:07:48 UTC Comment hidden (obsolete)
Comment 86 Nick Clemens 2020-11-02 16:07:52 UTC Comment hidden (obsolete)
Comment 87 Nick Clemens 2020-11-02 16:07:57 UTC Comment hidden (obsolete)
Comment 88 Nick Clemens 2020-11-02 16:08:02 UTC Comment hidden (obsolete)
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 Comment hidden (obsolete)
Comment 97 Joonas Kylmälä 2020-11-16 08:43:38 UTC Comment hidden (obsolete)
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 Comment hidden (obsolete)
Comment 101 paxed 2020-12-11 11:11:01 UTC Comment hidden (obsolete)
Comment 102 paxed 2020-12-11 11:11:06 UTC Comment hidden (obsolete)
Comment 103 paxed 2020-12-11 11:11:11 UTC Comment hidden (obsolete)
Comment 104 paxed 2020-12-11 11:11:16 UTC Comment hidden (obsolete)
Comment 105 paxed 2020-12-11 11:11:21 UTC Comment hidden (obsolete)
Comment 106 paxed 2020-12-11 11:11:26 UTC Comment hidden (obsolete)
Comment 107 paxed 2020-12-11 11:11:32 UTC Comment hidden (obsolete)
Comment 108 paxed 2020-12-11 11:11:37 UTC Comment hidden (obsolete)
Comment 109 paxed 2020-12-11 11:11:42 UTC Comment hidden (obsolete)
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 Comment hidden (obsolete)
Comment 112 Joonas Kylmälä 2021-02-12 12:06:23 UTC Comment hidden (obsolete)
Comment 113 Joonas Kylmälä 2021-02-12 12:06:27 UTC Comment hidden (obsolete)
Comment 114 Joonas Kylmälä 2021-02-12 12:06:33 UTC Comment hidden (obsolete)
Comment 115 Joonas Kylmälä 2021-02-12 12:06:38 UTC Comment hidden (obsolete)
Comment 116 Joonas Kylmälä 2021-02-12 12:06:43 UTC Comment hidden (obsolete)
Comment 117 Joonas Kylmälä 2021-02-12 12:06:48 UTC Comment hidden (obsolete)
Comment 118 Joonas Kylmälä 2021-02-12 12:06:53 UTC Comment hidden (obsolete)
Comment 119 Joonas Kylmälä 2021-02-12 12:06:59 UTC Comment hidden (obsolete)
Comment 120 Joonas Kylmälä 2021-02-12 12:07:03 UTC Comment hidden (obsolete)
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 Andrii Nugged 2021-04-13 05:07:13 UTC Comment hidden (obsolete)
Comment 123 Andrii Nugged 2021-04-13 05:07:48 UTC
rebased one conflicting patch
Comment 124 Andrii Nugged 2021-04-13 07:24:18 UTC Comment hidden (obsolete)
Comment 125 Andrii Nugged 2021-04-13 07:42:11 UTC Comment hidden (obsolete)
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 Comment hidden (obsolete)
Comment 135 Martin Renvoize 2021-07-09 11:36:52 UTC Comment hidden (obsolete)
Comment 136 Martin Renvoize 2021-07-09 11:36:59 UTC Comment hidden (obsolete)
Comment 137 Martin Renvoize 2021-07-09 11:37:05 UTC Comment hidden (obsolete)
Comment 138 Martin Renvoize 2021-07-09 11:37:11 UTC Comment hidden (obsolete)
Comment 139 Martin Renvoize 2021-07-09 11:37:17 UTC Comment hidden (obsolete)
Comment 140 Martin Renvoize 2021-07-09 11:37:28 UTC Comment hidden (obsolete)
Comment 141 Martin Renvoize 2021-07-09 11:37:38 UTC Comment hidden (obsolete)
Comment 142 Martin Renvoize 2021-07-09 11:37:44 UTC Comment hidden (obsolete)
Comment 143 Martin Renvoize 2021-07-09 11:37:54 UTC Comment hidden (obsolete)
Comment 144 Martin Renvoize 2021-07-09 11:38:04 UTC Comment hidden (obsolete)
Comment 145 Martin Renvoize 2021-07-09 11:38:12 UTC Comment hidden (obsolete)
Comment 146 Martin Renvoize 2021-07-09 11:38:20 UTC Comment hidden (obsolete)
Comment 147 Martin Renvoize 2021-07-09 11:44:46 UTC
This seems pretty solid to me.. remember to build the CSS on push ;).

Also, there's a weird QA script fail for me on POD encoding.. but I can't spot any weird characters to I'm a bit confused by it.  Leaving to the RM for a final check there.

Passing QA
Comment 148 Jonathan Druart 2021-07-12 13:13:41 UTC
Koha::Util::Search feels wrong, why not under Koha::SearchEngine?
Comment 149 Jonathan Druart 2021-07-12 13:14:18 UTC
(In reply to Martin Renvoize from comment #147)
> Also, there's a weird QA script fail for me on POD encoding.. but I can't
> spot any weird characters to I'm a bit confused by it.

There is a "–" vs "-".
Comment 150 Joonas Kylmälä 2021-07-23 08:12:02 UTC Comment hidden (obsolete)
Comment 151 Joonas Kylmälä 2021-07-23 08:12:10 UTC Comment hidden (obsolete)
Comment 152 Joonas Kylmälä 2021-07-23 08:12:17 UTC Comment hidden (obsolete)
Comment 153 Joonas Kylmälä 2021-07-23 08:12:25 UTC Comment hidden (obsolete)
Comment 154 Joonas Kylmälä 2021-07-23 08:12:32 UTC Comment hidden (obsolete)
Comment 155 Joonas Kylmälä 2021-07-23 08:12:39 UTC Comment hidden (obsolete)
Comment 156 Joonas Kylmälä 2021-07-23 08:12:46 UTC Comment hidden (obsolete)
Comment 157 Joonas Kylmälä 2021-07-23 08:12:53 UTC Comment hidden (obsolete)
Comment 158 Joonas Kylmälä 2021-07-23 08:13:00 UTC Comment hidden (obsolete)
Comment 159 Joonas Kylmälä 2021-07-23 08:13:07 UTC Comment hidden (obsolete)
Comment 160 Joonas Kylmälä 2021-07-23 08:13:15 UTC Comment hidden (obsolete)
Comment 161 Joonas Kylmälä 2021-07-23 08:13:22 UTC Comment hidden (obsolete)
Comment 162 Joonas Kylmälä 2021-07-23 08:13:29 UTC Comment hidden (obsolete)
Comment 163 Joonas Kylmälä 2021-07-23 08:14:15 UTC
(In reply to Jonathan Druart from comment #149)
> (In reply to Martin Renvoize from comment #147)
> > Also, there's a weird QA script fail for me on POD encoding.. but I can't
> > spot any weird characters to I'm a bit confused by it.
> 
> There is a "–" vs "-".

Fixed this in the latest patch, thanks for spotting.
Comment 164 Joonas Kylmälä 2021-07-23 08:23:07 UTC
(In reply to Jonathan Druart from comment #148)
> Koha::Util::Search feels wrong, why not under Koha::SearchEngine?

Because Koha::SearchEngine felt wrong too, it has functions to interact with the Zebra and Elasticsearch APIs, not to create specific queries. And then another question: why not have Koha::Search::something, well, I think Koha::Util::Search being under Koha::Util already helps to tell it is a collection of helper functions, and also then all our helper functions are in the same place to be found.
Comment 165 Martin Renvoize 2021-08-03 08:04:54 UTC Comment hidden (obsolete)
Comment 166 Martin Renvoize 2021-08-03 10:26:38 UTC Comment hidden (obsolete)
Comment 167 Martin Renvoize 2021-08-03 11:48:39 UTC
QA Question

Should 'Analytics: Show analytics' still display even when the syspref is enabled to display them inline?
Comment 168 Martin Renvoize 2021-08-03 12:26:52 UTC
OK, having dug a bit further I'm a little less sure about this patch set.. I love it in theory.. but I feel we're perhaps duplicating code a bit..

The code that governs the 'show analytics' link display already does a similar search but does it within the two -details' controller files.. I feel we should move to using the Util function and XSLT method from here for both cases.. and likely take some notice of UseControlNumber.
Comment 169 Martin Renvoize 2021-08-03 12:40:44 UTC
Comment on attachment 123087 [details] [review]
Bug 11175: Limit the amount of component parts returned

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

::: Koha/Util/Search.pm
@@ +34,5 @@
> +=cut
> +
> +sub get_component_part_query {
> +    my ($biblionumber) = @_;
> +

I think perhaps we should be taking UseControlNumber into account here...

See opac-details.pl and details.pl controllers for a near the same case.

  my $cleaned_title = $biblio->title;
  $cleaned_title =~ tr|/||;
  my $query =
    ( C4::Context->preference('UseControlNumber') and $record->field('001') )
    ? 'rcn:'. $record->field('001')->data . ' AND (bib-level:a OR bib-level:b)'
    : "Host-item:($cleaned_title)";

We should really utilise something similar here I feel.. and even better.. start using this in those two locations too.. or rather.. move that handling into XSLT.pm as you have done for the overall show parts code.
Comment 170 Martin Renvoize 2021-08-05 16:19:52 UTC Comment hidden (obsolete)
Comment 171 Martin Renvoize 2021-08-05 16:19:57 UTC Comment hidden (obsolete)
Comment 172 Martin Renvoize 2021-08-05 16:20:03 UTC Comment hidden (obsolete)
Comment 173 Martin Renvoize 2021-08-05 16:20:09 UTC Comment hidden (obsolete)
Comment 174 Martin Renvoize 2021-08-05 16:20:15 UTC Comment hidden (obsolete)
Comment 175 Martin Renvoize 2021-08-05 16:20:21 UTC Comment hidden (obsolete)
Comment 176 Martin Renvoize 2021-08-05 16:20:27 UTC Comment hidden (obsolete)
Comment 177 Martin Renvoize 2021-08-05 16:20:33 UTC Comment hidden (obsolete)
Comment 178 Martin Renvoize 2021-08-05 16:20:39 UTC Comment hidden (obsolete)
Comment 179 Martin Renvoize 2021-08-05 16:20:44 UTC Comment hidden (obsolete)
Comment 180 Martin Renvoize 2021-08-05 16:20:50 UTC Comment hidden (obsolete)
Comment 181 Martin Renvoize 2021-08-05 16:20:56 UTC Comment hidden (obsolete)
Comment 182 Martin Renvoize 2021-08-05 16:21:02 UTC Comment hidden (obsolete)
Comment 183 Martin Renvoize 2021-08-05 16:21:08 UTC Comment hidden (obsolete)
Comment 184 Martin Renvoize 2021-08-05 16:21:13 UTC Comment hidden (obsolete)
Comment 185 Martin Renvoize 2021-08-05 16:21:19 UTC Comment hidden (obsolete)
Comment 186 Martin Renvoize 2021-08-05 16:24:43 UTC Comment hidden (obsolete)
Comment 187 Martin Renvoize 2021-08-05 16:25:41 UTC
t/Koha/Util/Search.t needs updating for that final patch.. one to attack tomorrow.
Comment 188 Martin Renvoize 2021-08-06 07:07:42 UTC Comment hidden (obsolete)
Comment 189 Martin Renvoize 2021-08-06 07:12:25 UTC Comment hidden (obsolete)
Comment 190 Martin Renvoize 2021-08-06 07:12:31 UTC Comment hidden (obsolete)
Comment 191 Martin Renvoize 2021-08-06 07:12:37 UTC Comment hidden (obsolete)
Comment 192 Martin Renvoize 2021-08-06 07:12:43 UTC Comment hidden (obsolete)
Comment 193 Martin Renvoize 2021-08-06 07:12:49 UTC Comment hidden (obsolete)
Comment 194 Martin Renvoize 2021-08-06 07:12:55 UTC Comment hidden (obsolete)
Comment 195 Martin Renvoize 2021-08-06 07:13:01 UTC Comment hidden (obsolete)
Comment 196 Martin Renvoize 2021-08-06 07:13:07 UTC Comment hidden (obsolete)
Comment 197 Martin Renvoize 2021-08-06 07:13:13 UTC Comment hidden (obsolete)
Comment 198 Martin Renvoize 2021-08-06 07:13:19 UTC Comment hidden (obsolete)
Comment 199 Martin Renvoize 2021-08-06 07:13:25 UTC Comment hidden (obsolete)
Comment 200 Martin Renvoize 2021-08-06 07:13:31 UTC Comment hidden (obsolete)
Comment 201 Martin Renvoize 2021-08-06 07:13:38 UTC Comment hidden (obsolete)
Comment 202 Martin Renvoize 2021-08-06 07:13:43 UTC Comment hidden (obsolete)
Comment 203 Martin Renvoize 2021-08-06 07:13:49 UTC Comment hidden (obsolete)
Comment 204 Martin Renvoize 2021-08-06 07:13:55 UTC Comment hidden (obsolete)
Comment 205 Martin Renvoize 2021-08-06 08:27:30 UTC
Created attachment 123562 [details] [review]
Bug 11175: (follow-up) Missed line in previous commit
Comment 206 Jonathan Druart 2021-08-06 09:26:17 UTC
1. It needs more work, there is something broken (search does not return anything)

2. The UI is not the same as what is advertised in the screenshot, the component part records are display at the top (not on the right)

3. I don't think we need the overhead we introduced with the "show all" link. We call get_component_part_query from C4::XSLT (for that link) and Koha::Biblio (for the list we display). I would suggest to retrieve them all and have a expand/collapse link if we have "too many" element to display.
We could then move the get_component_part_query into Koha::Biblio->get_marc_components (Koha::Biblio->get_marc_host is doing that already).
Comment 207 Martin Renvoize 2021-08-06 09:28:40 UTC
Created attachment 123564 [details] [review]
Bug 11175: (follow-up) More Fixes

* Default to an empty arrayref from get_marc_components
* Standardise search syntax in Koha::Util::Search
Comment 208 Martin Renvoize 2021-08-06 09:46:50 UTC
(In reply to Jonathan Druart from comment #206)
> 1. It needs more work, there is something broken (search does not return
> anything)

OK, I've done more digging here.. in short, I think we need to make a distinction between "Analytics" and "Volumes of sets and traced series" akin to the change bug 7609 made way back.  At least for the UseControlNumber case.. that's what the search results end up empty as I moved the query without fully understanding it.

> 2. The UI is not the same as what is advertised in the screenshot, the
> component part records are display at the top (not on the right)

This is just a missing CSS build.. I feel down that hole too ;)

> 3. I don't think we need the overhead we introduced with the "show all"
> link. We call get_component_part_query from C4::XSLT (for that link) and
> Koha::Biblio (for the list we display). I would suggest retrieving them all
> and have a expand/collapse link if we have "too many" element to display.
> We could then move the get_component_part_query into
> Koha::Biblio->get_marc_components (Koha::Biblio->get_marc_host is doing that
> already).

I think in theory the reason wasn't to reduce display size but to reduce search return scope for performance. In reality, I'm not sure this is a real issue.. need to read the rest of the comments that asked for the reduction to be introduced.. it all feels very artificial.
Comment 209 Martin Renvoize 2021-08-06 11:12:26 UTC
(In reply to Martin Renvoize from comment #208)
> (In reply to Jonathan Druart from comment #206)
> > 1. It needs more work, there is something broken (search does not return
> > anything)
> 
> OK, I've done more digging here.. in short, I think we need to make a
> distinction between "Analytics" and "Volumes of sets and traced series" akin
> to the change bug 7609 made way back.  At least for the UseControlNumber
> case.. that's what the search results end up empty as I moved the query
> without fully understanding it.

Actually.. I think this is working as expected.. I just had other issues to resolve.

> > 3. I don't think we need the overhead we introduced with the "show all"
> > link. We call get_component_part_query from C4::XSLT (for that link) and
> > Koha::Biblio (for the list we display). I would suggest retrieving them all
> > and have a expand/collapse link if we have "too many" element to display.
> > We could then move the get_component_part_query into
> > Koha::Biblio->get_marc_components (Koha::Biblio->get_marc_host is doing that
> > already).
> 
> I think in theory the reason wasn't to reduce display size but to reduce
> search return scope for performance. In reality, I'm not sure this is a real
> issue.. need to read the rest of the comments that asked for the reduction
> to be introduced.. it all feels very artificial.

See comment 90. ES is hard coded to return 100 max unless passed a new max.. there's not option to pass 'all'.. Hense where we're all with passing a search string to get all.
Comment 210 Martin Renvoize 2021-08-06 11:45:58 UTC Comment hidden (obsolete)
Comment 211 Martin Renvoize 2021-08-06 11:46:06 UTC Comment hidden (obsolete)
Comment 212 Martin Renvoize 2021-08-06 12:10:43 UTC Comment hidden (obsolete)
Comment 213 Martin Renvoize 2021-08-06 12:10:49 UTC Comment hidden (obsolete)
Comment 214 Martin Renvoize 2021-08-06 12:10:57 UTC Comment hidden (obsolete)
Comment 215 Martin Renvoize 2021-08-06 12:11:05 UTC Comment hidden (obsolete)
Comment 216 Jonathan Druart 2021-08-06 12:51:56 UTC
-sub get_marc_components {
+sub get_marc_analytics {

Do we really want to use "analytics" here? It's not related to the EasyAnalytics feature, and "components" is used all over the place (perl and xslt variables, css classes, syspref, etc.)
Comment 217 Jonathan Druart 2021-08-06 12:54:29 UTC
On the other hand, "component" is confusing as well (at least for me).
Doesn't an official wording (in the MARC world) exist?
Comment 218 Katrin Fischer 2021-08-06 14:27:54 UTC
(In reply to Jonathan Druart from comment #217)
> On the other hand, "component" is confusing as well (at least for me).
> Doesn't an official wording (in the MARC world) exist?

Component/component part appears in the LOC documentation, while analytics doesn't:
LDR, pos. 7
https://www.loc.gov/marc/bibliographic/bdleader.html
773
https://www.loc.gov/marc/bibliographic/bd773.html
Comment 219 Martin Renvoize 2021-08-06 14:39:35 UTC
Fair enough.. I renamed it to analytics as the query limits to bib-level:a + bib-level:b now which corresponds to analytics in my understanding.. if we want all components then we should probably drop that filtering.  Katrin, I'd love your thoughts on that.
Comment 220 Martin Renvoize 2021-08-06 15:39:04 UTC
right.. back to FQA.. been testing with Katrin.

We found the searches don't work fully yet.

* rcn:003 001 - Works as expected
* rcn:(003)001 - Is needed in addition to above
* (rcn:001 AND cni:003) - Doesn't appear to be working
Comment 221 Martin Renvoize 2021-08-06 16:02:10 UTC Comment hidden (obsolete)
Comment 222 Martin Renvoize 2021-08-06 16:03:53 UTC
OK, that fixes the query issue.. for Zebra at least.. we'll test Elastic next.

However.. another issue we've spotted.

This does not work well with RLT languages..

If you have a record with both LTR and RTL in then the component parts box will stick itself between the two.
Comment 223 Katrin Fischer 2021-08-06 16:27:37 UTC
Created attachment 123583 [details]
Detail view of record with RTL and LTR script

Adding a screenshot to explain the issue a little better. 

I believe usability of the scroll box might not be too well if you have really a lot of records, because it limits you to a relatively small "view". 

Also slightly worried about handling on mobile interfaces as it sticks to the 2 column layout and won't stack as other elements on smaller sizes.

I haven't checked all the comments yet, but could we imagine a different placing? Maybe below or above the items?
Comment 224 Martin Renvoize 2021-08-09 07:29:41 UTC
I'm starting to ask myself if doing this within the XSLT is right at all...

It feels like perhaps we should be placing these details inside a tab alongside holdings, description and friends.  Right now, we're passing allot of data to the front end and then displaying very minimal details from it via XSLT.

I'm thinking that perhaps we should either get selective about what fields of the component part records we pass and just display a nice table or we can continue to pass whole records, but split the xslt into it's own file (or re-use the results xslt perhaps) and use that for more of an embedded results display in this tab.

Think I'm going to have to tag out of this for now, I need to get back to other things.
Comment 225 Martin Renvoize 2021-08-09 07:31:25 UTC Comment hidden (obsolete)
Comment 226 Martin Renvoize 2021-08-09 07:56:51 UTC Comment hidden (obsolete)
Comment 227 Martin Renvoize 2021-08-09 09:34:48 UTC Comment hidden (obsolete)
Comment 228 Martin Renvoize 2021-08-09 09:34:55 UTC Comment hidden (obsolete)
Comment 229 Martin Renvoize 2021-08-09 09:38:33 UTC Comment hidden (obsolete)
Comment 230 Martin Renvoize 2021-08-09 09:44:04 UTC
All patches can be applied now, the final two are an alternative implementation based on the work that preceded.

Test plan is as before, grab the test mrc records and import them, build the indexes then search for 'Easy piano'. Navigate to the host record and see what it looks like.. I new 'Components (200)' tab should appear and clicking on it should display a list of all the attached records.

The big difference with this implementation is that we no longer embed the analytics in the records passed to the XSLT parser. Instead, we pass each record in an array to the template and them use the Results XSLT to format their output.

It might be nice to have a simplified template for this display instead of using Results, but as a proof of concept I felt this was enough to showcase the alternative method.
Comment 231 Martin Renvoize 2021-08-09 09:45:35 UTC
I'm also wondering about the need for 'ShowComponentParts' pref at all now.. 

We already do the search anyway to ascertain whether we should display the 'Show analytics' link or not so we're not really saving anything performance-wise by disabling the display.
Comment 232 Martin Renvoize 2021-08-09 10:08:38 UTC Comment hidden (obsolete)
Comment 233 Martin Renvoize 2021-08-09 10:16:02 UTC Comment hidden (obsolete)
Comment 234 Martin Renvoize 2021-08-09 11:13:04 UTC Comment hidden (obsolete)
Comment 235 Martin Renvoize 2021-08-09 11:13:10 UTC Comment hidden (obsolete)
Comment 236 Martin Renvoize 2021-08-09 11:13:16 UTC Comment hidden (obsolete)
Comment 237 Martin Renvoize 2021-08-09 11:13:23 UTC Comment hidden (obsolete)
Comment 238 Martin Renvoize 2021-08-09 11:13:29 UTC Comment hidden (obsolete)
Comment 239 Martin Renvoize 2021-08-09 11:13:35 UTC Comment hidden (obsolete)
Comment 240 Martin Renvoize 2021-08-09 11:13:41 UTC Comment hidden (obsolete)
Comment 241 Martin Renvoize 2021-08-09 11:13:47 UTC Comment hidden (obsolete)
Comment 242 Martin Renvoize 2021-08-09 11:13:54 UTC Comment hidden (obsolete)
Comment 243 Martin Renvoize 2021-08-09 11:14:00 UTC Comment hidden (obsolete)
Comment 244 Martin Renvoize 2021-08-09 11:14:06 UTC Comment hidden (obsolete)
Comment 245 Martin Renvoize 2021-08-09 11:14:13 UTC Comment hidden (obsolete)
Comment 246 Martin Renvoize 2021-08-09 11:14:19 UTC Comment hidden (obsolete)
Comment 247 Martin Renvoize 2021-08-09 11:14:25 UTC Comment hidden (obsolete)
Comment 248 Martin Renvoize 2021-08-09 11:14:31 UTC Comment hidden (obsolete)
Comment 249 Martin Renvoize 2021-08-09 11:14:38 UTC Comment hidden (obsolete)
Comment 250 Martin Renvoize 2021-08-09 11:14:45 UTC Comment hidden (obsolete)
Comment 251 Martin Renvoize 2021-08-09 11:14:51 UTC Comment hidden (obsolete)
Comment 252 Martin Renvoize 2021-08-09 11:14:58 UTC Comment hidden (obsolete)
Comment 253 Martin Renvoize 2021-08-09 11:15:04 UTC Comment hidden (obsolete)
Comment 254 Martin Renvoize 2021-08-09 11:15:11 UTC Comment hidden (obsolete)
Comment 255 Martin Renvoize 2021-08-09 11:15:17 UTC Comment hidden (obsolete)
Comment 256 Martin Renvoize 2021-08-11 07:04:25 UTC Comment hidden (obsolete)
Comment 257 Martin Renvoize 2021-08-11 07:04:32 UTC Comment hidden (obsolete)
Comment 258 Martin Renvoize 2021-08-11 07:04:39 UTC Comment hidden (obsolete)
Comment 259 Martin Renvoize 2021-08-11 07:04:45 UTC Comment hidden (obsolete)
Comment 260 Martin Renvoize 2021-08-11 07:04:51 UTC Comment hidden (obsolete)
Comment 261 Martin Renvoize 2021-08-11 07:04:58 UTC Comment hidden (obsolete)
Comment 262 Martin Renvoize 2021-08-11 07:05:05 UTC Comment hidden (obsolete)
Comment 263 Martin Renvoize 2021-08-11 07:05:11 UTC Comment hidden (obsolete)
Comment 264 Martin Renvoize 2021-08-11 07:05:18 UTC Comment hidden (obsolete)
Comment 265 Martin Renvoize 2021-08-11 07:05:24 UTC Comment hidden (obsolete)
Comment 266 Martin Renvoize 2021-08-11 07:05:30 UTC Comment hidden (obsolete)
Comment 267 Martin Renvoize 2021-08-11 07:05:37 UTC Comment hidden (obsolete)
Comment 268 Martin Renvoize 2021-08-11 07:05:43 UTC Comment hidden (obsolete)
Comment 269 Martin Renvoize 2021-08-11 07:05:49 UTC Comment hidden (obsolete)
Comment 270 Martin Renvoize 2021-08-11 07:05:56 UTC Comment hidden (obsolete)
Comment 271 Martin Renvoize 2021-08-11 07:06:02 UTC Comment hidden (obsolete)
Comment 272 Martin Renvoize 2021-08-11 07:06:09 UTC Comment hidden (obsolete)
Comment 273 Martin Renvoize 2021-08-11 07:06:15 UTC Comment hidden (obsolete)
Comment 274 Martin Renvoize 2021-08-11 07:06:22 UTC Comment hidden (obsolete)
Comment 275 Martin Renvoize 2021-08-11 07:06:28 UTC Comment hidden (obsolete)
Comment 276 Martin Renvoize 2021-08-11 07:06:34 UTC Comment hidden (obsolete)
Comment 277 Andrii Nugged 2021-08-21 09:33:08 UTC
Can't apply on current master
Comment 278 Martin Renvoize 2021-08-27 08:36:35 UTC Comment hidden (obsolete)
Comment 279 Martin Renvoize 2021-08-27 08:36:44 UTC Comment hidden (obsolete)
Comment 280 Martin Renvoize 2021-08-27 08:36:51 UTC Comment hidden (obsolete)
Comment 281 Martin Renvoize 2021-08-27 08:36:59 UTC Comment hidden (obsolete)
Comment 282 Martin Renvoize 2021-08-27 08:37:08 UTC Comment hidden (obsolete)
Comment 283 Martin Renvoize 2021-08-27 08:37:16 UTC Comment hidden (obsolete)
Comment 284 Martin Renvoize 2021-08-27 08:37:26 UTC Comment hidden (obsolete)
Comment 285 Martin Renvoize 2021-08-27 08:37:39 UTC Comment hidden (obsolete)
Comment 286 Martin Renvoize 2021-08-27 08:37:47 UTC Comment hidden (obsolete)
Comment 287 Martin Renvoize 2021-08-27 08:37:57 UTC Comment hidden (obsolete)
Comment 288 Martin Renvoize 2021-08-27 08:38:11 UTC Comment hidden (obsolete)
Comment 289 Martin Renvoize 2021-08-27 08:38:19 UTC Comment hidden (obsolete)
Comment 290 Martin Renvoize 2021-08-27 08:38:27 UTC Comment hidden (obsolete)
Comment 291 Martin Renvoize 2021-08-27 08:38:34 UTC Comment hidden (obsolete)
Comment 292 Martin Renvoize 2021-08-27 08:38:41 UTC Comment hidden (obsolete)
Comment 293 Martin Renvoize 2021-08-27 08:38:48 UTC Comment hidden (obsolete)
Comment 294 Martin Renvoize 2021-08-27 08:38:55 UTC Comment hidden (obsolete)
Comment 295 Martin Renvoize 2021-08-27 08:39:03 UTC Comment hidden (obsolete)
Comment 296 Martin Renvoize 2021-08-27 08:39:12 UTC Comment hidden (obsolete)
Comment 297 Martin Renvoize 2021-08-27 08:39:21 UTC Comment hidden (obsolete)
Comment 298 Martin Renvoize 2021-08-27 08:39:31 UTC Comment hidden (obsolete)
Comment 299 Martin Renvoize 2021-08-27 08:39:40 UTC Comment hidden (obsolete)
Comment 300 Martin Renvoize 2021-08-27 08:42:48 UTC
Rebased on master :)
Comment 301 Martin Renvoize 2021-09-01 15:17:50 UTC Comment hidden (obsolete)
Comment 302 Martin Renvoize 2021-09-01 15:17:58 UTC Comment hidden (obsolete)
Comment 303 Martin Renvoize 2021-09-01 15:18:05 UTC Comment hidden (obsolete)
Comment 304 Martin Renvoize 2021-09-01 15:18:12 UTC Comment hidden (obsolete)
Comment 305 Martin Renvoize 2021-09-01 15:18:19 UTC Comment hidden (obsolete)
Comment 306 Martin Renvoize 2021-09-01 15:18:27 UTC Comment hidden (obsolete)
Comment 307 Martin Renvoize 2021-09-01 15:18:34 UTC Comment hidden (obsolete)
Comment 308 Martin Renvoize 2021-09-01 15:18:41 UTC Comment hidden (obsolete)
Comment 309 Martin Renvoize 2021-09-01 15:18:48 UTC Comment hidden (obsolete)
Comment 310 Martin Renvoize 2021-09-01 15:18:55 UTC Comment hidden (obsolete)
Comment 311 Martin Renvoize 2021-09-01 15:19:03 UTC Comment hidden (obsolete)
Comment 312 Martin Renvoize 2021-09-01 15:19:10 UTC Comment hidden (obsolete)
Comment 313 Martin Renvoize 2021-09-01 15:19:17 UTC Comment hidden (obsolete)
Comment 314 Martin Renvoize 2021-09-01 15:19:25 UTC Comment hidden (obsolete)
Comment 315 Martin Renvoize 2021-09-01 15:19:32 UTC Comment hidden (obsolete)
Comment 316 Martin Renvoize 2021-09-01 15:19:40 UTC Comment hidden (obsolete)
Comment 317 Martin Renvoize 2021-09-01 15:19:47 UTC Comment hidden (obsolete)
Comment 318 Martin Renvoize 2021-09-01 15:19:54 UTC Comment hidden (obsolete)
Comment 319 Martin Renvoize 2021-09-01 15:20:02 UTC Comment hidden (obsolete)
Comment 320 Martin Renvoize 2021-09-01 15:20:09 UTC Comment hidden (obsolete)
Comment 321 Martin Renvoize 2021-09-01 15:20:18 UTC Comment hidden (obsolete)
Comment 322 Martin Renvoize 2021-09-01 15:20:26 UTC Comment hidden (obsolete)
Comment 323 Martin Renvoize 2021-09-01 15:20:34 UTC Comment hidden (obsolete)
Comment 324 Martin Renvoize 2021-09-02 10:03:39 UTC
Another rebase and update of dependencies to account for issue found during QA and testing.
Comment 325 Andrii Nugged 2021-09-04 20:22:29 UTC
Martin, your rebase applies on master silently,
but conflicts (catalogue/detail.pl) when to apply over dependant (as in ticket  Bug 28484).

Ah, now I figured more: patch 124397 removes most of Bug 28484 changed code,


... so it looks like we just need to remove dependency reference from this BZ ticket?
Comment 326 Martin Renvoize 2021-09-06 07:39:32 UTC Comment hidden (obsolete)
Comment 327 Martin Renvoize 2021-09-06 07:39:39 UTC Comment hidden (obsolete)
Comment 328 Martin Renvoize 2021-09-06 07:39:47 UTC Comment hidden (obsolete)
Comment 329 Martin Renvoize 2021-09-06 07:39:53 UTC Comment hidden (obsolete)
Comment 330 Martin Renvoize 2021-09-06 07:40:00 UTC Comment hidden (obsolete)
Comment 331 Martin Renvoize 2021-09-06 07:40:08 UTC Comment hidden (obsolete)
Comment 332 Martin Renvoize 2021-09-06 07:40:15 UTC Comment hidden (obsolete)
Comment 333 Martin Renvoize 2021-09-06 07:40:23 UTC Comment hidden (obsolete)
Comment 334 Martin Renvoize 2021-09-06 07:40:30 UTC Comment hidden (obsolete)
Comment 335 Martin Renvoize 2021-09-06 07:40:40 UTC Comment hidden (obsolete)
Comment 336 Martin Renvoize 2021-09-06 07:40:47 UTC Comment hidden (obsolete)
Comment 337 Martin Renvoize 2021-09-06 07:40:55 UTC Comment hidden (obsolete)
Comment 338 Martin Renvoize 2021-09-06 07:41:03 UTC Comment hidden (obsolete)
Comment 339 Martin Renvoize 2021-09-06 07:41:10 UTC Comment hidden (obsolete)
Comment 340 Martin Renvoize 2021-09-06 07:41:17 UTC Comment hidden (obsolete)
Comment 341 Martin Renvoize 2021-09-06 07:41:27 UTC Comment hidden (obsolete)
Comment 342 Martin Renvoize 2021-09-06 07:41:35 UTC Comment hidden (obsolete)
Comment 343 Martin Renvoize 2021-09-06 07:41:43 UTC Comment hidden (obsolete)
Comment 344 Martin Renvoize 2021-09-06 07:41:51 UTC Comment hidden (obsolete)
Comment 345 Martin Renvoize 2021-09-06 07:41:58 UTC Comment hidden (obsolete)
Comment 346 Martin Renvoize 2021-09-06 07:42:06 UTC Comment hidden (obsolete)
Comment 347 Martin Renvoize 2021-09-06 07:42:13 UTC Comment hidden (obsolete)
Comment 348 Martin Renvoize 2021-09-06 07:42:21 UTC Comment hidden (obsolete)
Comment 349 Martin Renvoize 2021-09-06 08:08:30 UTC
(In reply to Andrew Nugged from comment #325)
> Martin, your rebase applies on master silently,
> but conflicts (catalogue/detail.pl) when to apply over dependant (as in
> ticket  Bug 28484).
> 
> Ah, now I figured more: patch 124397 removes most of Bug 28484 changed code,
> 
> 
> ... so it looks like we just need to remove dependency reference from this
> BZ ticket?

There was a BZ dependency chain issue, your right.

All sorted now and rebased.
Comment 350 Martin Renvoize 2021-09-06 12:24:20 UTC
The search changes introduced in bug 20277 will also need to be applied here.
Comment 351 Andrii Nugged 2021-10-07 08:48:57 UTC
bug and hotfix: we testing this on pre-prod for our libraries (we already using this feature from this ticket for a year), I found that we already have MARC::Record object in $part variable so no need to have:

    $part = MARC::Record->new_from_xml( $part, 'UTF-8' );

so just commenting out this line makes it work. I did this half-blindly, Martin, you might re-check this proposal if this is the real fix.
Comment 352 Martin Renvoize 2021-10-12 08:25:12 UTC
Interesting.. this appears to be part of bug 19893.. what's interesting it that as far as I can tell it changes the return value for ES searches but doesn't do the same for Zebra.. so unless I'm mistaken there's a bigger bug here.
Comment 353 Andrii Nugged 2021-10-12 09:13:06 UTC
I did such patch for our "test production" code which now serves lively for one library and works for them (and I needed this urgently because 11175 and 20447 already heavily used by our customer libraries):

- $part = MARC::Record->new_from_xml( $part, 'UTF-8' );
+ unless (ref $part eq 'MARC::Record') {
+     $part = MARC::Record->new_from_xml( $part, 'UTF-8' );
+ }

this is mimicking how it was before in other place.

but that also might not be THAT solution, yet it passed over the "unless" for us because it already has MARC::Record in $part for us (because of ES as you said).
Comment 354 Martin Renvoize 2021-10-12 09:33:20 UTC Comment hidden (obsolete)
Comment 355 Martin Renvoize 2021-10-12 09:33:29 UTC Comment hidden (obsolete)
Comment 356 Martin Renvoize 2021-10-12 09:33:36 UTC Comment hidden (obsolete)
Comment 357 Martin Renvoize 2021-10-12 09:33:44 UTC Comment hidden (obsolete)
Comment 358 Martin Renvoize 2021-10-12 09:33:51 UTC Comment hidden (obsolete)
Comment 359 Martin Renvoize 2021-10-12 09:33:58 UTC Comment hidden (obsolete)
Comment 360 Martin Renvoize 2021-10-12 09:34:06 UTC Comment hidden (obsolete)
Comment 361 Martin Renvoize 2021-10-12 09:34:14 UTC Comment hidden (obsolete)
Comment 362 Martin Renvoize 2021-10-12 09:34:21 UTC Comment hidden (obsolete)
Comment 363 Martin Renvoize 2021-10-12 09:34:30 UTC Comment hidden (obsolete)
Comment 364 Martin Renvoize 2021-10-12 09:34:39 UTC Comment hidden (obsolete)
Comment 365 Martin Renvoize 2021-10-12 09:34:47 UTC Comment hidden (obsolete)
Comment 366 Martin Renvoize 2021-10-12 09:34:54 UTC Comment hidden (obsolete)
Comment 367 Martin Renvoize 2021-10-12 09:35:02 UTC Comment hidden (obsolete)
Comment 368 Martin Renvoize 2021-10-12 09:35:10 UTC Comment hidden (obsolete)
Comment 369 Martin Renvoize 2021-10-12 09:35:18 UTC Comment hidden (obsolete)
Comment 370 Martin Renvoize 2021-10-12 09:35:26 UTC Comment hidden (obsolete)
Comment 371 Martin Renvoize 2021-10-12 09:35:34 UTC Comment hidden (obsolete)
Comment 372 Martin Renvoize 2021-10-12 09:35:41 UTC Comment hidden (obsolete)
Comment 373 Martin Renvoize 2021-10-12 09:35:49 UTC Comment hidden (obsolete)
Comment 374 Martin Renvoize 2021-10-12 09:35:56 UTC Comment hidden (obsolete)
Comment 375 Martin Renvoize 2021-10-12 09:36:04 UTC Comment hidden (obsolete)
Comment 376 Martin Renvoize 2021-10-12 09:36:11 UTC Comment hidden (obsolete)
Comment 377 Martin Renvoize 2021-10-12 09:36:19 UTC Comment hidden (obsolete)
Comment 378 Martin Renvoize 2021-10-12 09:36:26 UTC
Created attachment 126081 [details] [review]
Bug 11175: (QA follow-up) Display components tab if no holdings

This patch updates the default details tab selection to components for
the case when there are no holdings attached to the record and there is
no HTML5 content to display in preference.
Comment 379 Martin Renvoize 2021-10-12 09:48:22 UTC
Created attachment 126084 [details] [review]
Bug 11175: (QA follow-up) OPAC - Display components tab if no holdings

As for the staff client, this selects the default tab to be 'Components'
for the case where no holdings are found.
Comment 380 Jonathan Druart 2021-10-18 13:46:10 UTC
Review, all patches squashed:

1. +use Encode qw(decode);
Not used in C4/XSLT.pm
One patch is saying that we need it, but the squashed version does not show that.

2. ES index added to the default mappings but to the upgrade process. Is that expected?

3.
+[% IF ( ComponentParts ) %]<li id="components_tab"><a href="#components">Components ([% ( ComponentParts.size ) %])</a></li>[% END %]

+                                <a href="#components">Components ([% ( ComponentParts.size || 1 ) %])</a>

=> Missing filter AND not consistent (the '|| 1')
Comment 381 Jonathan Druart 2021-10-18 13:47:29 UTC
4. Shouldn't the limit (300) be configurable?
Comment 382 Jonathan Druart 2021-10-18 13:55:20 UTC
Cannot get anything on the detail pages, the search does not return anything (tested with both ES and zebra, reset_all + import of the mrc file + full reindex)

2 debug statements in get_marc_components:
warn $searchstr;
warn $total_hits;

output:
[WARN] Host-item:(Easy piano pieces for children / compiled and edited by Lisa Cox, Sam Lung and Andrew Skirrow.) at /kohadevbox/koha/Koha/Biblio.pm line 497.
[WARN] 0 at /kohadevbox/koha/Koha/Biblio.pm line 502.
Comment 383 Jonathan Druart 2021-10-18 14:12:22 UTC
(In reply to Jonathan Druart from comment #382)
> Cannot get anything on the detail pages, the search does not return anything
> (tested with both ES and zebra, reset_all + import of the mrc file + full
> reindex)
> 
> 2 debug statements in get_marc_components:
> warn $searchstr;
> warn $total_hits;
> 
> output:
> [WARN] Host-item:(Easy piano pieces for children / compiled and edited by
> Lisa Cox, Sam Lung and Andrew Skirrow.) at /kohadevbox/koha/Koha/Biblio.pm
> line 497.
> [WARN] 0 at /kohadevbox/koha/Koha/Biblio.pm line 502.

Caused by UseControlNumber=Don't use.
It works when switched to "Use" (Thanks Martin!)
Comment 384 Martin Renvoize 2021-10-18 14:51:55 UTC
(In reply to Jonathan Druart from comment #383)
> Caused by UseControlNumber=Don't use.
> It works when switched to "Use" (Thanks Martin!)

This, I think, highlights some flaws in our Host-item index.. I would have expected the search to work.. but I think that can be saved for another bug.
Comment 385 Martin Renvoize 2021-10-18 15:16:00 UTC
Created attachment 126441 [details] [review]
Bug 11175: (QA follow-up) Be more specific with title fields

We were using the MARC::Record title accessor, but that just stringifies
all the subfields of field 245. The indexer, and the XSLT, specifically
use 245a so we need to do the same.
Comment 386 Martin Renvoize 2021-10-18 15:16:09 UTC
Created attachment 126442 [details] [review]
Bug 11175: (QA follow-up) Drop useless use

We no longer need Encode in he XSLT module
Comment 387 Martin Renvoize 2021-10-18 15:34:00 UTC
Created attachment 126443 [details] [review]
Bug 11175: (QA follow-up) Restore link to more results

This patch restores the link to 'more results' should you exceed the
hard coded limit of 300 component parts.
Comment 388 Marcel de Rooy 2021-10-19 13:52:08 UTC
(In reply to Olli-Antti Kivilahti from comment #8)
> Thanks for the QA everyone, thanks for the uncovered bugs!
> I'll try to get back at this sometime next year :)

Hahaha
Comment 389 Marcel de Rooy 2021-10-22 06:12:33 UTC
QAing
Comment 390 Marcel de Rooy 2021-10-22 06:42:38 UTC
 14 files changed, 322 insertions(+), 55 deletions(-)

$showcomp variable naming; show_components better

Changes XSLTParse4Display; no tests. (Yes, there is no, but there should be.)

Code block if ($xslsyspref =~ m/Details/) {
We are fetching components while we may not use them.
Have the impression that the if inside the block should be incorporated into the main condition.

installer/data/mysql/atomicupdate/bug_11175.perl
Needs to be converted to new style

+('ShowComponentRecords', 'nowhere', 'nowhere|staff|opac|both','In which record detail pages to show list of the component records, as linked via 773','Choice'),
Should nowhere just be empty?
Comment 391 Marcel de Rooy 2021-10-22 06:43:28 UTC
            [% IF count == 0 %]
            [% IF ( Koha.Preference('HTML5MediaEnabled') == 'staff' or Koha.Preference('HTML5MediaEnabled') == 'both' ) && HTML5MediaSets.size %]
            $('#bibliodetails').tabs("option", "active", $('#media_tab').index() );
            [% ELSIF ComponentParts %]
            $('#bibliodetails').tabs("option", "active", $('#components_tab').index() );
            [% END %]
            [% END %]

What about indentation?
Comment 392 Marcel de Rooy 2021-10-22 06:46:29 UTC
koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slimUtils.xsl
koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss

Seems you only remove 2 newlines, please remove these changes.

+                            [% IF defaulttab == 'components' %]
+                            <li id="tab_components" class="ui-tabs-active">
+                            [% ELSE %]
+                            <li id="tab_components">
+                            [% END %]
+                                <a href="#components">Components ([% ( ComponentParts.size ) %])</a>
+                            </li>

Indentation.

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl b/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
index 105da508c7..2df0e1b3b4 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
@@ -541,7 +541,6 @@
             <xsl:value-of select="$partName" />
         </xsl:if>
     </xsl:template>
-
 </xsl:stylesheet>

 <!-- Stylus Studio meta-information - (c)1998-2002 eXcelon Corp.

Dont do this please
Comment 393 Marcel de Rooy 2021-10-22 06:53:32 UTC
my $components = $biblio->get_marc_components(300)

This 300 is hardcoded. At least a constant? Or some other configuration option?
Probably the latter.
Comment 394 Marcel de Rooy 2021-10-22 06:55:20 UTC
Not from this report, but very related:

        $self->{_components} = $results if ( defined($results) && scalar(@$results) );
    }

    return $self->{_components} || [];

Why do we save these component parts in the object and not use them later (if we would call again)?
Comment 395 Marcel de Rooy 2021-10-22 06:56:58 UTC
Looks quite good to me, but still needs attention for minor details. Perfection is in the details?
Comment 396 Jonathan Druart 2021-10-22 07:47:06 UTC
Created attachment 126687 [details] [review]
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit
Comment 397 Jonathan Druart 2021-10-22 08:11:57 UTC
Created attachment 126688 [details] [review]
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit
Comment 398 Jonathan Druart 2021-10-22 08:14:42 UTC
Created attachment 126689 [details] [review]
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit
Comment 399 Jonathan Druart 2021-10-22 08:17:55 UTC
Created attachment 126690 [details] [review]
Bug 11175: Uniformize ComponentParts in template

More robust as it takes empty array into account
Comment 400 Jonathan Druart 2021-10-22 09:04:09 UTC
Created attachment 126692 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

JD amended path
-    if ($xslsyspref =~ m/Details/) {
+    if ( $xslsyspref eq "OPACXSLTDetailsDisplay" || $xslsyspref eq "XSLTDetailsDisplay" ) {
Comment 401 Jonathan Druart 2021-10-22 09:04:22 UTC
Created attachment 126693 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 402 Jonathan Druart 2021-10-22 09:04:31 UTC
Created attachment 126694 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 403 Jonathan Druart 2021-10-22 09:04:40 UTC
Created attachment 126695 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 404 Jonathan Druart 2021-10-22 09:04:51 UTC
Created attachment 126696 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 405 Jonathan Druart 2021-10-22 09:05:01 UTC
Created attachment 126697 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 406 Jonathan Druart 2021-10-22 09:05:12 UTC
Created attachment 126698 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc

This fixes the QA tool failure.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 407 Jonathan Druart 2021-10-22 09:05:23 UTC
Created attachment 126699 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 408 Jonathan Druart 2021-10-22 09:05:34 UTC
Created attachment 126700 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 409 Jonathan Druart 2021-10-22 09:05:45 UTC
Created attachment 126701 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 410 Jonathan Druart 2021-10-22 09:05:55 UTC
Created attachment 126702 [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.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 411 Jonathan Druart 2021-10-22 09:06:05 UTC
Created attachment 126703 [details] [review]
Bug 11175: (follow-up) Rename routine get_marc_components

For consistency with other marc data accessor methods rename the new
'components' method to 'get_marc_components'.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 412 Jonathan Druart 2021-10-22 09:06:15 UTC
Created attachment 126704 [details] [review]
Bug 11175: (QA follow-up) Move preference to 'Display'

The 'ShowComponentRecords' preference was displaying under the 'Export'
section in the cataloging system preferences tab. As a display related
feature, I felt it was more appropriate to put it into the 'Display'
section.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 413 Jonathan Druart 2021-10-22 09:06:26 UTC
Created attachment 126705 [details] [review]
Bug 11175: (QA follow-up) Take account of bug 15851

We can simplify the code introduced by bug 15851 by moving the
'show_analytics_link' variable assignment into C4::XSLT and thus making
the code more DRY.

Taking the code in bug 15851 as inspiration this patch also adds proper
handling for UseControlNumber vs EasyAnalytics style 773 linking and
ensures we only return analytic component parts and no other records
containing 773's.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 414 Jonathan Druart 2021-10-22 09:06:36 UTC
Created attachment 126706 [details] [review]
Bug 11175: (QA follow-up) Move get_component_part_query

This patch removes Koha::Util::Search in preference to embedding the
search query builder in Koha::Biblio as get_analytics_query.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Bug 11175: (QA follow-up) Rename back to get_marc_components

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 415 Jonathan Druart 2021-10-22 09:06:45 UTC
Created attachment 126707 [details] [review]
Bug 11175: (QA follow-up) Only get biblio once

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 416 Jonathan Druart 2021-10-22 09:06:55 UTC
Created attachment 126708 [details] [review]
Bug 11175: (QA follow-up) Fix queries

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 417 Jonathan Druart 2021-10-22 09:07:05 UTC
Created attachment 126709 [details] [review]
Bug 11175: Display components in own tab

This patch updates the display so that rather than displaying the
components using the main XSLT and then using CSS to move them into a
box on the right side for display, which causes issues with RTL records;
We instead add them in a new tab beneath the rest of the record details
alongside holdings and other details.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Bug 11175: (QA follow-up) Fixes for bug 12561

Bug 12561 changed the prototype for XSLTParse4Display so this patcha
accounts for the additional calls to that method introduced in this
patchset.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 418 Jonathan Druart 2021-10-22 09:07:15 UTC
Created attachment 126710 [details] [review]
Bug 11175: (follow-up) Use clean_search_term

This patch uses the now public clean_search_term from bug 28316 to
sanitize the title string and prevent crashes in elastic search when
title contain reserved characters.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 419 Jonathan Druart 2021-10-22 09:07:26 UTC
Created attachment 126711 [details] [review]
Bug 11175: (QA follow-up) Account for difference in simple_search return

Thanks to a comment from Andrew, I discovered that the return from
simple_search_compat actually differs depending on if you are using
ElasticSearch or Zebra. The 'results' arrayref contains MARC::Record
objects for ES and raw marc strings if you are using Zebra.  This is
actually already dealt with in the misnamed new_record_from_zebra
function inside C4::Search.. so this patch simply replaces our
MARC::Record instantiation code with a call to that function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>
Comment 420 Jonathan Druart 2021-10-22 09:07:36 UTC
Created attachment 126712 [details] [review]
Bug 11175: (QA follow-up) Display components tab if no holdings

This patch updates the default details tab selection to components for
the case when there are no holdings attached to the record and there is
no HTML5 content to display in preference.

Bug 11175: (QA follow-up) OPAC - Display components tab if no holdings

As for the staff client, this selects the default tab to be 'Components'
for the case where no holdings are found.
Comment 421 Jonathan Druart 2021-10-22 09:07:46 UTC
Created attachment 126713 [details] [review]
Bug 11175: (QA follow-up) Be more specific with title fields

We were using the MARC::Record title accessor, but that just stringifies
all the subfields of field 245. The indexer, and the XSLT, specifically
use 245a so we need to do the same.
Comment 422 Jonathan Druart 2021-10-22 09:07:55 UTC
Created attachment 126714 [details] [review]
Bug 11175: (QA follow-up) Restore link to more results

This patch restores the link to 'more results' should you exceed the
hard coded limit of 300 component parts.
Comment 423 Jonathan Druart 2021-10-22 09:08:06 UTC
Created attachment 126715 [details] [review]
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit
Comment 424 Jonathan Druart 2021-10-22 09:08:16 UTC
Created attachment 126716 [details] [review]
Bug 11175: Uniformize ComponentParts in template

More robust as it takes empty array into account
Comment 425 Jonathan Druart 2021-10-22 09:08:56 UTC
Some patches have been squashed for readability.

(In reply to Marcel de Rooy from comment #390)
>  14 files changed, 322 insertions(+), 55 deletions(-)
> 
> $showcomp variable naming; show_components better

Too late, too complicated or we add a separate commit for that. Not a big deal IMO.

> Changes XSLTParse4Display; no tests. (Yes, there is no, but there should be.)

Still missing, I will try and provide some a bit later today.

> Code block if ($xslsyspref =~ m/Details/) {
> We are fetching components while we may not use them.
> Have the impression that the if inside the block should be incorporated into
> the main condition.

Done and squashed with the first patch.

> installer/data/mysql/atomicupdate/bug_11175.perl
> Needs to be converted to new style

I will do it when pushing it.

> +('ShowComponentRecords', 'nowhere', 'nowhere|staff|opac|both','In which
> record detail pages to show list of the component records, as linked via
> 773','Choice'),
> Should nowhere just be empty?

empty or 'nowhere' is fine.

(In reply to Marcel de Rooy from comment #391)
> What about indentation?

Done and squashed.

(In reply to Marcel de Rooy from comment #392)
> koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slimUtils.xsl
> koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss
> 
> Seems you only remove 2 newlines, please remove these changes.

Done, squashed.

> +                            [% IF defaulttab == 'components' %]
> +                            <li id="tab_components" class="ui-tabs-active">
> +                            [% ELSE %]
> +                            <li id="tab_components">
> +                            [% END %]
> +                                <a href="#components">Components ([% (
> ComponentParts.size ) %])</a>
> +                            </li>
> 
> Indentation.

Done, squashed.

> diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
> b/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
> index 105da508c7..2df0e1b3b4 100644
> --- a/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
> +++ b/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
> @@ -541,7 +541,6 @@
>              <xsl:value-of select="$partName" />
>          </xsl:if>
>      </xsl:template>
> -
>  </xsl:stylesheet>
> 
>  <!-- Stylus Studio meta-information - (c)1998-2002 eXcelon Corp.
> 
> Dont do this please

Done, squashed.

(In reply to Marcel de Rooy from comment #393)
> my $components = $biblio->get_marc_components(300)
> 
> This 300 is hardcoded. At least a constant? Or some other configuration
> option?
> Probably the latter.

Done, see follow-up.

(In reply to Marcel de Rooy from comment #394)
> Not from this report, but very related:
> 
>         $self->{_components} = $results if ( defined($results) &&
> scalar(@$results) );
>     }
> 
>     return $self->{_components} || [];
> 
> Why do we save these component parts in the object and not use them later
> (if we would call again)?

Yes, that's a bad pattern. We discussed that recently but cannot remember where.
We shouldn't cache that in the object IMO.
Comment 426 Martin Renvoize 2021-10-22 09:30:34 UTC
(In reply to Jonathan Druart from comment #425)
> Some patches have been squashed for readability.
> 
> (In reply to Marcel de Rooy from comment #390)
> >  14 files changed, 322 insertions(+), 55 deletions(-)
> > 
> > $showcomp variable naming; show_components better
> 
> Too late, too complicated or we add a separate commit for that. Not a big
> deal IMO.

I was trying to be light-touch when I first adopted this bug.. but happy to change this too.

> 
> > Changes XSLTParse4Display; no tests. (Yes, there is no, but there should be.)
> 
> Still missing, I will try and provide some a bit later today.

Damn, I meant to add these tests.

> 
> > Code block if ($xslsyspref =~ m/Details/) {
> > We are fetching components while we may not use them.
> > Have the impression that the if inside the block should be incorporated into
> > the main condition.

We use them for the 'Show analytics' link handling stuff.. I was also not a fan of always firing this query.. but that predates this patchset.

> (In reply to Marcel de Rooy from comment #394)
> > Not from this report, but very related:
> > 
> >         $self->{_components} = $results if ( defined($results) &&
> > scalar(@$results) );
> >     }
> > 
> >     return $self->{_components} || [];
> > 
> > Why do we save these component parts in the object and not use them later
> > (if we would call again)?
> 
> Yes, that's a bad pattern. We discussed that recently but cannot remember
> where.
> We shouldn't cache that in the object IMO.

The caching here appears to be completely broken to me.. likely by one of the many many rebases/follow-ups. The idea was that we execute this search from both C4::XSLT (for the 'Show analytics' link handling) AND each of the details controllers.. caching at the object level would mean we could re-use the results from the controller level at the XSLT call.  I'm not sure how else you could cache this if not at the object level.
Comment 427 Martin Renvoize 2021-10-22 09:33:29 UTC
Yup.. blames shows me that 'Bug 11175: (follow-up) Return empty array if no components' drops the `if (!defined($self->{_components})` which is actually what does the caching.
Comment 428 Martin Renvoize 2021-10-22 09:37:34 UTC
But.. as we don't actually pass the Biblio object around.. it's kinda pointless anyway.. feel free to just drop the whole _components internal stuff.. at this point it serves no purpose.

Good catch.. I've been too close to this and too along to spot everything all these months.
Comment 429 Marcel de Rooy 2021-10-22 10:04:44 UTC
Created attachment 126717 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

JD amended path
-    if ($xslsyspref =~ m/Details/) {
+    if ( $xslsyspref eq "OPACXSLTDetailsDisplay" || $xslsyspref eq "XSLTDetailsDisplay" ) {

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 430 Marcel de Rooy 2021-10-22 10:04:52 UTC
Created attachment 126718 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 431 Marcel de Rooy 2021-10-22 10:05:00 UTC
Created attachment 126719 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 432 Marcel de Rooy 2021-10-22 10:05:09 UTC
Created attachment 126720 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 433 Marcel de Rooy 2021-10-22 10:05:18 UTC
Created attachment 126721 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 434 Marcel de Rooy 2021-10-22 10:05:26 UTC
Created attachment 126722 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 435 Marcel de Rooy 2021-10-22 10:05:34 UTC
Created attachment 126723 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc

This fixes the QA tool failure.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 436 Marcel de Rooy 2021-10-22 10:05:42 UTC
Created attachment 126724 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 437 Marcel de Rooy 2021-10-22 10:05:49 UTC
Created attachment 126725 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 438 Marcel de Rooy 2021-10-22 10:05:57 UTC
Created attachment 126726 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 439 Marcel de Rooy 2021-10-22 10:06:05 UTC
Created attachment 126727 [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.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 440 Marcel de Rooy 2021-10-22 10:06:13 UTC
Created attachment 126728 [details] [review]
Bug 11175: (follow-up) Rename routine get_marc_components

For consistency with other marc data accessor methods rename the new
'components' method to 'get_marc_components'.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 441 Marcel de Rooy 2021-10-22 10:06:22 UTC
Created attachment 126729 [details] [review]
Bug 11175: (QA follow-up) Move preference to 'Display'

The 'ShowComponentRecords' preference was displaying under the 'Export'
section in the cataloging system preferences tab. As a display related
feature, I felt it was more appropriate to put it into the 'Display'
section.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 442 Marcel de Rooy 2021-10-22 10:06:30 UTC
Created attachment 126730 [details] [review]
Bug 11175: (QA follow-up) Take account of bug 15851

We can simplify the code introduced by bug 15851 by moving the
'show_analytics_link' variable assignment into C4::XSLT and thus making
the code more DRY.

Taking the code in bug 15851 as inspiration this patch also adds proper
handling for UseControlNumber vs EasyAnalytics style 773 linking and
ensures we only return analytic component parts and no other records
containing 773's.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 443 Marcel de Rooy 2021-10-22 10:06:38 UTC
Created attachment 126731 [details] [review]
Bug 11175: (QA follow-up) Move get_component_part_query

This patch removes Koha::Util::Search in preference to embedding the
search query builder in Koha::Biblio as get_analytics_query.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Bug 11175: (QA follow-up) Rename back to get_marc_components

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 444 Marcel de Rooy 2021-10-22 10:06:46 UTC
Created attachment 126732 [details] [review]
Bug 11175: (QA follow-up) Only get biblio once

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 445 Marcel de Rooy 2021-10-22 10:06:54 UTC
Created attachment 126733 [details] [review]
Bug 11175: (QA follow-up) Fix queries

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 446 Marcel de Rooy 2021-10-22 10:07:02 UTC
Created attachment 126734 [details] [review]
Bug 11175: Display components in own tab

This patch updates the display so that rather than displaying the
components using the main XSLT and then using CSS to move them into a
box on the right side for display, which causes issues with RTL records;
We instead add them in a new tab beneath the rest of the record details
alongside holdings and other details.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Bug 11175: (QA follow-up) Fixes for bug 12561

Bug 12561 changed the prototype for XSLTParse4Display so this patcha
accounts for the additional calls to that method introduced in this
patchset.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 447 Marcel de Rooy 2021-10-22 10:07:10 UTC
Created attachment 126735 [details] [review]
Bug 11175: (follow-up) Use clean_search_term

This patch uses the now public clean_search_term from bug 28316 to
sanitize the title string and prevent crashes in elastic search when
title contain reserved characters.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 448 Marcel de Rooy 2021-10-22 10:07:19 UTC
Created attachment 126736 [details] [review]
Bug 11175: (QA follow-up) Account for difference in simple_search return

Thanks to a comment from Andrew, I discovered that the return from
simple_search_compat actually differs depending on if you are using
ElasticSearch or Zebra. The 'results' arrayref contains MARC::Record
objects for ES and raw marc strings if you are using Zebra.  This is
actually already dealt with in the misnamed new_record_from_zebra
function inside C4::Search.. so this patch simply replaces our
MARC::Record instantiation code with a call to that function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 449 Marcel de Rooy 2021-10-22 10:07:27 UTC
Created attachment 126737 [details] [review]
Bug 11175: (QA follow-up) Display components tab if no holdings

This patch updates the default details tab selection to components for
the case when there are no holdings attached to the record and there is
no HTML5 content to display in preference.

Bug 11175: (QA follow-up) OPAC - Display components tab if no holdings

As for the staff client, this selects the default tab to be 'Components'
for the case where no holdings are found.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 450 Marcel de Rooy 2021-10-22 10:07:34 UTC
Created attachment 126738 [details] [review]
Bug 11175: (QA follow-up) Be more specific with title fields

We were using the MARC::Record title accessor, but that just stringifies
all the subfields of field 245. The indexer, and the XSLT, specifically
use 245a so we need to do the same.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 451 Marcel de Rooy 2021-10-22 10:07:42 UTC
Created attachment 126739 [details] [review]
Bug 11175: (QA follow-up) Restore link to more results

This patch restores the link to 'more results' should you exceed the
hard coded limit of 300 component parts.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 452 Marcel de Rooy 2021-10-22 10:07:50 UTC
Created attachment 126740 [details] [review]
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 453 Marcel de Rooy 2021-10-22 10:07:58 UTC
Created attachment 126741 [details] [review]
Bug 11175: Uniformize ComponentParts in template

More robust as it takes empty array into account

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 454 Marcel de Rooy 2021-10-22 10:08:08 UTC
Created attachment 126742 [details] [review]
Bug 11175: (QA follow-up) Simplify block in XSLT

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 455 Marcel de Rooy 2021-10-22 10:08:16 UTC
Created attachment 126743 [details] [review]
Bug 11175: (QA follow-up) Remove caching _components

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 456 Jonathan Druart 2021-10-22 10:32:59 UTC
Comment on attachment 126742 [details] [review]
Bug 11175: (QA follow-up) Simplify block in XSLT

Tests are not passing with this patch, keeping my version.
Comment 457 Jonathan Druart 2021-10-22 10:35:21 UTC
Created attachment 126744 [details] [review]
Bug 11175: Add tests and correct show_analytics logic
Comment 458 Jonathan Druart 2021-10-22 10:44:21 UTC
Created attachment 126745 [details] [review]
Bug 11175: Add tests and correct show_analytics logic
Comment 459 Marcel de Rooy 2021-10-22 11:03:57 UTC
(In reply to Jonathan Druart from comment #458)
> Created attachment 126745 [details] [review] [review]
> Bug 11175: Add tests and correct show_analytics logic

Somehow it still feels a bit strange to do this in the XSLT module.
Comment 460 Marcel de Rooy 2021-10-22 11:05:19 UTC
And what about showing a link without fetching all components? In terms of performance?
Comment 461 Martin Renvoize 2021-10-22 13:16:09 UTC
(In reply to Marcel de Rooy from comment #459)
> (In reply to Jonathan Druart from comment #458)
> > Created attachment 126745 [details] [review] [review] [review]
> > Bug 11175: Add tests and correct show_analytics logic
> 
> Somehow it still feels a bit strange to do this in the XSLT module.

It was moved to the XSLT module for two reasons..

1. DRY the code out.. it was sat in the controllers before and was slightly different in each.. so had bugs.
2. We were already passing a variable through to XSLT so we could use it in the stylesheets to work out whether to show the link or not.. thus it made sense to deal with it in the XSLT parser rather than passing it around at multiple levels.
Comment 462 Martin Renvoize 2021-10-22 13:18:36 UTC
(In reply to Marcel de Rooy from comment #460)
> And what about showing a link without fetching all components? In terms of
> performance?

You mean undoing bug 15851 effectively reverting it and dropping the requested bugfix/feature.?
Comment 463 Katrin Fischer 2021-10-22 13:28:40 UTC
(In reply to Martin Renvoize from comment #462)
> (In reply to Marcel de Rooy from comment #460)
> > And what about showing a link without fetching all components? In terms of
> > performance?
> 
> You mean undoing bug 15851 effectively reverting it and dropping the
> requested bugfix/feature.?

This has an on/off switch now, so I think if you are worried about performance, you can not use the feature for now and still implement something later on if needed.
Comment 464 Martin Renvoize 2021-10-22 13:31:20 UTC
(In reply to Katrin Fischer from comment #463)
> This has an on/off switch now, so I think if you are worried about
> performance, you can not use the feature for now and still implement
> something later on if needed.

I think you're misunderstanding.. regardless of the preference the search will take place.. and has been taking place since bug 15851... I just make sure we don't execute it twice.
Comment 465 Martin Renvoize 2021-10-22 13:33:28 UTC
Note: Bug 26314 will do the same for volumes.. the only way we can selectively show/now show the 'Show volumes' link is to pre-execute the search to see if it will get results ;)
Comment 466 Martin Renvoize 2021-10-22 13:35:56 UTC
The performance you DO save by disabling this one is an XSLT parse of each of the resultant linked records.
Comment 467 Katrin Fischer 2021-10-22 14:15:40 UTC
(In reply to Martin Renvoize from comment #466)
> The performance you DO save by disabling this one is an XSLT parse of each
> of the resultant linked records.

Thx for clarifying!
Comment 468 Marcel de Rooy 2021-10-22 14:26:39 UTC
Well, just asking. Dont worry.

Not saying that we should revert things, people. But the fact that something has been pushed does certainly not always mean that it cannot be done better, different, etc. Step back and look again.
Comment 469 Marcel de Rooy 2021-10-22 14:37:47 UTC
(In reply to Martin Renvoize from comment #464)
> I think you're misunderstanding.. regardless of the preference the search
> will take place.. and has been taking place since bug 15851... I just make
> sure we don't execute it twice.

I am not sure if this is correct btw. Where does that happen?
Comment 470 Marcel de Rooy 2021-10-22 14:41:41 UTC Comment hidden (obsolete)
Comment 471 Marcel de Rooy 2021-10-23 12:37:32 UTC
Still thinking about this a bit, and looking here:

        for my $part ( @{$components} ) {
            $part = C4::Search::new_record_from_zebra( 'biblioserver', $part );
            push @{$parts},
              XSLTParse4Display(
                {
                    biblionumber => $biblionumber,

Why pass the same biblionumber here when running through all component records?
Comment 472 Marcel de Rooy 2021-10-24 11:52:36 UTC
(In reply to Marcel de Rooy from comment #471)

> Why pass the same biblionumber here when running through all component
> records?

Beginning to understand some of what happened here ;) Finally :)
We still need a bit of fixing things that may have been broken during the process.

Changing status. Will be submitting a follow-up. Dont touch please (today or tomorrow).
Comment 473 Marcel de Rooy 2021-10-24 15:22:45 UTC
I wrote two follow-ups now. WIll attach them tomorrow.

But did look a bit closer to analytical records in Koha and they are quite different from what is done here. They use 773 but thats about it. So there seems to be no reason to braid(?) them together here. Making the analytics disappear when there are components.

Note: the components here work with 773 and record control numbers looked up in biblio records. Analytics works different, storing item numbers(!) in 773$9. This seems to be something that we add on top of MARC21, if I understand correctly (so no default MARC21).

Conclusion: We should keep them really separate from each other. Will have to add that to my follow-up still. Which should be very trivial.

Katrin: Could you confirm?
Comment 474 Marcel de Rooy 2021-10-25 09:46:37 UTC
Created attachment 126816 [details] [review]
Bug 11175: (QA follow-up) Remove caching _components

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 475 Marcel de Rooy 2021-10-25 09:46:46 UTC
Created attachment 126817 [details] [review]
Bug 11175: (QA follow-up) Do not fetch twice

We were fetching components in the first call of XSLTParse4Display
(opac-detail around L220). And again (opac-detail around L660).
Same for catalogue/detail.pl.
Moving the XSLT block in both scripts and removing the code from
XSLT.pm which again makes the tests obsolete.

Not hiding the link when there are components. Might be helpful too.

Also fixing biblionumber in the XSLTParse4Display call for parts.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 476 Marcel de Rooy 2021-10-25 09:53:20 UTC
Tested this and feeling more comfortable now.
The value of the MaxComponents syspref is 300 now. Which feels a bit high. But no problem.
The EasyAnalytics stuff in Koha made this development a bit harder to understand. That area needs probably a bit more attention.

Switching back to SO.
If Martin agrees, we can move it back to PQA imho.
Comment 477 Marcel de Rooy 2021-10-25 10:00:40 UTC
+                            [% IF ComponentParts.size == Koha.Preference('MaxComponentRecords')%]

Might be better btw to check with >.
Comment 478 Katrin Fischer 2021-10-25 19:36:21 UTC
(In reply to Marcel de Rooy from comment #473)
> I wrote two follow-ups now. WIll attach them tomorrow.
> 
> But did look a bit closer to analytical records in Koha and they are quite
> different from what is done here. They use 773 but thats about it. So there
> seems to be no reason to braid(?) them together here. Making the analytics
> disappear when there are components.
> 
> Note: the components here work with 773 and record control numbers looked up
> in biblio records. Analytics works different, storing item numbers(!) in
> 773$9. This seems to be something that we add on top of MARC21, if I
> understand correctly (so no default MARC21).
> 
> Conclusion: We should keep them really separate from each other. Will have
> to add that to my follow-up still. Which should be very trivial.
> 
> Katrin: Could you confirm?

I believe there are some major differences between the features - so fully agree.

What we are looking for here is linking between bibliographic records. We link a record to another record, using either $w or a combination of title and author from 773 for search. So this is about hierarchies on the bibliographic record level.

EasyAnalytics as I understand it, is linking an item to multiple records using non-standard MARC subfields of 773. It's a useful feature, but a different use case. For example it could probably be used very well for "bound-togethers", where different works where combined into one bound unit (often found in historical holdings).

We could probably make both work together by checking for the prefs and maybe existence of certain subfields in 773, but as UseControlnumber and Easyanalytics are already mutually exclusive right now, I think it's outside of scope here.
Comment 479 Marcel de Rooy 2021-10-26 06:53:39 UTC
(In reply to Katrin Fischer from comment #478)
> I believe there are some major differences between the features - so fully
> agree.
> 
> What we are looking for here is linking between bibliographic records. We
> link a record to another record, using either $w or a combination of title
> and author from 773 for search. So this is about hierarchies on the
> bibliographic record level.
> 
> EasyAnalytics as I understand it, is linking an item to multiple records
> using non-standard MARC subfields of 773. It's a useful feature, but a
> different use case. For example it could probably be used very well for
> "bound-togethers", where different works where combined into one bound unit
> (often found in historical holdings).
> 
> We could probably make both work together by checking for the prefs and
> maybe existence of certain subfields in 773, but as UseControlnumber and
> Easyanalytics are already mutually exclusive right now, I think it's outside
> of scope here.

Thx for replying. We are on the same track.
Comment 480 Martin Renvoize 2021-10-26 07:13:32 UTC
Fair enough, if you feel we should move the check back to the two controllers I'm OK with that.. I've DRY'd the code in other regards now so that bit of refactoring can get undone.  You're incorrect in saying that the search query would have ever been executed twice though.. we do not use the 'Details' templates for the display of the component parts when inlined, so the block with the query within it would not have been executed.

Regarding having the 'Show analytics' link regardless of the preference.. that's fine too; Though personally, I feel it perhaps confuses the end-user with two links to display the same information.. one without a re-fetch and one with.

I feel this bug has gone on long enough and any further refinements should be in their own bugs.. it worries me that so much is still going on at the end of the development cycle of the bug personally.

Don't you also need to remove some XSLT tests if you're moving the code back out into the Controllers?
Comment 481 Martin Renvoize 2021-10-26 07:17:08 UTC
Now things have changed again I'm not confident in rebasing the set again.. no longer applies.
Comment 482 Martin Renvoize 2021-10-26 07:30:09 UTC
Needs rebasing for bug 29284 conflicts.. this will probably be easier with a significant squash and loss of history.
Comment 483 Marcel de Rooy 2021-10-26 07:37:08 UTC
(In reply to Martin Renvoize from comment #480)
> Fair enough, if you feel we should move the check back to the two
> controllers I'm OK with that.. I've DRY'd the code in other regards now so
> that bit of refactoring can get undone.  You're incorrect in saying that the
> search query would have ever been executed twice though.. we do not use the
> 'Details' templates for the display of the component parts when inlined, so
> the block with the query within it would not have been executed.

Never say never:
[2021/10/26 07:31:06] [WARN] components: C4::XSLT at /usr/share/koha/Koha/Biblio.pm line 503.
[2021/10/26 07:31:06] [WARN] components: CGI::Compile::ROOT::usr_share_koha_opac_opac_2ddetail_2epl at /usr/share/koha/Koha/Biblio.pm line 503.

Without the last follow-up. One view of opac-detail. First warn comes from XSLT as expected. Second call from opac-detail line 664 (really, as expected too). Just read the code. Ha, who really DRYed here? :)

> Regarding having the 'Show analytics' link regardless of the preference..
> that's fine too; Though personally, I feel it perhaps confuses the end-user
> with two links to display the same information.. one without a re-fetch and
> one with.

No time left for a debate here ;) If this would really be so confusing, we can remove it later or just css it away. Dont think so.

> I feel this bug has gone on long enough and any further refinements should
> be in their own bugs.. it worries me that so much is still going on at the
> end of the development cycle of the bug personally.
> 
> Don't you also need to remove some XSLT tests if you're moving the code back
> out into the Controllers?

Was already done.
Comment 484 Marcel de Rooy 2021-10-26 07:37:31 UTC
(In reply to Martin Renvoize from comment #482)
> Needs rebasing for bug 29284 conflicts.. this will probably be easier with a
> significant squash and loss of history.

I'll do it.
Comment 485 Marcel de Rooy 2021-10-26 07:48:48 UTC
Created attachment 126869 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

JD amended path
-    if ($xslsyspref =~ m/Details/) {
+    if ( $xslsyspref eq "OPACXSLTDetailsDisplay" || $xslsyspref eq "XSLTDetailsDisplay" ) {

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 486 Marcel de Rooy 2021-10-26 07:48:56 UTC
Created attachment 126870 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 487 Marcel de Rooy 2021-10-26 07:49:04 UTC
Created attachment 126871 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 488 Marcel de Rooy 2021-10-26 07:49:12 UTC
Created attachment 126872 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 489 Marcel de Rooy 2021-10-26 07:49:19 UTC
Created attachment 126873 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 490 Marcel de Rooy 2021-10-26 07:49:27 UTC
Created attachment 126874 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 491 Marcel de Rooy 2021-10-26 07:49:34 UTC
Created attachment 126875 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Bug 11175: (QA follow-up) Replace en dash with hyphen in perldoc

This fixes the QA tool failure.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 492 Marcel de Rooy 2021-10-26 07:49:42 UTC
Created attachment 126876 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 493 Marcel de Rooy 2021-10-26 07:49:50 UTC
Created attachment 126877 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 494 Marcel de Rooy 2021-10-26 07:49:57 UTC
Created attachment 126878 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 495 Marcel de Rooy 2021-10-26 07:50:06 UTC
Created attachment 126879 [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.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 496 Marcel de Rooy 2021-10-26 07:50:14 UTC
Created attachment 126880 [details] [review]
Bug 11175: (follow-up) Rename routine get_marc_components

For consistency with other marc data accessor methods rename the new
'components' method to 'get_marc_components'.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 497 Marcel de Rooy 2021-10-26 07:50:21 UTC
Created attachment 126881 [details] [review]
Bug 11175: (QA follow-up) Move preference to 'Display'

The 'ShowComponentRecords' preference was displaying under the 'Export'
section in the cataloging system preferences tab. As a display related
feature, I felt it was more appropriate to put it into the 'Display'
section.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 498 Marcel de Rooy 2021-10-26 07:50:29 UTC
Created attachment 126882 [details] [review]
Bug 11175: (QA follow-up) Take account of bug 15851

We can simplify the code introduced by bug 15851 by moving the
'show_analytics_link' variable assignment into C4::XSLT and thus making
the code more DRY.

Taking the code in bug 15851 as inspiration this patch also adds proper
handling for UseControlNumber vs EasyAnalytics style 773 linking and
ensures we only return analytic component parts and no other records
containing 773's.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 499 Marcel de Rooy 2021-10-26 07:50:39 UTC
Created attachment 126883 [details] [review]
Bug 11175: (QA follow-up) Move get_component_part_query

This patch removes Koha::Util::Search in preference to embedding the
search query builder in Koha::Biblio as get_analytics_query.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Bug 11175: (QA follow-up) Rename back to get_marc_components

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 500 Marcel de Rooy 2021-10-26 07:50:47 UTC
Created attachment 126884 [details] [review]
Bug 11175: (QA follow-up) Only get biblio once

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 501 Marcel de Rooy 2021-10-26 07:50:55 UTC
Created attachment 126885 [details] [review]
Bug 11175: (QA follow-up) Fix queries

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 502 Marcel de Rooy 2021-10-26 07:51:03 UTC
Created attachment 126886 [details] [review]
Bug 11175: Display components in own tab

This patch updates the display so that rather than displaying the
components using the main XSLT and then using CSS to move them into a
box on the right side for display, which causes issues with RTL records;
We instead add them in a new tab beneath the rest of the record details
alongside holdings and other details.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Bug 11175: (QA follow-up) Fixes for bug 12561

Bug 12561 changed the prototype for XSLTParse4Display so this patcha
accounts for the additional calls to that method introduced in this
patchset.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 503 Marcel de Rooy 2021-10-26 07:51:11 UTC
Created attachment 126887 [details] [review]
Bug 11175: (follow-up) Use clean_search_term

This patch uses the now public clean_search_term from bug 28316 to
sanitize the title string and prevent crashes in elastic search when
title contain reserved characters.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 504 Marcel de Rooy 2021-10-26 07:51:22 UTC
Created attachment 126888 [details] [review]
Bug 11175: (QA follow-up) Account for difference in simple_search return

Thanks to a comment from Andrew, I discovered that the return from
simple_search_compat actually differs depending on if you are using
ElasticSearch or Zebra. The 'results' arrayref contains MARC::Record
objects for ES and raw marc strings if you are using Zebra.  This is
actually already dealt with in the misnamed new_record_from_zebra
function inside C4::Search.. so this patch simply replaces our
MARC::Record instantiation code with a call to that function.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Nugged <nugged@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 505 Marcel de Rooy 2021-10-26 07:51:30 UTC
Created attachment 126889 [details] [review]
Bug 11175: (QA follow-up) Display components tab if no holdings

This patch updates the default details tab selection to components for
the case when there are no holdings attached to the record and there is
no HTML5 content to display in preference.

Bug 11175: (QA follow-up) OPAC - Display components tab if no holdings

As for the staff client, this selects the default tab to be 'Components'
for the case where no holdings are found.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 506 Marcel de Rooy 2021-10-26 07:51:38 UTC
Created attachment 126890 [details] [review]
Bug 11175: (QA follow-up) Be more specific with title fields

We were using the MARC::Record title accessor, but that just stringifies
all the subfields of field 245. The indexer, and the XSLT, specifically
use 245a so we need to do the same.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 507 Marcel de Rooy 2021-10-26 07:51:46 UTC
Created attachment 126891 [details] [review]
Bug 11175: (QA follow-up) Restore link to more results

This patch restores the link to 'more results' should you exceed the
hard coded limit of 300 component parts.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 508 Marcel de Rooy 2021-10-26 07:51:55 UTC
Created attachment 126892 [details] [review]
Bug 11175: Add syspref MaxComponentRecords to remove hardcoded limit

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 509 Marcel de Rooy 2021-10-26 07:52:02 UTC
Created attachment 126893 [details] [review]
Bug 11175: Uniformize ComponentParts in template

More robust as it takes empty array into account

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 510 Marcel de Rooy 2021-10-26 07:52:11 UTC
Created attachment 126894 [details] [review]
Bug 11175: (QA follow-up) Remove caching _components

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 511 Marcel de Rooy 2021-10-26 07:52:19 UTC
Created attachment 126895 [details] [review]
Bug 11175: (QA follow-up) Do not fetch twice

We were fetching components in the first call of XSLTParse4Display
(opac-detail around L220). And again (opac-detail around L660).
Same for catalogue/detail.pl.
Moving the XSLT block in both scripts and removing the code from
XSLT.pm which again makes the tests obsolete.

Not hiding the link when there are components. Might be helpful too.

Also fixing biblionumber in the XSLTParse4Display call for parts.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 512 Martin Renvoize 2021-10-26 07:58:59 UTC
Latest rebase effectively reverts bug 29284, failing QA
Comment 513 Martin Renvoize 2021-10-26 09:50:19 UTC
Created attachment 126897 [details] [review]
Bug 11175: (QA follow-up) Restore bug 29284

This patch restores the functional fixes introduced in bug 29284 and
also prevents the 'Show analytics' link from displaying when no
component parts are found and inline display is enabled.
Comment 514 Martin Renvoize 2021-10-26 09:51:55 UTC
Created attachment 126898 [details] [review]
Bug 11175: (QA follow-up) Restore bug 29284

This patch restores the functional fixes introduced in bug 29284 and
also prevents the 'Show analytics' link from displaying when no
component parts are found and inline display is enabled.
Comment 515 Marcel de Rooy 2021-10-26 11:34:28 UTC
Created attachment 126904 [details] [review]
Bug 11175: (QA follow-up) Restore bug 29284

This patch restores the functional fixes introduced in bug 29284 and
also prevents the 'Show analytics' link from displaying when no
component parts are found and inline display is enabled.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: Fixed error with $err vs $error(s) :)
Comment 516 Marcel de Rooy 2021-10-26 12:49:14 UTC
Created attachment 126911 [details] [review]
Bug 11175: (QA follow-up) Test message contents for analytics error

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 517 Marcel de Rooy 2021-10-26 12:57:57 UTC
Created attachment 126912 [details] [review]
Bug 11175: (QA follow-up) Test message contents for analytics error

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 518 Jonathan Druart 2021-10-26 13:18:23 UTC
Created attachment 126913 [details] [review]
Bug 11175: Add tests

Add coverage for the message set by get_marc_components when the search
fails.
Also cleans a bit the weird return comparisons.
Comment 519 Jonathan Druart 2021-10-26 14:41:45 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 520 Martin Renvoize 2021-10-26 16:51:07 UTC
Nice one.. faith in community restored.. sorry for being grumpy
Comment 521 Olli-Antti Kivilahti 2021-11-11 08:50:40 UTC
Bug 11175,
we have gone such a long way together.
I am so proud of you, our bug.
Comment 522 David Cook 2022-05-30 01:44:09 UTC
(In reply to Martin Renvoize from comment #520)
> Nice one.. faith in community restored.. sorry for being grumpy

Now I'm feeling grumpy reading through these patches again 😅.

(In reply to Marcel de Rooy from comment #499)
> Created attachment 126883 [details] [review] [review]
> Bug 11175: (QA follow-up) Move get_component_part_query

This patch removed the double-quotes from the search terms which causes this to break when using Zebra if you're using a title like "MyTitle: MySubtitle" since the following query is a syntax error:

Host-item=(MyTitle= MySubtitle)

It should be:

Host-item=("MyTitle= MySubtitle")

According to https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html#query-string-syntax, it looks like Elasticsearch works the same way. 

Single words don't need to be quoted but terms consisting of more than one word do.

Regarding Zebra, we actually have the tools to easily catch this in our unit tests:

#!/usr/bin/perl
use strict;
use warnings;
use C4::Context;
use ZOOM;
my $Zconn = C4::Context->Zconn;
eval {
    my $bad = ZOOM::Query::CCL2RPN->new("Host-item=(MyTitle= MySubtitle)",$Zconn);
};
warn $@ if $@;
my $good = ZOOM::Query::CCL2RPN->new('Host-item=("MyTitle= MySubtitle")',$Zconn);

That code will output the following error:

ZOOM error 10014 "CCL parsing error" (addinfo: "Unknown qualifier") from diag-set 'ZOOM' at test.pl line 10.

It looks like https://metacpan.org/pod/Search::Elasticsearch::Compat::QueryParser#check() could provide similar error checking for Elasticsearch. 

If we just syntax checked our query strings, we'd run into far fewer problems... 

Anyway, I'll open a new bug report for this one. I just figured it would be good for folk to hear about it.
Comment 523 David Cook 2022-05-30 01:47:27 UTC
(In reply to David Cook from comment #522)
> (In reply to Martin Renvoize from comment #520)
> > Nice one.. faith in community restored.. sorry for being grumpy
> 
> Now I'm feeling grumpy reading through these patches again 😅.
> 

Actually now that I think about it... I feel like I've already grumped about this code recently...

Ah yes: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29418#c16
Comment 524 David Cook 2022-05-30 03:30:59 UTC
(In reply to David Cook from comment #522)
>
> Regarding Zebra, we actually have the tools to easily catch this in our unit
> tests:
> 

Except of course we're feeding the output of "get_components_query" into $searcher->simple_search_compat or into search.pl/opac-search.pl "q" parameter which both transform the query differently (both for Zebra and for Elasticsearch so we get at least 4x more things to check). 

Well... if it were easy it would already be done 🤣. Someday...