Bug 30162 - XSLT has broken link for traced series because of OPAC/staff interface confusion
Summary: XSLT has broken link for traced series because of OPAC/staff interface confusion
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-02-23 14:15 UTC by Katrin Fischer
Modified: 2023-12-28 20:44 UTC (History)
4 users (show)

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


Attachments
the Series link koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl is not generated correctly (3.25 KB, patch)
2022-09-09 06:21 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 30162 - XSLT has broken link for traced series because of OPAC/staff interface confusion (2.21 KB, patch)
2022-09-11 14:25 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 30162: Fix staff interface link in OPAC XSLT files for 830$w (3.18 KB, patch)
2023-02-14 09:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 30162: Fix staff interface link in OPAC XSLT files for 830$w (3.18 KB, patch)
2023-02-24 13:29 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 30162: Fix staff interface link in OPAC XSLT files for 830$w (3.23 KB, patch)
2023-03-12 17:51 UTC, David Nind
Details | Diff | Splinter Review
Bug 30162: Fix staff interface link in OPAC XSLT files for 830$w (3.28 KB, patch)
2023-03-28 09:55 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2022-02-23 14:15:14 UTC
In the Utils file for the OPAC XSLT is a hardcoded link for the staff interface, which means the link is broken in OPAC and generates an ugly error when clicked:

https://git.koha-community.org/Koha-community/Koha/src/branch/master/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl#L473

                    <xsl:when test="$UseControlNumber = '1' and marc:subfield[@code='w']">
                        <a href="/cgi-bin/koha/catalogue/search.pl?q=rcn:{marc:subfield[@code='w']}">
                            <xsl:call-template name="chopPunctuation">
                                <xsl:with-param name="chopString">
                                    <xsl:call-template name="subfieldSelect">
                                        <xsl:with-param name="codes">a_t</xsl:with-param>
                                    </xsl:call-template>
                                </xsl:with-param>
                            </xsl:call-template>
                        </a>

It should use the searchurl variable used in other spots of the same template.

Also it doesn't work for me with the {} - I believe they might only work with Elasticsearch or ICU? Removing them makes the link behave correctly.

The record for testing should have these fields and USeControlNumber needs to be set to use:

490 _1 
_aKirche, Wirtschaft, Gesellschaft
_v1 
830 _0 	
_aKirche, Wirtschaft, Gesellschaft
_v1
_w(DE-627)577593544
Comment 1 Katrin Fischer 2022-02-23 14:18:34 UTC
Locally fixed as:
- <a href="/cgi-bin/koha/catalogue/search.pl?q=rcn:{marc:subfield[@code='w']}">
+ <a><xsl:attribute name="href"><xsl:value-of select="$searchurl"/>?q=rcn:<xsl:value-of select="marc:subfield[@code='w']"/></xsl:attribute>
Comment 2 Mark Hofstetter 2022-09-09 06:16:23 UTC
We always have problems with the rcn search

Please try on koha testing docker:


1. set UseControlNumber to "Use"
2. for the BKS Framework make 830 $0 $w $t $v editable
3. choose two records:
   A (eg Perl best practices / Damian Conway. (Record number 5)) as child
   B Programming Perl / Tom Christiansen, Brian D. Foy & Larry Wall. (Record number 262) as parent record
4. edit the child record, 
   - set 490 indicator 1 to 1
   - set 830$w (and maybe 830$0 to control-number of the parent in my case 17259930)
   - set 830$t = "perl series title" and 830$v = "Volume 1"

5. apply the patch as suggested by Katrin Fischer 2022-02-23 14:18:34 UTC 

6. go to the child record, the "Series" link will be something like
opac-search.pl?q=rcn:17259930

7. if you click the link it leads to the child record itself and not to the parent record

8. I would suggest the following patch point to the control-number
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl b/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
index b734afd754..20d2c0ae83 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
@@ -470,7 +470,7 @@
             <xsl:for-each select="marc:datafield[@tag=830]">
                 <xsl:choose>
                     <xsl:when test="$UseControlNumber = '1' and marc:subfield[@code='w']">
