[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

Jonathan Druart <jonathan.druart at biblibre.com> changed:

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

--- Comment #36 from Jonathan Druart <jonathan.druart at biblibre.com> ---
(In reply to M. de Rooy from comment #35)
> QA Comment: 

Marcel,
Thanks a lot to had a look at this.

> 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. 
> [...]
> [2]
> At this moment you are blocking an update without subfield in the interface.
> This makes it hard to reach this die statement..

It is more or less intended.
All changes done on these patches (and linked) are not trivial and I don't want
to add complexity for now.
This limitation will exist until the entire feature will be pushed.
(Not enough time at the moment to work on it without knowing when it will be
pushed...).

> [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? 

Could you please retest with *all* patches applied?
Actually I split patches into different bug reports but all patches should be
QA at once (patches from bug 11413 and bug 11395).
I think this is fixed by the patch "Bug 11413: Change the field number logic".

> [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

These warnings existed before this patch set isn't it?

I can open another bug report to get rid of warnings if they still exist after
applying all patches.

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