Bug 26314 - "Volumes: show volumes" showing regardless of whether there are volumes linked to the record
Summary: "Volumes: show volumes" showing regardless of whether there are volumes linke...
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Assignee: Martin Renvoize
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 15851 28316 28484 28830
Blocks:
  Show dependency treegraph
 
Reported: 2020-08-27 22:28 UTC by Katrin Fischer
Modified: 2023-09-18 18:43 UTC (History)
6 users (show)

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


Attachments
Bug 26314: Only display volumes link when required (6.37 KB, patch)
2021-08-09 10:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: Only display volumes link when required (8.13 KB, patch)
2021-09-01 13:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: Only display volumes link when required (8.12 KB, patch)
2021-09-01 15:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: (follow-up) Use clean_search_term (1.16 KB, patch)
2021-09-01 15:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: Only display volumes link when required (8.12 KB, patch)
2021-10-18 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: (follow-up) Use clean_search_term (1.16 KB, patch)
2021-10-18 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: (follow-up) Make title search explicit (1.02 KB, patch)
2021-10-18 17:13 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: Only display volumes link when required (7.70 KB, patch)
2021-10-27 07:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: (follow-up) Use clean_search_term (1.16 KB, patch)
2021-10-27 07:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: (follow-up) Make title search explicit (1.02 KB, patch)
2021-10-27 07:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: Update for changes to bug 11175 methodology (8.12 KB, patch)
2021-10-27 07:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: Only display volumes link when required (7.56 KB, patch)
2023-09-08 15:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: (follow-up) Use clean_search_term (1.16 KB, patch)
2023-09-08 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: (follow-up) Make title search explicit (1.02 KB, patch)
2023-09-08 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: Update for changes to bug 11175 methodology (9.71 KB, patch)
2023-09-08 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: (QA follow-up) Resolve new QA script failures (1.72 KB, patch)
2023-09-08 15:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26314: Only display volumes link when required (7.61 KB, patch)
2023-09-08 16:06 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 26314: (follow-up) Use clean_search_term (1.22 KB, patch)
2023-09-08 16:06 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 26314: (follow-up) Make title search explicit (1.08 KB, patch)
2023-09-08 16:06 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 26314: Update for changes to bug 11175 methodology (9.77 KB, patch)
2023-09-08 16:06 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 26314: (QA follow-up) Resolve new QA script failures (1.77 KB, patch)
2023-09-08 16:06 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 26314: Only display volumes link when required (7.61 KB, patch)
2023-09-15 14:06 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: (follow-up) Use clean_search_term (1.22 KB, patch)
2023-09-15 14:06 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: (follow-up) Make title search explicit (1.08 KB, patch)
2023-09-15 14:06 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: Update for changes to bug 11175 methodology (9.77 KB, patch)
2023-09-15 14:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: (QA follow-up) Resolve new QA script failures (1.83 KB, patch)
2023-09-15 14:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: (QA follow-up) Improve POD and fix links for UseControlNumber off (7.48 KB, patch)
2023-09-15 14:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: Only display volumes link when required (7.67 KB, patch)
2023-09-15 14:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: (follow-up) Use clean_search_term (1.28 KB, patch)
2023-09-15 14:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: (follow-up) Make title search explicit (1.14 KB, patch)
2023-09-15 14:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: Update for changes to bug 11175 methodology (9.82 KB, patch)
2023-09-15 14:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: (QA follow-up) Resolve new QA script failures (1.83 KB, patch)
2023-09-15 14:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26314: (QA follow-up) Improve POD and fix links for UseControlNumber off (7.48 KB, patch)
2023-09-15 14:07 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2020-08-27 22:28:37 UTC
When cataloguing multi part records with sets and volumes, we currently show a link to "Show volumes" on the set record. The link is always shown on a set record, even if there are no linked volumes.

Bug 15851 solves the same issue for analytics. volumes would be a great next step.

At the moment Show volumes is only shown on monographs, but we could add the feature for serial records as well.
Comment 1 Martin Renvoize 2021-08-09 10:45:09 UTC
Created attachment 123615 [details] [review]
Bug 26314: Only display volumes link when required

This patch makes C4::XSLT query for volumes the same way it would do
with the generated link (i.e. based on UseControlNumber) and passes
a flag to the XSLT so it displays (or not) the 'Show volumes' link.