-                        <a href="/cgi-bin/koha/catalogue/search.pl?q=rcn:{marc:subfield[@code='w']}">
+                        <a><xsl:attribute name="href"><xsl:value-of select="$searchurl"/>?q=control-number:<xsl:value-of select="str:encode-uri(marc:subfield[@code='w'], true())"/></xsl:attribute>
                             <xsl:call-template name="chopPunctuation">
                                 <xsl:with-param name="chopString">
                                     <xsl:call-template name="subfieldSelect">
Comment 3 Mark Hofstetter 2022-09-09 06:21:02 UTC
Created attachment 140360 [details] [review]
the Series link koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl is not generated correctly

to reproduce

1. set UseControlNumber to "Use"
2. for the BKS Framework make 830 $0 $w $t $v editable
3. choose two records:
   A (eg Perl best practices / Damian Conway. (Record number 5)) as child
   B Programming Perl / Tom Christiansen, Brian D. Foy & Larry Wall. (Record number 262) as parent record
4. edit the child record,
   - set 490 indicator 1 to 1
   - set 830$w (and maybe 830$0 to control-number of the parent in my case 17259930)
   - set 830$t = "perl series title" and 830$v = "Volume 1"

5. apply the patch as suggested by Katrin Fischer 2022-02-23 14:18:34 UTC

6. go to the child record, the "Series" link will be something like
opac-search.pl?q=rcn:17259930

7. if you click the link it leads to the child record itself and not to the parent record

8. I would suggest the following patch point to the control-number
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl b/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
index b734afd754..20d2c0ae83 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
@@ -470,7 +470,7 @@
             <xsl:for-each select="marc:datafield[@tag=830]">
                 <xsl:choose>
                     <xsl:when test="$UseControlNumber = '1' and marc:subfield[@code='w']">
-                        <a href="/cgi-bin/koha/catalogue/search.pl?q=rcn:{marc:subfield[@code='w']}">
+                        <a><xsl:attribute name="href"><xsl:value-of select="$searchurl"/>?q=control-number:<xsl:value-of select="str:encode-uri(marc:subfield[@code='w'], true())"/></xsl:attribute>
                             <xsl:call-template name="chopPunctuation">
                                 <xsl:with-param name="chopString">
                                     <xsl:call-template name="subfieldSelect">
Comment 4 Katrin Fischer 2022-09-11 13:25:43 UTC
Not sure if this was ready for testing yet, but:

The proposed fix doesn't only fix the link, but it changes the functionality. :(

When this feature was introduced, we decided on a horizontal search for traced series. Meaning that if you click on the link, you find the other volumes of the series. (using rcn search for $w)

This patch changes it go a hierarchical link linking upwards to the traced serial record. (using control-number search for 001)

I think if you really want to change this, it should be a system preference AND a separate bug. This here is only about fixing a broken link, so changing the behaviour is out of scope. Having separate bugs also helps with backporting.

Please also have a look at: https://wiki.koha-community.org/wiki/Commit_messages
We need the bug number first in the commit message, so the tools work right: Bug XXXX: ....
Commit message also needs to have a problem description and the test plan.

Bug status and Assignee should also be changed. Hmm - maybe this was still work in progress?

I am feeling a little guilty for filing and forgetting about this one - so if you don't have the time now - I could try and provide the link fix myself.
Comment 5 Mark Hofstetter 2022-09-11 14:25:16 UTC
Created attachment 140403 [details] [review]
Bug 30162 - XSLT has broken link for traced series because of OPAC/staff interface confusion

the link to for series defined in 830$w in OPAC is generated incorrectly this patch
creates the link properly

to test:
1. set UseControlNumber to "Use"
2. for the BKS Framework make 830 $0 $w $t $v editable
3. choose two records:
   A (eg Perl best practices / Damian Conway. (Record number 5)) as child
   B Programming Perl / Tom Christiansen, Brian D. Foy & Larry Wall. (Record number 262) as parent record
4. edit the child record,
   - set 490 indicator 1 to 1
   - set 830$w (and maybe 830$0 to control-number of the parent in my case 17259930)
   - set 830$t = "perl series title" and 830$v = "Volume 1"
5. before the pathc you'll get a 404 error because of the wrong
6. apply patch
7. the link should now work, and point to the series, (not to the parent record)
Comment 6 Mark Hofstetter 2022-09-11 14:27:48 UTC
ok now that I understand the intention it makes sense for me, made "just" a bugfix
Comment 7 Victor Grousset/tuxayo 2022-09-26 16:35:43 UTC
Comment on attachment 140360 [details] [review]
the Series link koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl is not generated correctly

