Bug 7943

Summary: Untranslatable strings in OPAC's authority search
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: I18N/L10NAssignee: Jared Camins-Esakov <jcamins>
Status: CLOSED FIXED QA Contact: Ian Walls <koha.sekjal>
Severity: normal    
Priority: P5 - low CC: chris, m.de.rooy, paul.poulain
Version: 3.8   
Hardware: All   
OS: All   
URL: cgi-bin/koha/opac-authorities-home.pl?...
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: screenshot
Bug 7943: Authority search results are untranslatable
Bug 7943 follow-up: Fix template filename
[SIGNED-OFF] Bug 7943: Authority search results are untranslatable
[SIGNED-OFF] Bug 7943 follow-up: Fix template filename
[3.8.x] Bug 7943: Authority search results are untranslatable
[3.8.x] Bug 7943 follow-up: Fix template filename

Description Katrin Fischer 2012-04-11 08:43:43 UTC
Created attachment 9111 [details]
screenshot

used for/see from: is not translatable as is the "Default" name for the default framework.

I think the problem is the way the display is built:
http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/AuthoritiesMarc.pm#l1094

Another issue with the display is, that all authorities show up as Default, while they are correct in the database.
Comment 1 Jared Camins-Esakov 2012-06-13 22:00:45 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2012-06-15 13:25:27 UTC
Katrin pointed out that I should know for QA purposes that I did not add the AuthDisplayHierarchy syspref, nor, indeed, modify the UNIMARC hierarchy code in any meaningful way. The fact that the syspref is not included in default installations is, I presume, intentional, though I have no idea why we wouldn't include it in all installations.
Comment 3 Jared Camins-Esakov 2012-06-15 13:26:58 UTC
(In reply to comment #2)
> Katrin pointed out that I should know for QA purposes that I did not add the
> AuthDisplayHierarchy syspref, nor, indeed, modify the UNIMARC hierarchy code
> in any meaningful way. The fact that the syspref is not included in default
> installations is, I presume, intentional, though I have no idea why we
> wouldn't include it in all installations.

That I should let *you*, unidentified QAer, know. Time for some coffee.
Comment 4 Jared Camins-Esakov 2012-06-15 14:06:24 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2012-06-15 14:14:58 UTC
Created attachment 10292 [details] [review]
[SIGNED-OFF] Bug 7943: Authority search results are untranslatable

The HTML for authority search results was previously generated in
C4::AuthoritiesMarc::BuildSummary, which meant that it couldn't be
translated. This patch moves the HTML generation into the templates
by introducing a new authorities-search-results.inc include file for
both the OPAC and the Intranet which contains a Template::Toolkit BLOCK
for rendering the authority results. Fixes the authority autocomplete
by removing the untranslatable strings, and returning only data from
the database.

To test:
1. Apply patch.
2. Test authority searching in the authority module in the staff client
3. Test authority searching in the authority control plugin in the
   cataloguing module (and the plugin for UNIMARC field 210$c, if you
   can figure out how)
4. Test authority searching in the OPAC

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested with MARC21 data.

1) Applies cleanly on current HEAD.

2) Authority search in staff

Patch works wonderfully, only some small notes found while testing that have
not been changed by this patch:

ENH note: Search terms show up nowhere. So if I want to change the sorting, I have to
repeat typing in my search term. Even if the form does not keep the term, it
should be visible somewhere on the screen what I searched for.

ENH note: The pagination on top and at the bottom of the result list are formatted
differently. Maybe some missing CSS?

ENH note: Also the authority type is not shown at all in the result list.

3) Cataloguing and authority plugins

The autocomplete function works nicely.

ENH note: There is only one small enhancement
I could imagine. If I start my search from 100 it will limit the search to
'persons' but the autocomplete will also suggest other authorities. It would
be a bit cleaner, if the autocomplete could limit by the appropriate authority
type too. Very nice feature.

Plugins overall work nicely. Created links include the authority numbers and work
correctly.

4) Authority search in OPAC

Works nicely. Display is consistent, but translatability greatly improved.

ENH note: In staff we use 'Details' in OPAC we use 'View full heading' - I wonder
if maybe 'details' would be better understandable for users?

Note: Code reveals a system preference 'AuthDisplayHierarchy' that is
not available in the system preference editor. I talked to Jared and he
will work on this feature later on. For now it's no regression, as the
preference has never been visible.
Comment 6 Katrin Fischer 2012-06-15 14:16:45 UTC
Created attachment 10293 [details] [review]
[SIGNED-OFF] Bug 7943 follow-up: Fix template filename

In preparation for some later work on authorities, the filename for the
OPAC authority viewer changed.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 7 Ian Walls 2012-06-15 15:06:37 UTC
Code is cleanly written and well documented.  Followup is simple, providing a small consistency change to enable further work.  New template block is translatable, which is the ultimate goal.