To test:
1. Apply the first patch
2. Have a known record without volumes
3. Open the record in the OPAC
=> FAIL: It shows the 'Show volumes' link
4. Have a record known to have volumes
5. Open the record in the OPAC
=> SUCCESS: It shows the 'Show volumes' link
6. Apply this patch and restart_all
7. Reload the above records
=> SUCCESS: It shows the link where it has to, and hides it where it
   shouldn't be displayed.
8. Repeat for Intranet
9. Sign off :-D
Comment 2 Martin Renvoize 2021-08-09 10:45:56 UTC
Built with inspiration based on bug 11175
Comment 3 Fridolin Somers 2021-08-17 05:37:15 UTC
Nice.

But like for analyticals, please add eval() in get_marc_volumes() in case search fails, see Bug 28682
Comment 4 Martin Renvoize 2021-08-17 05:45:10 UTC
Good call, I wasn't aware of that bug.  I'll add a follow-up layer today.
Comment 5 Martin Renvoize 2021-09-01 13:39:49 UTC
Created attachment 124367 [details] [review]
Bug 26314: Only display volumes link when required

This patch makes C4::XSLT query for volumes the same way it would do
with the generated link (i.e. based on UseControlNumber) and passes
a flag to the XSLT so it displays (or not) the 'Show volumes' link.

To test:
1. Apply the first patch
2. Have a known record without volumes
3. Open the record in the OPAC
=> FAIL: It shows the 'Show volumes' link
4. Have a record known to have volumes
5. Open the record in the OPAC
=> SUCCESS: It shows the 'Show volumes' link
6. Apply this patch and restart_all
7. Reload the above records
=> SUCCESS: It shows the link where it has to, and hides it where it
   shouldn't be displayed.
8. Repeat for Intranet
9. Sign off :-D
Comment 6 Martin Renvoize 2021-09-01 14:34:50 UTC
Regarding the eval Fridolin mentions.. I'm tempted to switch the get_volumes_query routine to using build_query_compat internally instead which should yield us a more reliably constructed search query string with special characters removed. That way, with bug 28316 work progressing we will get the advantages it brings.
Comment 7 Martin Renvoize 2021-09-01 15:14:01 UTC
Created attachment 124380 [details] [review]
Bug 26314: Only display volumes link when required

This patch makes C4::XSLT query for volumes the same way it would do
with the generated link (i.e. based on UseControlNumber) and passes
a flag to the XSLT so it displays (or not) the 'Show volumes' link.

To test:
1. Apply the first patch
2. Have a known record without volumes
3. Open the record in the OPAC
=> FAIL: It shows the 'Show volumes' link
4. Have a record known to have volumes
5. Open the record in the OPAC
=> SUCCESS: It shows the 'Show volumes' link
6. Apply this patch and restart_all
7. Reload the above records
=> SUCCESS: It shows the link where it has to, and hides it where it
   shouldn't be displayed.