I'm assuming this attachement:
https://bugs.koha-community.org/bugzilla3/buglist.cgi?quicksearch=30162
isn't valid anymore and is just a previous version of the current patch.
Comment 8 Anke Bruns 2022-09-26 17:03:15 UTC
We tried to test this patch but couldn't reproduce the 404 error, and we couldn't even link the two records either by proceding as mentioned.

490 was empty so we couldn't set the indicator to 1 in the first place. Then, the entries in 830$0, $t, $v and $w didn't seem to have any effect (we made sure to set UseControlNumber to "Use"). Even when we put some content into 490 so we were able to set the indicator to 1, it didn't have any effect visible to us.

Can you detail the steps to take in order to get linked records (and/or to reproduce the error)?

Which is the name of the link that is broken (that leads to 404)?
Comment 9 Anke Bruns 2022-09-26 17:10:26 UTC
False alert :-)

All I said above applies to the Staff client but we can reproduce the error in the OPAC!

However, we tried the patch, but it couldn't be applied in the sandbox:

error: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl).
error: could not build fake ancestor
Patch failed at 0001 Bug 30162 - XSLT has broken link for traced series because of OPAC/staff interface confusion
Comment 10 Katrin Fischer 2022-11-20 13:49:56 UTC
Patch doesn't apply in a mean way, wondering if this was maybe written for an older version/not on master branch?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 30162 - XSLT has broken link for traced series because of OPAC/staff interface confusion
error: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl).
error: could not build fake ancestor
Patch failed at 0001 Bug 30162 - XSLT has broken link for traced series because of OPAC/staff interface confusion
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-30162---XSLT-has-broken-link-for-traced-series-_pb72j.patch
Comment 11 Katrin Fischer 2022-11-20 14:03:25 UTC
I tried rewriting this patch for master, but there is no longer any mention of the control-number index in the Utils file on master:

        <xsl:for-each select="marc:datafield[@tag=830 and @ind1!='z']">
            <xsl:choose>
                <xsl:when test="$UseControlNumber = '1' and marc:subfield[@code='w']">
                    <a href="/cgi-bin/koha/catalogue/search.pl?q=rcn:{marc:subfield[@code='w']}">


More so, to me it looks like it has always been rcn and not control-numer - could this have been a local change? Scratching my head looking at the git histories here:

https://git.koha-community.org/Koha-community/Koha/commit/c4e04e2cbaff0a9af124dc5a28ddbdc7058405d0
Comment 12 Katrin Fischer 2023-02-14 09:58:29 UTC
Created attachment 146610 [details] [review]
Bug 30162: Fix staff interface link in OPAC XSLT files for 830$w

All the links in the Utils files shoudl be built using the $searchurl
variable to adapt the link to either staff or OPAC.

To test:
* Enable UseControlNumber system preference
* Create a record with the following fields:

490 1 _ ‡aEffective software development series
830 _ 0 ‡aEffective software development series.‡w13736978

  $w can be anything for this use case or another $w from your catalog.
  Easiest might be to enable the Advanced cataloguing editor to be
  able to insert these lines easily. If you are using the sample data,
  the example shoudl work as is.

* Save the record and open it in the staff interface and the OPAC
* The link in the OPAC should be broken, staff should work
* Apply patch
* Now both links should work

Co-authored-by: Anke Bruns <Anke.Bruns@gwdg.de>
Comment 13 Anke Bruns 2023-02-14 18:06:43 UTC
Tried to test the patch today but found some unexpected behavior in the sandbox:

- Link in OPAC was broken as expected, but...

- Link in Staff interface didn't lead to other titles in the same series but instead to the detail view of the very record in which the link was clicked. This was true as well for my catalogued test record as for the record already in the sandbox which belonged to the same series "Effective software development series".

- I tried to apply the patch anyway but received an error message:

"fatal: detected dubious ownership in repository at '/kohadevbox/koha'
To add an exception for this directory, call:

	git config --global --add safe.directory /kohadevbox/koha
Traceback (most recent call last):
  File "/usr/bin/git-bz", line 2716, in <module>
    applied = do_apply(bug_ref)
  File "/usr/bin/git-bz", line 1688, in do_apply
    git_dir = git.rev_parse(git_dir=True)
  File "/usr/bin/git-bz", line 192, in f
    return git_run(command, *args, **kwargs)
  File "/usr/bin/git-bz", line 174, in git_run
    raise CalledProcessError(process.returncode, " ".join(to_run))
