Bug 10691

Summary: 5xx not properly linked by authid in authority search result list
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: MARC Authority data supportAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: normal    
Priority: P5 - low CC: bgkriegel, fridolin.somers, gitbot, gmcharlt, jcamins, jonathan.druart, kyle, tomascohen
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10948
Change sponsored?: Sponsored Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 10948    
Bug Blocks: 10987    
Attachments: Bug 10691: 5xx not properly linked by authid in authority search result list
Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]
Bug 10691: 5xx not properly linked by authid in authority search result list
Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]
Bug 10691: 5xx not properly linked by authid in authority search result list
Bug 10691: Small glitch fixed
[SIGNED-OFF] Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]
[SIGNED-OFF] Bug 10691: 5xx not properly linked by authid in authority search result list
[SIGNED-OFF] Bug 10691: Small glitch fixed
Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]
Bug 10691: 5xx not properly linked by authid in authority search result list
Bug 10691: 5xx not properly linked by authid in authority search result list [staff]
Bug 10691: 5xx not properly linked by authid in authority search result list
Bug 10691: Small glitch fixed
Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]
Bug 10691: 5xx not properly linked by authid in authority search result list
Bug 10691: Small glitch fixed
Bug 10691: (follow-up) restore behaviour for control field ($w)
Bug 10691: (follow-up) restore behaviour for control field ($w)
Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]
Bug 10691: 5xx not properly linked by authid in authority search result list
Bug 10691: (follow-up) restore behaviour for control field ($w)
Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]
Bug 10691: 5xx not properly linked by authid in authority search result list
Bug 10691: (follow-up) restore behaviour for control field ($w)
Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]
Bug 10691: 5xx not properly linked by authid in authority search result list
Bug 10691: (follow-up) restore behaviour for control field ($w)

Description Katrin Fischer 2013-08-07 12:13:06 UTC
Authority search in OPAC does not use the authid for creating the link in the result list, but it does on the detail page.

Example:
https://eh-freiburg.bsz-bw.de/cgi-bin/koha/opac-authorities-home.pl?op=do_search&type=opac&authtypecode=PERSO_NAME&operator=start&value=Schmidt&marclist=mainentry&and_or=and&orderby=HeadingAsc

Heading: Schmidt, Andrea
Link: Betriebswirtin
Comment 1 Tomás Cohen Arazi 2013-09-17 18:33:20 UTC
(In reply to Katrin Fischer from comment #0)
> Authority search in OPAC does not use the authid for creating the link in
> the result list, but it does on the detail page.
> 
> Example:
> https://eh-freiburg.bsz-bw.de/cgi-bin/koha/opac-authorities-home.
> pl?op=do_search&type=opac&authtypecode=PERSO_NAME&operator=start&value=Schmid
> t&marclist=mainentry&and_or=and&orderby=HeadingAsc

It looks like "View full heading" points to the correct authid to me. Is it fixed by you there or am I not understanding the problem you describe?
Comment 2 Katrin Fischer 2013-09-18 07:53:44 UTC
Sorry for being confusing :) The link in the result list searches for the string, the link on the authority detail page uses the authid. Both should be using the authid.
Comment 3 Tomás Cohen Arazi 2013-09-18 14:46:16 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2013-09-18 15:20:38 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2013-09-18 20:29:16 UTC
Even though both patches make the results look as expected, such is not the case for records that link records with no RLIN but only plain text. In such cases a search link should be created.
Comment 6 Tomás Cohen Arazi 2013-10-02 14:58:46 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2013-10-02 14:59:11 UTC Comment hidden (obsolete)
Comment 8 Tomás Cohen Arazi 2013-10-02 15:39:54 UTC Comment hidden (obsolete)
Comment 9 Tomás Cohen Arazi 2013-10-02 15:52:19 UTC Comment hidden (obsolete)
Comment 10 I'm just a bot 2013-10-03 07:02:03 UTC
Patch applied cleanly, go forth and signoff
Comment 11 Bernardo Gonzalez Kriegel 2013-10-09 11:59:55 UTC Comment hidden (obsolete)
Comment 12 Bernardo Gonzalez Kriegel 2013-10-09 12:00:11 UTC Comment hidden (obsolete)
Comment 13 Bernardo Gonzalez Kriegel 2013-10-09 12:03:03 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2013-10-21 21:28:12 UTC
Hi Bernardo,
failed to fix the conflict correctly I think - could you take a look and rebase?
Comment 15 Tomás Cohen Arazi 2013-10-21 22:37:34 UTC Comment hidden (obsolete)
Comment 16 Tomás Cohen Arazi 2013-10-21 22:40:39 UTC Comment hidden (obsolete)
Comment 17 Tomás Cohen Arazi 2013-10-21 22:41:13 UTC
Comment on attachment 22211 [details] [review]
Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]