8. Repeat for Intranet
9. Sign off :-D
Comment 8 Martin Renvoize 2021-09-01 15:14:05 UTC
Created attachment 124381 [details] [review]
Bug 26314: (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.
Comment 9 Martin Renvoize 2021-10-18 17:13:33 UTC
Created attachment 126448 [details] [review]
Bug 26314: Only display volumes link when required

This patch makes C4::XSLT query for volumes the same way it would do
with the generated link (i.e. based on UseControlNumber) and passes
a flag to the XSLT so it displays (or not) the 'Show volumes' link.

To test:
1. Apply the first patch
2. Have a known record without volumes
3. Open the record in the OPAC
=> FAIL: It shows the 'Show volumes' link
4. Have a record known to have volumes
5. Open the record in the OPAC
=> SUCCESS: It shows the 'Show volumes' link
6. Apply this patch and restart_all
7. Reload the above records
=> SUCCESS: It shows the link where it has to, and hides it where it
   shouldn't be displayed.
8. Repeat for Intranet
9. Sign off :-D
Comment 10 Martin Renvoize 2021-10-18 17:13:36 UTC
Created attachment 126449 [details] [review]
Bug 26314: (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.
Comment 11 Martin Renvoize 2021-10-18 17:13:40 UTC
Created attachment 126450 [details] [review]
Bug 26314: (follow-up) Make title search explicit

This patch updates the get_volumes_query search returned when
UseControlNumber is disabled such that it matches the query produced by
XSLT sheets by replacing the ->title accessor with the more specific
->subfield('245', "a") accessor to just take subfield `a` into account.
Comment 12 Marcel de Rooy 2021-10-25 10:07:05 UTC
I think we should not load this logic into XSLT module. It does not belong there imo.
Just pass the xslt_variable show_volumes_link in the detail scripts using it (probably opac-detail, catalogue detail, shelves?)
If this would result in code duplication, add a biblio method for it?
Comment 13 Martin Renvoize 2021-10-26 10:57:37 UTC
Understood.

Let's get 11175 over the finish line before we continue on this one however.  I think we should also pass the query string itself as an XSLT param and drop the logic in the XSLT.. that way we can guarantee the link search and the search used in the controllers are always in sync.
Comment 14 Martin Renvoize 2021-10-27 07:52:42 UTC
Created attachment 126952 [details] [review]
Bug 26314: Only display volumes link when required

This patch makes C4::XSLT query for volumes the same way it would do
with the generated link (i.e. based on UseControlNumber) and passes
a flag to the XSLT so it displays (or not) the 'Show volumes' link.

To test:
1. Apply the first patch
2. Have a known record without volumes
3. Open the record in the OPAC
=> FAIL: It shows the 'Show volumes' link
4. Have a record known to have volumes
5. Open the record in the OPAC
=> SUCCESS: It shows the 'Show volumes' link
6. Apply this patch and restart_all
7. Reload the above records
=> SUCCESS: It shows the link where it has to, and hides it where it
   shouldn't be displayed.
8. Repeat for Intranet
9. Sign off :-D
Comment 15 Martin Renvoize 2021-10-27 07:52:46 UTC
Created attachment 126953 [details] [review]
Bug 26314: (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.
Comment 16 Martin Renvoize 2021-10-27 07:52:49 UTC
Created attachment 126954 [details] [review]
Bug 26314: (follow-up) Make title search explicit

This patch updates the get_volumes_query search returned when
UseControlNumber is disabled such that it matches the query produced by
XSLT sheets by replacing the ->title accessor with the more specific
->subfield('245', "a") accessor to just take subfield `a` into account.
Comment 17 Martin Renvoize 2021-10-27 07:52:53 UTC
Created attachment 126955 [details] [review]
Bug 26314: Update for changes to bug 11175 methodology

This moves the show_volumes calculation back out of C4::XSLT into the
controller scripts and refined the search query builder slightly based
on the XSLT equivilent.
Comment 18 Martin Renvoize 2021-10-27 07:57:23 UTC
I still think it might be a good idea to pass the search string from the controller instead of constructing it in two places.. keeps it dry.
Comment 19 Marcel de Rooy 2021-10-27 11:03:46 UTC
(In reply to Martin Renvoize from comment #18)
> I still think it might be a good idea to pass the search string from the
> controller instead of constructing it in two places.. keeps it dry.

Not sure what you mean here.
Constructing the query for components and volumes kind of the same? The shared logic could be moved?
Comment 20 Martin Renvoize 2021-10-27 15:29:12 UTC
(In reply to Marcel de Rooy from comment #19)
> (In reply to Martin Renvoize from comment #18)
> > I still think it might be a good idea to pass the search string from the
> > controller instead of constructing it in two places.. keeps it dry.
> 
> Not sure what you mean here.
> Constructing the query for components and volumes kind of the same? The
> shared logic could be moved?

Constructing the queries for 'components parts' and 'volumes' is similar, but not the same.. it makes sense to have methods for the two distinctly.

What I'm talking about is that the XSLT's also construct the same search query strings.. it would be cleaner to pass the constructed query string to the xslt as a parameter so we're re-using the code that builds the query.
Comment 21 Michaela Sieber 2023-09-06 14:07:01 UTC
+1
Comment 22 Katrin Fischer 2023-09-08 14:07:39 UTC
I tried to rebase, but I get stuck at the very last patch with a 'fake ancestor'. 
Martin, could you have another look maybe? I think I got you some testers :)
Comment 23 Martin Renvoize 2023-09-08 15:18:58 UTC
Created attachment 155437 [details] [review]
Bug 26314: Only display volumes link when required

This patch makes C4::XSLT query for volumes the same way it would do
with the generated link (i.e. based on UseControlNumber) and passes
a flag to the XSLT so it displays (or not) the 'Show volumes' link.

To test:
1. Apply the first patch
2. Have a known record without volumes
3. Open the record in the OPAC
=> FAIL: It shows the 'Show volumes' link
4. Have a record known to have volumes
5. Open the record in the OPAC
=> SUCCESS: It shows the 'Show volumes' link
6. Apply this patch and restart_all
7. Reload the above records
=> SUCCESS: It shows the link where it has to, and hides it where it
   shouldn't be displayed.
8. Repeat for Intranet
9. Sign off :-D
Comment 24 Martin Renvoize 2023-09-08 15:19:01 UTC
Created attachment 155438 [details] [review]
Bug 26314: (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.
Comment 25 Martin Renvoize 2023-09-08 15:19:03 UTC
Created attachment 155439 [details] [review]
Bug 26314: (follow-up) Make title search explicit

This patch updates the get_volumes_query search returned when
UseControlNumber is disabled such that it matches the query produced by
XSLT sheets by replacing the ->title accessor with the more specific
->subfield('245', "a") accessor to just take subfield `a` into account.
Comment 26 Martin Renvoize 2023-09-08 15:19:06 UTC
Created attachment 155440 [details] [review]
Bug 26314: Update for changes to bug 11175 methodology

This moves the show_volumes calculation back out of C4::XSLT into the
controller scripts and refined the search query builder slightly based
on the XSLT equivilent.
Comment 27 Martin Renvoize 2023-09-08 15:19:09 UTC
Created attachment 155441 [details] [review]
Bug 26314: (QA follow-up) Resolve new QA script failures
Comment 28 Martin Renvoize 2023-09-08 15:21:46 UTC
Rebased as requested.. I've made sure the QA scripts are happy and that the tests are all passing.. but I've not actually confirmed the functionality is as it was originally intended.  (Honestly, it's long enough ago, I can't totally remember how it's meant to work :( )

I look forward to feedback though, so please test :)
Comment 29 Michaela Sieber 2023-09-08 15:26:49 UTC
(In reply to Martin Renvoize from comment #28)
> Rebased as requested.. I've made sure the QA scripts are happy and that the
> tests are all passing.. but I've not actually confirmed the functionality is
> as it was originally intended.  (Honestly, it's long enough ago, I can't
> totally remember how it's meant to work :( )
> 
> I look forward to feedback though, so please test :)

Thanks Martin! I start testing in a few minutes
Comment 30 Biblibre Sandboxes 2023-09-08 16:06:22 UTC
Created attachment 155446 [details] [review]
Bug 26314: Only display volumes link when required

This patch makes C4::XSLT query for volumes the same way it would do
with the generated link (i.e. based on UseControlNumber) and passes
a flag to the XSLT so it displays (or not) the 'Show volumes' link.

To test:
1. Apply the first patch
2. Have a known record without volumes
3. Open the record in the OPAC
=> FAIL: It shows the 'Show volumes' link
4. Have a record known to have volumes
5. Open the record in the OPAC
=> SUCCESS: It shows the 'Show volumes' link
6. Apply this patch and restart_all
7. Reload the above records
=> SUCCESS: It shows the link where it has to, and hides it where it
   shouldn't be displayed.
8. Repeat for Intranet
9. Sign off :-D

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 31 Biblibre Sandboxes 2023-09-08 16:06:25 UTC
Created attachment 155447 [details] [review]
Bug 26314: (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: Michaela Sieber <michaela.sieber@kit.edu>
Comment 32 Biblibre Sandboxes 2023-09-08 16:06:28 UTC
Created attachment 155448 [details] [review]
Bug 26314: (follow-up) Make title search explicit

This patch updates the get_volumes_query search returned when
UseControlNumber is disabled such that it matches the query produced by
XSLT sheets by replacing the ->title accessor with the more specific
->subfield('245', "a") accessor to just take subfield `a` into account.

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 33 Biblibre Sandboxes 2023-09-08 16:06:31 UTC
Created attachment 155449 [details] [review]
Bug 26314: Update for changes to bug 11175 methodology

This moves the show_volumes calculation back out of C4::XSLT into the
controller scripts and refined the search query builder slightly based
on the XSLT equivilent.

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 34 Biblibre Sandboxes 2023-09-08 16:06:33 UTC
Created attachment 155450 [details] [review]
Bug 26314: (QA follow-up) Resolve new QA script failures

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 35 Michaela Sieber 2023-09-08 16:23:33 UTC

Works as expected :-)

If you would like to have a look on my tests please check Sandbox:
https://staff-b26314.sandboxes.biblibre.eu/



If you search for 

index-term-genre:Monografische Reihe

you will find 3 series

only the second has the 'Show volumes' link

The first and the third don't have this link because there are no volumes.
Comment 36 Michaela Sieber 2023-09-08 16:38:14 UTC
P(In reply to Michaela Sieber from comment #35)
> 
> Works as expected :-)
> 
> If you would like to have a look on my tests please check Sandbox:
> https://staff-b26314.sandboxes.biblibre.eu/


I forgot to mention that System preference UseControlNumber is set to ‘Use’

"Use record control number ($w subfields) and control number (001) for linking of bibliographic records. "
Comment 37 Martin Renvoize 2023-09-08 20:56:24 UTC
Awesome, thanks for testing Michaela 😁
Comment 38 Martin Renvoize 2023-09-08 21:00:51 UTC
Comment on attachment 155446 [details] [review]
Bug 26314: Only display volumes link when required

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

Note to self, do we still need to 008 and other check we have in the XSLT here after the penultimate patch?
Comment 39 Katrin Fischer 2023-09-15 11:15:22 UTC
> 
> Note to self, do we still need to 008 and other check we have in the XSLT
> here after the penultimate patch?

The POD in this patch is not completely right as it only talks about 773, we could correct that, but I am not sure if it's worth changing up things now.

There are two ways to express a Set > volumes relationship:

Case 1: 773
Set:
001 		xxx
003 		DE-627

Volume with dependent title:
773		
_a volume title
_w xxx


Set:
001 		1020782544
003 		DE-627

Case 2: 8xx/449 pairs
Volume with independent title:
830 		0 	
_a Advances in analysis and geometry ;
_v Volume 5
_w 1020782544
_x 2511-0438 


And actually Michaela tested the second case. So we know it works with both cases, with and without the (003) prefix $w. (I verified)

The problem I see is the rcn index is broad. It indexes all $w subfields from various linking fields.

That's why the link includes NOT (bib-level:a OR bib-level:b) - so we can differentiate between analytical records and monographic ones.

... and that's why I thikn we better keep the checks on 008 and LDR.
Comment 40 Katrin Fischer 2023-09-15 14:06:54 UTC
Created attachment 155670 [details] [review]
Bug 26314: Only display volumes link when required

This patch makes C4::XSLT query for volumes the same way it would do
with the generated link (i.e. based on UseControlNumber) and passes
a flag to the XSLT so it displays (or not) the 'Show volumes' link.

To test:
1. Apply the first patch
2. Have a known record without volumes
3. Open the record in the OPAC
=> FAIL: It shows the 'Show volumes' link
4. Have a record known to have volumes
5. Open the record in the OPAC
=> SUCCESS: It shows the 'Show volumes' link
6. Apply this patch and restart_all
7. Reload the above records
=> SUCCESS: It shows the link where it has to, and hides it where it
   shouldn't be displayed.
8. Repeat for Intranet
9. Sign off :-D

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 41 Katrin Fischer 2023-09-15 14:06:56 UTC
Created attachment 155671 [details] [review]
Bug 26314: (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: Michaela Sieber <michaela.sieber@kit.edu>
Comment 42 Katrin Fischer 2023-09-15 14:06:59 UTC
Created attachment 155672 [details] [review]
Bug 26314: (follow-up) Make title search explicit

This patch updates the get_volumes_query search returned when
UseControlNumber is disabled such that it matches the query produced by
XSLT sheets by replacing the ->title accessor with the more specific
->subfield('245', "a") accessor to just take subfield `a` into account.

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 43 Katrin Fischer 2023-09-15 14:07:01 UTC
Created attachment 155673 [details] [review]
Bug 26314: Update for changes to bug 11175 methodology

This moves the show_volumes calculation back out of C4::XSLT into the
controller scripts and refined the search query builder slightly based
on the XSLT equivilent.

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 44 Katrin Fischer 2023-09-15 14:07:04 UTC
Created attachment 155674 [details] [review]
Bug 26314: (QA follow-up) Resolve new QA script failures

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 45 Katrin Fischer 2023-09-15 14:07:07 UTC
Created attachment 155675 [details] [review]
Bug 26314: (QA follow-up) Improve POD and fix links for UseControlNumber off

When UseControlNumber is off, we previously did a search on the title index,
but this was not actually correct. It might work when we have a dependent
title and the title also appears on 245$p, but there is a better way:

The title of the set record will be found in:
* 773$3 for a dependent title
* 800/810/811/830 for an independent title

The search indices to use are host-item and title-series.

As we are not only looking for 773, POD was adjusted to reflect that.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 46 Katrin Fischer 2023-09-15 14:07:27 UTC
Created attachment 155676 [details] [review]
Bug 26314: Only display volumes link when required

This patch makes C4::XSLT query for volumes the same way it would do
with the generated link (i.e. based on UseControlNumber) and passes
a flag to the XSLT so it displays (or not) the 'Show volumes' link.

To test:
1. Apply the first patch
2. Have a known record without volumes
3. Open the record in the OPAC
=> FAIL: It shows the 'Show volumes' link
4. Have a record known to have volumes
5. Open the record in the OPAC
=> SUCCESS: It shows the 'Show volumes' link
6. Apply this patch and restart_all
7. Reload the above records
=> SUCCESS: It shows the link where it has to, and hides it where it
   shouldn't be displayed.
8. Repeat for Intranet
9. Sign off :-D

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 47 Katrin Fischer 2023-09-15 14:07:30 UTC
Created attachment 155677 [details] [review]
Bug 26314: (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: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 48 Katrin Fischer 2023-09-15 14:07:33 UTC
Created attachment 155678 [details] [review]
Bug 26314: (follow-up) Make title search explicit

This patch updates the get_volumes_query search returned when
UseControlNumber is disabled such that it matches the query produced by
XSLT sheets by replacing the ->title accessor with the more specific
->subfield('245', "a") accessor to just take subfield `a` into account.

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 49 Katrin Fischer 2023-09-15 14:07:35 UTC
Created attachment 155679 [details] [review]
Bug 26314: Update for changes to bug 11175 methodology

This moves the show_volumes calculation back out of C4::XSLT into the
controller scripts and refined the search query builder slightly based
on the XSLT equivilent.

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 50 Katrin Fischer 2023-09-15 14:07:38 UTC
Created attachment 155680 [details] [review]
Bug 26314: (QA follow-up) Resolve new QA script failures

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 51 Katrin Fischer 2023-09-15 14:07:41 UTC
Created attachment 155681 [details] [review]
Bug 26314: (QA follow-up) Improve POD and fix links for UseControlNumber off

When UseControlNumber is off, we previously did a search on the title index,
but this was not actually correct. It might work when we have a dependent
title and the title also appears on 245$p, but there is a better way:

The title of the set record will be found in:
* 773$3 for a dependent title
* 800/810/811/830 for an independent title

The search indices to use are host-item and title-series.

As we are not only looking for 773, POD was adjusted to reflect that.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 52 Katrin Fischer 2023-09-15 14:28:10 UTC
Just a note on the 008: Yes, it still makes sense:

For continuing resources:
008, Pos. 21 = m - Monographic series

As the 008 fields change depending on LDR 6 and 7, there are additional requirements:

LDR, pos. 6 = Language material
AND
LDR, pos. 7 = 
 b - Serial component part
 i - integrating resource
 s - serial

We check LDR pos. 6 but omit pos=7. This was there previously and I think it does no harm for now.
Comment 53 Tomás Cohen Arazi 2023-09-18 18:37:04 UTC
Comment on attachment 155678 [details] [review]
Bug 26314: (follow-up) Make title search explicit


>-        my $cleaned_title = $marc->title;
>+        my $cleaned_title = $marc->subfield('245', "a");

Is this correct if we think of UNIMARC?
Comment 54 Katrin Fischer 2023-09-18 18:43:43 UTC
(In reply to Tomás Cohen Arazi from comment #53)
> Comment on attachment 155678 [details] [review] [review]
> Bug 26314: (follow-up) Make title search explicit
> 
> 
> >-        my $cleaned_title = $marc->title;
> >+        my $cleaned_title = $marc->subfield('245', "a");
> 
> Is this correct if we think of UNIMARC?

No, it wouldn't be, but this is currently marked as a MARC21 only feature.