Created attachment 49823 [details] [review] Bug 13438: Convert normarc framework plugins to new style Converts NORMARC plugins to new style (with builder and launcher). See also bugs 10480 and 13437. Note: Search.pm is not needed in these plugins. Tabs have been removed in the same pass. Note 2: For the 008 plugin the tag editor already seems to have some problems (js errors on e.g. f18 for Velg materialtype, etc.) This makes that pressing OK does not always work. Outside the scope of this report. Test plan: [1] Run t/db_dependent/FrameworkPlugin.t -incl cataloguing/value_builder/normarc*.pl. (Ignore substr and uninitialized warnings; these are not in the scope of this patch.) [2] Connect the plugin. Test focus (leader,008) and click event. (See note 2.) [3] Look for warnings in the log related(!) to this change.
Oops Subroutine date_entered redefined at /usr/share/koha/masterclone/cataloguing/value_builder/normarc_field_008.pl line 29.
Created attachment 49824 [details] [review] Bug 13438: Follow-up for removing date_entered Defining this sub in normarc_field_008.pl is not necessary. Use the one from Koha::Util::FrameworkPlugin. Test plan: Run the 008 plugin again.
Comment on attachment 49823 [details] [review] Bug 13438: Convert normarc framework plugins to new style Review of attachment 49823 [details] [review]: ----------------------------------------------------------------- ::: cataloguing/value_builder/normarc_field_007.pl @@ +88,1 @@ > #FIXME: There is probably a more-elegant way to do this! You mean something like: use Data::Dumper; my $str='abcdef'; my $len=22-length $str; @tmp = split //,$str; push @tmp,(undef)x($len); print Dumper(\@tmp);" And some sort of my @field_numbers = map { "f$_" } (0..21); Then use a loop to template->param it all?
(In reply to M. Tompsett from comment #4) > Comment on attachment 49823 [details] [review] [review] > Bug 13438: Convert normarc framework plugins to new style > > Review of attachment 49823 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: cataloguing/value_builder/normarc_field_007.pl > @@ +88,1 @@ > > #FIXME: There is probably a more-elegant way to do this! Marc, This is existing code (also in marc21_field_007). Probably only touched here due to preceding tabs. Fixing this is not in the scope of this report. If there is no report yet, we should open a new one. Btw I am not sure if we can still reproduce it. (Just tried a few combinations of f5,6,7 in normarc.)
I might be missing something, but when I run the test thingy I get errors: $ sudo koha-shell -c "prove t/db_dependent/FrameworkPlugin.t -incl cataloguing/value_builder/normarc*.pl" kohadev [sudo] password for magnus: Feil. Prøv igjen. [sudo] password for magnus: t/db_dependent/FrameworkPlugin.t ................ 1/5 substr outside of string at /home/magnus/scripts/kohaclone/cataloguing/value_builder/marc21_field_007.pl line 73. substr outside of string at /home/magnus/scripts/kohaclone/cataloguing/value_builder/marc21_field_007.pl line 74. [...] substr outside of string at /home/magnus/scripts/kohaclone/cataloguing/value_builder/marc21_field_007.pl line 92. Use of uninitialized value $f3 in concatenation (.) or string at /home/magnus/scripts/kohaclone/cataloguing/value_builder/marc21_field_007.pl line 99. [...] Use of uninitialized value $f22 in concatenation (.) or string at /home/magnus/scripts/kohaclone/cataloguing/value_builder/marc21_field_007.pl line 99. oAuth error: Unsupported Use attribute (114) authtype Bib-1 DBIx::Class::Schema::resultset(): resultset() expects a source name at /usr/share/koha/lib/Koha/Objects.pm line 237 # Child (Test05 -- tests with build and launch for default plugins) exited without calling finalize() t/db_dependent/FrameworkPlugin.t ................ 5/5 # Failed test 'Test05 -- tests with build and launch for default plugins' # at /usr/local/share/perl/5.18.2/Test/Builder.pm line 279. # Looks like you failed 1 test of 5. # Looks like your test exited with 255 just after 5. t/db_dependent/FrameworkPlugin.t ................ Dubious, test returned 255 (wstat 65280, 0xff00) Failed 1/5 subtests Cannot detect source of '-incl'! at /usr/local/share/perl/5.18.2/TAP/Parser/IteratorFactory.pm line 261. TAP::Parser::IteratorFactory::detect_source('TAP::Parser::IteratorFactory=HASH(0x27879b0)', 'TAP::Parser::Source=HASH(0x2787aa0)') called at /usr/local/share/perl/5.18.2/TAP/Parser/IteratorFactory.pm line 211 TAP::Parser::IteratorFactory::make_iterator('TAP::Parser::IteratorFactory=HASH(0x27879b0)', 'TAP::Parser::Source=HASH(0x2787aa0)') called at /usr/local/share/perl/5.18.2/TAP/Parser.pm line 470 TAP::Parser::_initialize('TAP::Parser=HASH(0x2787ae8)', 'HASH(0x25da478)') called at /usr/local/share/perl/5.18.2/TAP/Object.pm line 55 TAP::Object::new('TAP::Parser', 'HASH(0x25da478)') called at /usr/local/share/perl/5.18.2/TAP/Object.pm line 130 TAP::Object::_construct('TAP::Harness=HASH(0x22f38e8)', 'TAP::Parser', 'HASH(0x25da478)') called at /usr/local/share/perl/5.18.2/TAP/Harness.pm line 852 TAP::Harness::make_parser('TAP::Harness=HASH(0x22f38e8)', 'TAP::Parser::Scheduler::Job=HASH(0x25da658)') called at /usr/local/share/perl/5.18.2/TAP/Harness.pm line 651 TAP::Harness::_aggregate_single('TAP::Harness=HASH(0x22f38e8)', 'TAP::Parser::Aggregator=HASH(0x23f7770)', 'TAP::Parser::Scheduler=HASH(0x25da6d0)') called at /usr/local/share/perl/5.18.2/TAP/Harness.pm line 743 TAP::Harness::aggregate_tests('TAP::Harness=HASH(0x22f38e8)', 'TAP::Parser::Aggregator=HASH(0x23f7770)', 't/db_dependent/FrameworkPlugin.t', '-incl', 'cataloguing/value_builder/normarc_field_007.pl', 'cataloguing/value_builder/normarc_field_008.pl', 'cataloguing/value_builder/normarc_leader.pl') called at /usr/local/share/perl/5.18.2/TAP/Harness.pm line 558 TAP::Harness::__ANON__() called at /usr/local/share/perl/5.18.2/TAP/Harness.pm line 571 TAP::Harness::runtests('TAP::Harness=HASH(0x22f38e8)', 't/db_dependent/FrameworkPlugin.t', '-incl', 'cataloguing/value_builder/normarc_field_007.pl', 'cataloguing/value_builder/normarc_field_008.pl', 'cataloguing/value_builder/normarc_leader.pl') called at /usr/local/share/perl/5.18.2/App/Prove.pm line 546 App::Prove::_runtests('App::Prove=HASH(0x1e852c8)', 'HASH(0x22c42f0)', 't/db_dependent/FrameworkPlugin.t', '-incl', 'cataloguing/value_builder/normarc_field_007.pl', 'cataloguing/value_builder/normarc_field_008.pl', 'cataloguing/value_builder/normarc_leader.pl') called at /usr/local/share/perl/5.18.2/App/Prove.pm line 504 App::Prove::run('App::Prove=HASH(0x1e852c8)') called at /usr/local/bin/prove line 12 I have replaced some repeated errors with [...] This is on a gitified package install.
Thanks Magnus for testing. You discovered that prove does not like extra arguments. "Cannot detect source of '--incl'! at /usr/share/perl/5.14/TAP/Parser/IteratorFactory.pm line 263" Prove provides a way (I learned now too): It is possible to supply arguments to tests. To do so separate them from prove's own arguments with the arisdottle, '::'. For example prove -v t/mytest.t :: --url http://example.com Also note that the overwhelming number of uninitialized warnings was only hidden by disabling the warnings pragma. If you would only add "use warnings" in the plugins before this patch and run prove, you will see them all. So now we see those warnings, we should do something about them.. (I already have a report opened for that.) prove t/db_dependent/FrameworkPlugin.t :: --incl cataloguing/value_builder/normarc_*.pl t/db_dependent/FrameworkPlugin.t .. 1/5 [LOTs of uninitialized warnings] t/db_dependent/FrameworkPlugin.t .. ok All tests successful. Files=1, Tests=5, 2 wallclock secs ( 0.03 usr 0.01 sys + 2.08 cusr 0.10 csys = 2.22 CPU) Result: PASS
Hi, testing loading no-NB frameworks. Leader and 007 seems right, 007 gives warnings on log file. Buy 008 does not save, only resets the value.
(In reply to Bernardo Gonzalez Kriegel from comment #8) > Hi, > testing loading no-NB frameworks. > > Leader and 007 seems right, 007 gives warnings on log file. > Buy 008 does not save, only resets the value. Thx for testing. I have the same problem with 008 (as noted on the commit message), but it is not related to this patch. You have the same problem before this patch. Perhaps Magnus can clarify or open a new report for it ?
(In reply to Marcel de Rooy from comment #9) > I have the same problem with 008 (as noted on the commit message), but it is > not related to this patch. You have the same problem before this patch. Not exactly the same, it fails to load current values but can save new ones. With patch clicking Save does nothing, for me of course (i'm having doubts about my testing env)
Yes, there is a problem with 008. My plan was to fix this here: Bug 10997 - Modernize value_builders for NORMARC but I have not found the time yet...
Created attachment 51453 [details] [review] Bug 13438: Fix report function in normarc_field_008 js The report function did not deal well with undefined fields and did not respect the length of all elements. NOTE: This is a quick fix for something that is currently broken. A next (larger) step would be to bring it back to the approach in the marc21 counterpart. Test plan: Make sure that normarc_field_008.pl is connected to your field 008. Click tag editor for 008, save some values and go back to tag editor. Test different material types in the tag editor.
Have been asking assistance for months. Abandoning this development.