Bug 9497

Summary: Make it easier to add to the the list of possible encodings for Z39.50 servers and add ISO 8859-1
Product: Koha Reporter: Magnus Enger <magnus>
Component: CatalogingAssignee: Magnus Enger <magnus>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P5 - low CC: bgkriegel, chris, jcamins, m.de.rooy, mtj
Version: master   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 9498    
Attachments: Bug 9497 - Make it easier to add new encodings for Z39.50 servers and add ISO 8859-1
Bug 9497 - Make it easier to add new encodings for Z39.50 servers and add ISO 8859-1
Bug 9497 - Make it easier to add new encodings for Z39.50 servers and add ISO 8859-1

Description Magnus Enger 2013-01-27 12:23:19 UTC
At least the Norwegian BIBSYS Z39.50 server delivers records in ISO 8859-1, but that is not an option when you edit Z39.50 servers. Patch coming in a moment!
Comment 1 Magnus Enger 2013-01-27 12:49:00 UTC
*** Bug 8081 has been marked as a duplicate of this bug. ***
Comment 2 Magnus Enger 2013-01-27 12:50:13 UTC
Broadening the scope of this bug a bit.
Comment 3 Magnus Enger 2013-01-27 12:58:21 UTC
Problems:

This code in admin/z3950servers.pl was creating separate variables for each encoding:

$template->param( $_ . $data->{$_} => 1)
        for ( qw( syntax encoding ) );

And the corresponding code in the template had to check each of these variables to decide which one should be selected:

[% IF ( encodingutf8 ) %]
<option value="utf8" selected="selected">UTF-8</option>
[% ELSE %]
<option value="utf8">UTF-8</option>
[% END %]
[% IF ( encodingEUC_KR ) %]
<option value="EUC-KR" selected="selected">EUC-KR</option>
[% ELSE %]
<option value="EUC-KR">EUC-KR</option>
[% END %]
etc. 

This has a couple of problems:

- Encodings with a hyphen ("-") in them create invalid variable names in TT, as seen in Bug 8081
- Adding a new encoding means adding 5 lines to the template

My proposed solution creates just one variable for the encoding and sets it to the name of the encoding, thus avoiding the hyphen problem. It also generates the options from a list of encodings (defined in the template), which means that future encodings can just be added to the list.
Comment 4 Magnus Enger 2013-01-27 13:04:56 UTC Comment hidden (obsolete)
Comment 5 Bernardo Gonzalez Kriegel 2013-01-27 13:59:05 UTC Comment hidden (obsolete)
Comment 6 Mason James 2013-01-30 06:40:12 UTC
Created attachment 14933 [details] [review]
Bug 9497 - Make it easier to add new encodings for Z39.50 servers and add ISO 8859-1

This patch does three things:
- Makes admin/z3950servers.pl create one variable for encoding,
  not one variable for each encoding
- Makes the template create encoding options from a list
- Adds ISO 8859-1 to the list of available encodings
See the bug for a lengthier description

To test:
- Apply the patch
- Edit one Z39.50 server several times, choosing each available encoding in turn
- Check that for each encoding, the correct encoding is shown both in the list of
  Z39.50 servers and in the dropdown on the edit screen

Comment: Works as described, no errors.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Comment 7 Mason James 2013-01-30 06:42:10 UTC
(In reply to comment #6)
> Created attachment 14933 [details] [review]
> Bug 9497 - Make it easier to add new encodings for Z39.50 servers and add
> ISO 8859-1
> 
> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
> Signed-off-by: Mason James <mtj@kohaaloha.com>

testing 1 commit(s) (applied to f8686e9 'Bug 9116: Remove some useless tmp var')

passing QA, works well :)
Comment 8 Jared Camins-Esakov 2013-01-31 15:57:21 UTC
This patch has been pushed to master.
Comment 9 Chris Cormack 2013-02-01 08:53:35 UTC
Pushed to 3.10.x, will be in 3.10.3
Comment 10 Magnus Enger 2013-04-03 19:07:11 UTC
Works as expected (tested with packages built off current master).