Bug 6465 - Errors in UNIMARC plugins for fixed length fields (for | and space) (T::T issue)
Summary: Errors in UNIMARC plugins for fixed length fields (for | and space) (T::T issue)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: PATCH-Sent (DO NOT USE) major (vote)
Assignee: Janusz Kaczmarek
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks: 5917
  Show dependency treegraph
 
Reported: 2011-06-05 21:36 UTC by Janusz Kaczmarek
Modified: 2013-12-05 20:05 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
A screenshot illustrating the problem (59.37 KB, image/png)
2011-06-05 21:36 UTC, Janusz Kaczmarek
Details
Proposed patch (55.05 KB, patch)
2011-06-05 21:56 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Proposed patch (55.06 KB, patch)
2011-06-05 22:10 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Proposed patch (corrected) (57.07 KB, patch)
2011-06-06 11:54 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Proposed patch (corrected) (57.15 KB, patch)
2011-06-18 11:50 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2011-06-05 21:36:31 UTC
Created attachment 4369 [details]
A screenshot illustrating the problem

When editing, with a plugin, a fixed length field of an existing (or new) record, some characters are changed when invoking the plugin (i.e. are not correctly passed to the plugin window). (The initial values for these fields set in .pl in variable $result are not respected as well).

The attached screenshot illustrates the problem.

This is because after switching to T::T variable names like:

<!-- TMPL_IF name="f5|" -->
or
<!-- TMPL_IF name="f8 " -->

(which were used in most of templates in /value_builder) are not allowed. In translation process these constructs have been replaced with 

[% IF ( f5pipe ) %] 
and
[% IF ( f8 ) %]

The corresponding *.pl files remain unchanged and provide the elements in the result hash as follows:

    my $f5  = substr( $result, 5,  1 );
[...]
    $template->param(
[...]
        "f5$f5"   => 1,

So, if there is a pipe (or a space), the has key will be "f5|" ("f5 " resp.), which worked well with HTML::Template, but not with T::T.

The same applies most probably to MARC-21.
Comment 1 Janusz Kaczmarek 2011-06-05 21:56:13 UTC Comment hidden (obsolete)
Comment 2 Janusz Kaczmarek 2011-06-05 22:01:48 UTC
The patch proposed make use of the idea of "f5pipe" in .tt files and adds also things like "f5space".  (Sometimes it will be also dblpipe and dblspace.) 

Now a wrapper was needed in .pl to give the right things to the template param hash. The wrapper was added where needed.

This id done for UNIMARC only.  Most probably analogous things are to be done for MARC-21.  Or perhaps anybody has a better idea...?
Comment 3 Janusz Kaczmarek 2011-06-05 22:10:12 UTC Comment hidden (obsolete)
Comment 4 Janusz Kaczmarek 2011-06-06 11:54:12 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2011-06-15 13:18:51 UTC
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y
Applying: Errata to Bugfix 6465 - Errors in UNIMARC plugins for fixed length fields (for | and space) (T::T issue)
fatal: sha1 information is lacking or useless (cataloguing/value_builder/unimarc_field_100.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Errata to Bugfix 6465 - Errors in UNIMARC plugins for fixed length fields (for | and space) (T::T issue)
Comment 6 Janusz Kaczmarek 2011-06-18 11:50:29 UTC
Created attachment 4502 [details] [review]
Proposed patch (corrected)

Katrin,

I am sorry for the trouble -- this is because I have generated the patch from my, slightly modified, version.  Now it should apply smoothly, I hope.
Comment 7 Katrin Fischer 2011-07-17 13:32:59 UTC
I can confirm patch applies now to current master. I had problems reproducing the error, someone more familiar with UNIMARC should test this.
Comment 8 Julian Maurice 2011-07-22 09:07:09 UTC
I met something odd and I don't know if it's a bug or not.
In 100$a I have "19970701d1848    m  y0frey50      ba"
But for marked characters:         ^^
I have "|- Filling character" in plugin.
Is this a normal behaviour?
Comment 9 Janusz Kaczmarek 2011-07-22 10:15:44 UTC
Julian, if I understand things well, what you are referring to has no relation to my patch nor with the problem described by me.  

The positions you are pointing at (i.e. 100/17-19) are Target Audience Code.  According to my knowledge about field 100 (http://archive.ifla.org/VI/3/p1996-1/uni1.htm#100) spaces are not allowed there at all--one should choose from among [a b c d e k m u].  And "if some data in a field is supplied but not all, the omitted data element positions will contain fill characters" which is |.

This is why having spaces in 100/17-19 you get | in the plugin, both having my patch applied or not. 


But if you wanted to see what I am referring to in my patch look at positions 17 and 18 in Unimarc record label for example, where spaces are allowed.
Comment 10 Julian Maurice 2011-08-05 07:56:38 UTC
Ok, I finally understood the problem, and this patch fix it. Signed off.
Something similar have to be done with marc21 plugins.
Comment 11 Ian Walls 2011-09-26 00:46:22 UTC
Corrects plugins to use 'wrapper' subroutine.  Subroutine defined consistently on all plugins, and the necessary params are updated in the templates.  

Unfortunately, I am unfamiliar with UNIMARC, and cannot provide any feedback on the actual functionality of this patch.  However, the code appears sounds.  I'm trusting the signoff and marking as Passed QA.
Comment 12 Chris Cormack 2011-09-26 01:37:16 UTC
As with Ian I am trusting that the UNIMARC functionality works ok, please test thoroughly.

Scripts all pass syntax checks. Pushed
Comment 13 Jared Camins-Esakov 2012-12-31 00:26:52 UTC
There have been no further reports of problems so I am marking this bug resolved.