More followup may be required to address Katrin's enhancement requests, and as other work comes in, but this as it stands Passes QA.
Comment 8 Paul Poulain 2012-06-20 08:08:47 UTC
Patch pushed, should not be pushed immediately in 3.8, as it contains some string changes, and we're string freeze for 3.8.2
Also worth having a longer testing period, as it's a large patch that could introduce some hidden side effect
Comment 9 Chris Cormack 2012-06-22 01:56:18 UTC
Patch does not apply for 3.8.x 

Here are the offending files

#	both modified:      authorities/ysearch.pl
#	both modified:      koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css
#	both modified:      koha-tmpl/opac-tmpl/prog/en/css/opac.css

Please fix and resubmit for inclusion into 3.8.x
Comment 10 Jared Camins-Esakov 2012-06-22 02:52:16 UTC
Created attachment 10420 [details] [review]
[3.8.x] Bug 7943: Authority search results are untranslatable

The HTML for authority search results was previously generated in
C4::AuthoritiesMarc::BuildSummary, which meant that it couldn't be
translated. This patch moves the HTML generation into the templates
by introducing a new authorities-search-results.inc include file for
both the OPAC and the Intranet which contains a Template::Toolkit BLOCK
for rendering the authority results. Fixes the authority autocomplete
by removing the untranslatable strings, and returning only data from
the database.

To test:
1. Apply patch.
2. Test authority searching in the authority module in the staff client
3. Test authority searching in the authority control plugin in the
   cataloguing module (and the plugin for UNIMARC field 210$c, if you
   can figure out how)
4. Test authority searching in the OPAC

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested with MARC21 data.

1) Applies cleanly on current HEAD.

2) Authority search in staff

Patch works wonderfully, only some small notes found while testing that have
not been changed by this patch:

ENH note: Search terms show up nowhere. So if I want to change the sorting, I have to
repeat typing in my search term. Even if the form does not keep the term, it
should be visible somewhere on the screen what I searched for.

ENH note: The pagination on top and at the bottom of the result list are formatted
differently. Maybe some missing CSS?

ENH note: Also the authority type is not shown at all in the result list.

3) Cataloguing and authority plugins

The autocomplete function works nicely.

ENH note: There is only one small enhancement
I could imagine. If I start my search from 100 it will limit the search to
'persons' but the autocomplete will also suggest other authorities. It would
be a bit cleaner, if the autocomplete could limit by the appropriate authority
type too. Very nice feature.

Plugins overall work nicely. Created links include the authority numbers and work
correctly.

4) Authority search in OPAC

Works nicely. Display is consistent, but translatability greatly improved.

ENH note: In staff we use 'Details' in OPAC we use 'View full heading' - I wonder
if maybe 'details' would be better understandable for users?

Note: Code reveals a system preference 'AuthDisplayHierarchy' that is
not available in the system preference editor. I talked to Jared and he
will work on this feature later on. For now it's no regression, as the
preference has never been visible.

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Rebased for 3.8.x on 21 June 2012.
Comment 11 Jared Camins-Esakov 2012-06-22 03:00:31 UTC
Created attachment 10421 [details] [review]
[3.8.x] Bug 7943 follow-up: Fix template filename

In preparation for some later work on authorities, the filename for the
OPAC authority viewer changed.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 12 Chris Cormack 2012-07-26 21:53:53 UTC
Thank you very much for the 3.8.x based patches, applied cleanly passed all functional, and unit tests.

Pushed to the 3.8.x will be in 3.8.4
Comment 13 Paul Poulain 2012-08-02 13:31:43 UTC
I think (although i'm not sure it's the culprit) that this patch broke the display of authorities search, I see "
 HASH(0xb4c3d88) " instead of the summary of the authority

it's a blocker !

Side question = is it unimarc specific or no ?
Comment 14 Marcel de Rooy 2012-08-02 13:38:06 UTC
(In reply to comment #13)
> I think (although i'm not sure it's the culprit) that this patch broke the
> display of authorities search, I see "
>  HASH(0xb4c3d88) " instead of the summary of the authority
> 
> it's a blocker !
> 
> Side question = is it unimarc specific or no ?

Please have a look at the patch in the Passed QA queue for staff display!
Comment 15 Paul Poulain 2012-08-02 13:47:32 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > I think (although i'm not sure it's the culprit) that this patch broke the
> > display of authorities search, I see "
> >  HASH(0xb4c3d88) " instead of the summary of the authority
> > 
> > it's a blocker !
> > 
> > Side question = is it unimarc specific or no ?
> 
> Please have a look at the patch in the Passed QA queue for staff display!

yep. Jared pointed it on IRC. It passed QA between this morning and now, that's why I missed it. Thanks though
Comment 16 Chris Cormack 2012-08-24 02:20:55 UTC
All patches are in 3.8.x and Master and working well