[Koha-bugs] [Bug 11319] Marc modification templates improvements

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11319

M. de Rooy <m.de.rooy at rijksmuseum.nl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Failed QA

--- Comment #35 from M. de Rooy <m.de.rooy at rijksmuseum.nl> ---
QA Comment: 
In general these patches look good to me. I have the impression that this
feature still needs some work however.
Here are some comments to start with:

[1] The last patch (template mods) adds the restriction: A move/copy/update
should have a subfield. If I want to update control field 003 to some text, it
should be possible. There is no subfield. (Blocker.) 
It also says: move/copy source and dest should have either a subfield or not. I
agree that it is normal behavior. But suppose that I want to copy some field to
a controlfield? This is somewhat unlikely, but it would not be wrong. (No
blocker since I do not have a real application for it at hand.) 

[2] Some code in the third patch for update_field: 
+    if ( not $subfieldName or $subfieldName eq '' ) {
+        # FIXME I'm not sure the actual implementation is correct.
+        die "This action is not implemented yet";
+        #_update_field({ record => $record, field => $fieldName, values =>
\@values });
This actually is the situation I was referring to (I want to update a control
field without subfield).
At this moment you are blocking an update without subfield in the interface.
This makes it hard to reach this die statement..

[3] Just looking at some code in SimpleMarc.pm, I notice the following:
field_equals just picks the first field value and runs the regex. If I do not
pass a subfield, it does not run the regex over the whole field just one
subfield. If I pass a subfield and the record contains repeated subfields, it
just checks the first one. This is not the behavior that I would (intuitively)
expect from the interface. Note that your patch does not really touch that part
of code, but you do touch the routine. Is improving that functionality not even
more important than passing a hashref? 

[4] BTW See some warnings in the log:
[Wed Jul 30 16:37:51 2014] stage-marc-import.pl: Filehandle STDOUT reopened as
FH only for input at Template/Provider.pm line 964., referer:
/cgi-bin/koha/tools/stage-marc-import.pl
[Wed Jul 30 16:37:51 2014] stage-marc-import.pl: Filehandle STDOUT reopened as
FH only for input at Template/Provider.pm line 964., referer:
/cgi-bin/koha/tools/stage-marc-import.pl
[Wed Jul 30 16:53:18 2014] marc_modification_templates.pl: Use of uninitialized
value in string eq at tools/marc_modification_templates.pl line 70., referer:
/cgi-bin/koha/tools/marc_modification_templates.pl?template_id=3&op=delete_action&mmta_id=8
[Wed Jul 30 16:53:31 2014] marc_modification_templates.pl: Use of uninitialized
value $template_id in string eq at C4/MarcModificationTemplates.pm line 84.,
referer: /cgi-bin/koha/tools/marc_modification_templates.pl

Setting status to Failed QA

-- 
You are receiving this mail because:
You are watching all bug changes.