>From 7d62376b2e76dbed2fc45921b0a4fcbed6b9f496 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@gmail.com>
>Date: Wed, 18 Sep 2013 12:18:20 -0300
>Subject: [PATCH] Bug 10691: 5xx not properly linked by authid in authority
> search result list [Staff]
>
>This patch changes the URL and data used to show the 'see also' links
>on the Staff's authority search results page.
>
>Bonus points: makes some strings translatable.
>
>To test:
>- On your dev setup (master) create some authority records (i created personal name authorities).
>- Pick one of them and link 400$a to another one, do the same with 500$a
>- Add some other 400$a and 500$a entries with plain text (i.e. no linking)
>- Make sure zebra is running and changes got indexed.
>- In the staff interface search for the authority that is linked to the others.
>- Check the 'see also:' link points to an authority search
>- Apply the patch
>- Reload/re-do the search
>- Check the 'see also:' link points to the authority id for linked authorities,
>  and to an authority search result in the case of plain text entries.
>- Check that the authority search from the cataloguing interface still works as usual.
>
>Regards
>To+
>
>Sponsored-by: Universidad Nacional de Cordoba
>Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
>
>Work as described. No errors
>---
> .../en/includes/authorities-search-results.inc     | 96 ++++++++++++++--------
> .../en/modules/authorities/searchresultlist.tt     |  2 +-
> 2 files changed, 63 insertions(+), 35 deletions(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/authorities-search-results.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/authorities-search-results.inc
>index 2ee4571..616bb1b 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/includes/authorities-search-results.inc
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/authorities-search-results.inc
>@@ -1,4 +1,7 @@
> [% BLOCK showreference %]
>+    [% SET authidurl = '/cgi-bin/koha/authorities/detail.pl?authid=' %]
>+    [% SET searchurl = '/cgi-bin/koha/authorities/authorities-home.pl?op=do_search&type=intranet&marclist=any&operator=contains&orderby=HeadingAsc&value=' %]
>+
>     [% IF marcflavour == 'UNIMARC' %]
>         [% SWITCH type %]
>         [% CASE 'broader' %]
>@@ -11,30 +14,48 @@
>             <span class="RT">RT: [% heading | html %]</span>
>         [% END %]
>     [% ELSE %]
>-        [% IF ( label ) %]<span class="label">[% label | html %]</span>[% END %]
>+        <span class="label">
>+        [% IF ( type=='seefrom' ) %]
>+            used for/see from:
>+        [% ELSIF  ( type=='seealso' ) %]
>+            see also:
>+        [% END %]
>+        </span>
>         <span class="heading">
>-        [% IF ( linkpath && search ) %]<a href="[% link | url %][% search | url %]">[% heading | html %]</a>
>-        [% ELSE %][% heading | html %][% END %]
>+        [% IF ( type=='seealso' ) %]
>+            [% IF ( authid ) %]
>+            <a href="[% authidurl | url %][% authid | url %]">[% heading | html %]</a>
>+            [% ELSE %]
>+            <a href="[% searchurl | url %][% heading | html %]">[% heading | html %]</a>
>+            [% END %]
>+        [% ELSIF  ( type=='seealso' ) %]
>+            [% heading | html %]
>+        [% END %]
>         </span>
>-        [% UNLESS ( type=='seefrom' || type=='seealso' ) %]<span class="type">[% SWITCH type %]
>-        [% CASE 'earlier' %](Earlier heading)
>-        [% CASE 'later' %](Later heading)
>-        [% CASE 'acronym' %](Acronym)
>-        [% CASE 'musical' %](Musical composition)
>-        [% CASE 'broader' %](Broader heading)
>-        [% CASE 'narrower' %](Narrower heading)
>-        [% CASE 'parent' %](Immediate parent body)
>-        [% CASE %][% IF type %]([% type | html %])[% END %]
>-        [% END %]</span>[% END %]
>+        [% UNLESS ( type=='seefrom' || type=='seealso' ) %]
>+        <span class="type">
>+            [% SWITCH type %]
>+            [% CASE 'earlier' %](Earlier heading)
>+            [% CASE 'later' %](Later heading)
>+            [% CASE 'acronym' %](Acronym)
>+            [% CASE 'musical' %](Musical composition)
>+            [% CASE 'broader' %](Broader heading)
>+            [% CASE 'narrower' %](Narrower heading)
>+            [% CASE 'parent' %](Immediate parent body)
>+            [% CASE %][% IF type %]([% type | html %])
>+            [% END %]
>+        [% END %]
>+        </span>
>+        [% END %]
>     [% END %]
> [% END %]
> [% BLOCK authresult %]
>     [% IF ( summary.summary ) %][% summary.summary | html %]:[% END %]
>     [% UNLESS ( summary.summaryonly ) %]
>         <div class="authorizedheading">
>-        [% FOREACH authorize IN summary.authorized %]
>-          <span class="authorizedheading">[% authorize.heading | html %]</span>
>-        [% END %]
>+          [% FOREACH authorize IN summary.authorized %]
>+            <span class="authorizedheading">[% authorize.heading | html %]</span>
>+          [% END %]
>         </div>
>         [% IF ( marcflavour == 'UNIMARC' ) %]
>           [% IF summary.notes %]
>@@ -52,36 +73,43 @@
>             [% END %]
>             </div>
>           [% END %]
>-          [% IF summary.seealso %]
>-            <div class="authres_seealso">
>-            [% FOREACH see IN summary.seealso %]
>-              [% PROCESS showreference heading=see.heading label="" type=see.type search='' %]
>-              [% IF ! loop.last %] ; [% END %]
>+            [% IF summary.seealso %]
>+              <div class="authres_seealso">
>+              [% FOREACH see IN summary.seealso %]
>+                 [% PROCESS showreference heading=see.heading label="" type=see.type search='' %]
>+                 [% IF ! loop.last %] ; [% END %]
>+              [% END %]
>+              </div>
>             [% END %]
>-            </div>
>-          [% END %]
>-          [% IF summary.otherscript %]
>-            <div class="authres_otherscript">
>-            [% FOREACH other IN summary.otherscript %]
>-              [% PROCESS language lang=other.lang | trim %]:
>-              [% other.term %]
>-              [% IF ! loop.last %] ; [% END %]
>+            [% IF summary.otherscript %]
>+              <div class="authres_otherscript">
>+              [% FOREACH other IN summary.otherscript %]
>+                [% PROCESS language lang=other.lang | trim %]:
>+                [% other.term %]
>+                [% IF ! loop.last %] ; [% END %]
>+              [% END %]
>+              </div>
>             [% END %]
>-            </div>
>-          [% END %]
>-
>         [% ELSE %]
>             [% IF ( summary.seefrom ) %]
>                 [% FOREACH seefro IN summary.seefrom %]
>                     <div class="seefrom authref">
>-                    [% PROCESS showreference heading=seefro.heading label="used for/see from:" type=seefro.type search='' %]
>+                    [% PROCESS showreference
>+                        heading=seefro.heading
>+                        type=seefro.type
>+                        authid=seefro.authid
>+                    %]
>                     </div>
>                 [% END %]
>             [% END %]
>             [% IF ( summary.seealso ) %]
>                 [% FOREACH seeals IN summary.seealso %]
>                     <div class="seealso authref">
>-                    [% PROCESS showreference heading=seeals.heading label="see also:" type=seeals.type linkpath=link search=seeals.search %]
>+                    [% PROCESS showreference
>+                        heading=seeals.heading
>+                        type=seeals.type
>+                        authid=seeals.authid
>+                    %]
>                     </div>
>                 [% END %]
>             [% END %]
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/searchresultlist.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/searchresultlist.tt
>index f73b819..49ba49f 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/searchresultlist.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/searchresultlist.tt
>@@ -80,7 +80,7 @@ function searchauthority() {
>     [% ELSE %]
>     <tr data-authid="[% resul.authid %]">
>     [% END %]
>-      <td>[% PROCESS authresult summary=resul.summary link="/cgi-bin/koha/authorities/authorities-home.pl?op=do_search&type=intranet&marclist=any&operator=contains&orderby=HeadingAsc&value=" %]</td>
>+      <td>[% PROCESS authresult summary=resul.summary %]</td>
>       <td><a href="detail.pl?authid=[% resul.authid %]">Details</a></td>
>   [% UNLESS ( resul.isEDITORS ) %]
>       <td>
>-- 
>1.8.3.2
Comment 18 Tomás Cohen Arazi 2013-10-21 22:43:51 UTC Comment hidden (obsolete)
Comment 19 Tomás Cohen Arazi 2013-10-21 22:44:08 UTC Comment hidden (obsolete)
Comment 20 Tomás Cohen Arazi 2013-10-21 22:44:22 UTC Comment hidden (obsolete)
Comment 21 Katrin Fischer 2013-10-27 08:23:16 UTC
Tomas, I am sorry, but now no links show up at all for my data - after applying the patch and refreshing the authority result page, all links are gone.
Comment 22 Jonathan Druart 2013-11-07 16:10:06 UTC
QA comment:
I tested on a sandbox and did not find the link issue.
Links for "see also" have the expected behavior.
Marked as Passed QA.
Comment 23 Jonathan Druart 2013-11-07 16:10:32 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2013-11-07 16:10:37 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2013-11-07 16:10:42 UTC Comment hidden (obsolete)
Comment 26 Jonathan Druart 2013-11-07 16:16:58 UTC
Tomás, you will win a bonus point if you provide a patch for the bootstrap theme :)
Comment 27 Jared Camins-Esakov 2013-12-11 03:16:50 UTC
Unfortunately this breaks the authority results display when 5xx$w is set: http://screencast.com/t/J40kB4bJ8
Comment 28 Tomás Cohen Arazi 2013-12-11 14:23:00 UTC Comment hidden (obsolete)
Comment 29 Tomás Cohen Arazi 2013-12-11 14:50:14 UTC Comment hidden (obsolete)
Comment 30 Galen Charlton 2013-12-13 15:38:58 UTC
I've pushed the patch for bug 11174, which means that an (easy) rebase is in order for this patch series.  In the process, I suggest that you may as well clean up the remaining 'c' parameter names, e.g., and_orc, operatorc, and so forth, as none of them will work.
Comment 31 Tomás Cohen Arazi 2013-12-13 17:53:38 UTC Comment hidden (obsolete)
Comment 32 Tomás Cohen Arazi 2013-12-13 17:53:58 UTC Comment hidden (obsolete)
Comment 33 Tomás Cohen Arazi 2013-12-13 17:54:21 UTC Comment hidden (obsolete)
Comment 34 Jared Camins-Esakov 2014-01-13 18:45:54 UTC Comment hidden (obsolete)
Comment 35 Jared Camins-Esakov 2014-01-13 18:46:28 UTC Comment hidden (obsolete)
Comment 36 Jared Camins-Esakov 2014-01-13 18:46:55 UTC Comment hidden (obsolete)
Comment 37 Jonathan Druart 2014-01-16 15:56:20 UTC
Still good to me.
No regression found since my last tests.
Marked as Passed QA.
Comment 38 Jonathan Druart 2014-01-16 15:58:18 UTC
Created attachment 24461 [details] [review]
Bug 10691: 5xx not properly linked by authid in authority search result list [Staff]

This patch changes the URL and data used to show the 'see also' links
on the Staff's authority search results page.

Bonus points: makes some strings translatable.

To test:
- On your dev setup (master) create some authority records (i created personal name authorities).
- Pick one of them and link 400$a to another one, do the same with 500$a
- Add some other 400$a and 500$a entries with plain text (i.e. no linking)
- Make sure zebra is running and changes got indexed.
- In the staff interface search for the authority that is linked to the others.
- Check the 'see also:' link points to an authority search
- Apply the patch
- Reload/re-do the search
- Check the 'see also:' link points to the authority id for linked authorities,
  and to an authority search result in the case of plain text entries.
- Check that the authority search from the cataloguing interface still works as usual.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Work as described. No errors

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 39 Jonathan Druart 2014-01-16 15:58:25 UTC
Created attachment 24462 [details] [review]
Bug 10691: 5xx not properly linked by authid in authority search result list

This patch changes the URL and data used to show the 'see also' links
on the OPAC's authority search results page.

Bonus points: makes some strings translatable.

To test:
- On your dev setup (master) create some authority records (i created personal name authorities).
- Pick one of them and link 400$a to another one, do the same with 500$a
- Add some other 400$a and 500$a without linking (i.e. plain text)
- Make sure zebra is running and changes got indexed.
- In the OPAC search for the authority that is linked to the others.
- Check the 'see also:' link points to an authority search
- Apply the patch
- Reload/re-do the search
- Check the 'see also:' link points to the authority id in the case of linked authorities,
  and to an authority search in the case of plain text names.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Work as described. No errors

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 40 Jonathan Druart 2014-01-16 15:58:31 UTC
Created attachment 24463 [details] [review]
Bug 10691: (follow-up) restore behaviour for control field ($w)

As noted by Jared, on the presence of 4XX$w or 5XX$w the display got broken
by this refactoring. This patch restores de previous behaviour in that front.

The OPAC themes get fixed too, and the Bootstrap one gets this fix too.

Applied the fix that Galen proposed on comment #30 regarding Zeno's fix in bug 11174.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 41 Galen Charlton 2014-01-16 23:09:57 UTC
Pushed to master, along with a follow-up that changes the HTML comments to TT comments.

Thanks, Tomás!
Comment 42 Fridolin Somers 2014-04-23 15:59:53 UTC
Patches pushed to 3.14.x, will be in 3.14.06
Comment 43 Kyle M Hall 2014-06-06 13:47:37 UTC
Pushed to 3.12.x will be in 3.12.13