subprocess.CalledProcessError: Command 'git rev-parse --git-dir' returned non-zero exit status 128"
Comment 14 Katrin Fischer 2023-02-14 18:10:04 UTC
Hi Anke,

the patch $w link here works as expected - it finds other records (including itself) with the same $w link. As we faked the example, it doesn't make a lot of sense.

The error you see is due to the sandbox issue :( Adding Joubu.
Comment 15 Jonathan Druart 2023-02-15 06:52:27 UTC
Can you retry today on biblibre's sandboxes please?

https://sandboxes.biblibre.eu/

There was an error that should be fixed now.
Comment 16 Katrin Fischer 2023-02-17 09:30:08 UTC
I believe this should test ok now with the hopefully fixed sandboxes. Maybe try the Biblibre ones first since we had some issues yesterday on the others.
Comment 17 Anke Bruns 2023-02-24 11:00:02 UTC
Sandbox needs some more fixing.
Comment 18 Katrin Fischer 2023-02-24 13:29:52 UTC
Created attachment 147329 [details] [review]
Bug 30162: Fix staff interface link in OPAC XSLT files for 830$w

All the links in the Utils files shoudl be built using the $searchurl
variable to adapt the link to either staff or OPAC.

To test:
* Enable UseControlNumber system preference
* Create a record with the following fields:

490 1 _ ‡aEffective software development series
830 _ 0 ‡aEffective software development series.‡w13736978

  $w can be anything for this use case or another $w from your catalog.
  Easiest might be to enable the Advanced cataloguing editor to be
  able to insert these lines easily. If you are using the sample data,
  the example shoudl work as is.

* Save the record and open it in the staff interface and the OPAC
* The link in the OPAC should be broken, staff should work
* Apply patch
* Now both links should work

Co-authored-by: Anke Bruns <Anke.Bruns@gwdg.de>
Comment 19 Katrin Fischer 2023-02-24 13:30:45 UTC
Sorry, I forgot to obsolete Mark's patch and mine did not apply on top of this. It should be ok now and I just rebased the patch on master too.
Comment 20 David Nind 2023-03-12 17:51:31 UTC
Created attachment 148111 [details] [review]
Bug 30162: Fix staff interface link in OPAC XSLT files for 830$w

All the links in the Utils files shoudl be built using the $searchurl
variable to adapt the link to either staff or OPAC.

To test:
* Enable UseControlNumber system preference
* Create a record with the following fields:

490 1 _ ‡aEffective software development series
830 _ 0 ‡aEffective software development series.‡w13736978

  $w can be anything for this use case or another $w from your catalog.
  Easiest might be to enable the Advanced cataloguing editor to be
  able to insert these lines easily. If you are using the sample data,
  the example shoudl work as is.

* Save the record and open it in the staff interface and the OPAC
* The link in the OPAC should be broken, staff should work
* Apply patch
* Now both links should work

Co-authored-by: Anke Bruns <Anke.Bruns@gwdg.de>
Signed-off-by: David Nind <david@davidnind.com>
Comment 21 Nick Clemens 2023-03-28 09:55:56 UTC
Created attachment 148809 [details] [review]
Bug 30162: Fix staff interface link in OPAC XSLT files for 830$w

All the links in the Utils files shoudl be built using the $searchurl
variable to adapt the link to either staff or OPAC.

To test:
* Enable UseControlNumber system preference
* Create a record with the following fields:

490 1 _ ‡aEffective software development series
830 _ 0 ‡aEffective software development series.‡w13736978

  $w can be anything for this use case or another $w from your catalog.
  Easiest might be to enable the Advanced cataloguing editor to be
  able to insert these lines easily. If you are using the sample data,
  the example shoudl work as is.

* Save the record and open it in the staff interface and the OPAC
* The link in the OPAC should be broken, staff should work
* Apply patch
* Now both links should work

Co-authored-by: Anke Bruns <Anke.Bruns@gwdg.de>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Tomás Cohen Arazi 2023-03-30 10:14:47 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 23 Matt Blenkinsop 2023-04-11 11:12:16 UTC
Nice work everyone!

Pushed to stable for 22.11.x