|
Description
Marcel de Rooy
2014-12-11 11:45:08 UTC
Created attachment 39093 [details] [review] Bug 13437: Perltidy marc21_linking_section Preliminary work before converting to new plugin style. Test plan: Run perltidy -pro=xt/perltidyrc on marc21_linking_section and compare. Created attachment 39095 [details] [review] Bug 13437: Preliminary changes for marc21 plugins field 008 Preliminary work on marc21_field_008.pl and 008_authorities.pl. Moving $dateentered en $defaultval to lower scope level. Test plan: Use both plugins in the marc21 editor (biblios/authorities). Still more to come..Please wait Created attachment 39140 [details] [review] Bug 13437: Perltidy some plugin files before conversion Preliminary work before converting to new plugin style. Several files still contained tabs or had strange indentation. Perltidied: marc21_linking_section.pl Perltidied: marc21_field_007.pl Perltidied: marc21_leader_authorities.pl Perltidied: marc21_leader.pl Perltidied: marc21_leader_book.pl Perltidied: marc21_leader_computerfile.pl Perltidied: marc21_leader_video.pl Test plan: Run perltidy -pro=xt/perltidyrc on marc21_linking_section and compare. Try another one too :) Created attachment 39141 [details] [review] Bug 13437: Preliminary changes for marc21 plugins field 008 Preliminary work on marc21_field_008.pl and 008_authorities.pl. Moving $dateentered en $defaultval to lower scope level. Test plan: Use both plugins in the marc21 editor (biblios/authorities). Created attachment 39142 [details] [review] Bug 13437: Trivial edits on marc21 plugins before conversion marc21_field_003.pl: copy-pasta comment about date removed marc21_field_005.pl: commented use removed marc21_field_006.pl: old/irrelevant pod lines marc21_field_007.pl: old/irrelevant pod lines marc21_field_008.pl: old/irrelevant pod lines, move sub par line twice marc21_field_008_authorities.pl: whitespace, old/irrelevant pod lines marc21_field_040c.pl: two commented lines removed marc21_field_040d.pl: whitespace, commented lines, old/irrelevant pod marc21_field_245h.pl: whitespace marc21_linking_section.pl: relocated some comment lines, and replaced a new CGI object by the one passed in via the plugin launcher (agreed, this may not be so trivial as the other changes) Test plan: These (trivial) changes are hard to test. Pick a few plugins and verify that behavior is not changed in the marc editor. For the brave: Try marc21_linking_section.pl. Two more coming Created attachment 39177 [details] [review] Bug 13437: Conversion of marc21 cataloguing plugins to new style This patch converts marc21 plugins to new style by making the following modifications: [1] Replace use strict with use Modern::Perl. This implies that we now re-enabled warnings. There are no redefine warnings anymore, but note that we need to silence some warnings from individual plugins that were covered by disabling the warnings pragma until now. Silencing these individual warnings is outside the scope of this report. [2] Sub plugin_javascript is replaced by an anonymous subroutine $builder. [3] The parameters of $builder are combined in a params hashref. In most cases we only need $params->{id} for the function name. [4] Javascript function Clicxxx is renamed to Clickxxx. [5] The builder does no longer return function_name. [6] Sub plugin is replaced by subroutine $launcher. [7] The parameters of $launcher are combined in a params hashref. We only use $params->{cgi}. Mostly we save that to $input. One exception: $query. [8] The plugins returns a hash with $builder and/or $launcher. Test plan: [1] Run t/db_dependent/FrameworkPlugin.t -incl cataloguing/value_builder/ marc21*.pl. This should catch compile errors and general problems when building or launching these plugins. NOTE: You will see several initialize warnings from individual plugins that were hidden until now by disabling warnings. This is fine; we will be able to address these warnings now on new reports. [2] Check behavior of several plugins in the marc editor. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Checked all marc21 plugins. Attached unused plugins to some field. Some plugins (unused by default) may need some further attention, but also outside the scope of this report. Created attachment 39184 [details] [review] Bug 13437: Replace javascript function parameter in builder The builder contains js functions with a parameter like subfield_managed or i or something similar. This parameter contains the html id of the field corresponding with the plugin. With the functionality of Koha::FrameworkPlugin in place, we can eliminate one js function call and get the same id via the event passed in. Note that this actually makes the function a 'real' event handler. Also note that in many cases this parameter was not used but the id was borrowed from a perl variable like $params->{id}. If the field is not cloned, this is not a problem. But some fields can be cloned and should not use the static perl value but should get it from the event. Test plan: Look for js errors when loading the marc editor. Since the Focus or Click event code has been touched for most marc21 plugins, move your cursor into the field or click on the tag editor button. Verify that the focus event updates the correct field or the click event correctly launches the plugin AND the value comes back into the right field. Bonus: Attach a plugin with popup (like leader) to 040$d. Clone this field. Verify that the two launch buttons operate on the correct value. (This resolves a current bug.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Checked all 15 plugins. Plugin marc21_linking_section seems to work, but I could not get it to pass back something useful into my field. (Same without this patch.) Tested the clone button with leader on 040d. Ready for signoff. This patch set can go on its own. The see also reports may bring some later refinements, but are not needed right now. Created attachment 39193 [details] [review] Bug 13437: Preliminary changes for marc21 plugins field 008 Preliminary work on marc21_field_008.pl and 008_authorities.pl. Moving $dateentered en $defaultval to lower scope level. date_entered is a new sub (in a module); sub Field008 has been reduced to one line. Added a trivial unit test for date_entered. Note: the format used in date_entered could be added in DateUtils, moving this logic to a better place. Test plan: Use both plugins in the marc21 editor (biblios/authorities). Run the adjusted unit test. Created attachment 39194 [details] [review] Bug 13437: Trivial edits on marc21 plugins before conversion marc21_field_003.pl: copy-pasta comment about date removed marc21_field_005.pl: commented use removed marc21_field_006.pl: old/irrelevant pod lines marc21_field_007.pl: old/irrelevant pod lines marc21_field_008.pl: old/irrelevant pod lines, move sub par line twice marc21_field_008_authorities.pl: whitespace, old/irrelevant pod lines marc21_field_040c.pl: two commented lines removed marc21_field_040d.pl: whitespace, commented lines, old/irrelevant pod marc21_field_245h.pl: whitespace marc21_linking_section.pl: relocated some comment lines, and replaced a new CGI object by the one passed in via the plugin launcher (agreed, this may not be so trivial as the other changes) Test plan: These (trivial) changes are hard to test. Pick a few plugins and verify that behavior is not changed in the marc editor. For the brave: Try marc21_linking_section.pl. Created attachment 39195 [details] [review] Bug 13437: Conversion of marc21 cataloguing plugins to new style This patch converts marc21 plugins to new style by making the following modifications: [1] Replace use strict with use Modern::Perl. This implies that we now re-enabled warnings. There are no redefine warnings anymore, but note that we need to silence some warnings from individual plugins that were covered by disabling the warnings pragma until now. Silencing these individual warnings is outside the scope of this report. [2] Sub plugin_javascript is replaced by an anonymous subroutine $builder. [3] The parameters of $builder are combined in a params hashref. In most cases we only need $params->{id} for the function name. [4] Javascript function Clicxxx is renamed to Clickxxx. [5] The builder does no longer return function_name. [6] Sub plugin is replaced by subroutine $launcher. [7] The parameters of $launcher are combined in a params hashref. We only use $params->{cgi}. Mostly we save that to $input. One exception: $query. [8] The plugins returns a hash with $builder and/or $launcher. Test plan: [1] Run t/db_dependent/FrameworkPlugin.t -incl cataloguing/value_builder/ marc21*.pl. This should catch compile errors and general problems when building or launching these plugins. NOTE: You will see several initialize warnings from individual plugins that were hidden until now by disabling warnings. This is fine; we will be able to address these warnings now on new reports. [2] Check behavior of several plugins in the marc editor. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Checked all marc21 plugins. Attached unused plugins to some field. Some plugins (unused by default) may need some further attention, but also outside the scope of this report. Created attachment 39196 [details] [review] Bug 13437: Replace javascript function parameter in builder The builder contains js functions with a parameter like subfield_managed or i or something similar. This parameter contains the html id of the field corresponding with the plugin. With the functionality of Koha::FrameworkPlugin in place, we can eliminate one js function call and get the same id via the event passed in. Note that this actually makes the function a 'real' event handler. Also note that in many cases this parameter was not used but the id was borrowed from a perl variable like $params->{id}. If the field is not cloned, this is not a problem. But some fields can be cloned and should not use the static perl value but should get it from the event. Test plan: Look for js errors when loading the marc editor. Since the Focus or Click event code has been touched for most marc21 plugins, move your cursor into the field or click on the tag editor button. Verify that the focus event updates the correct field or the click event correctly launches the plugin AND the value comes back into the right field. Bonus: Attach a plugin with popup (like leader) to 040$d. Clone this field. Verify that the two launch buttons operate on the correct value. (This resolves a current bug.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Checked all 15 plugins. Plugin marc21_linking_section seems to work, but I could not get it to pass back something useful into my field. (Same without this patch.) Tested the clone button with leader on 040d. Second patch slightly adjusted. Created attachment 39371 [details] [review] [SIGNED-OFF] Bug 13437: Perltidy some plugin files before conversion Preliminary work before converting to new plugin style. Several files still contained tabs or had strange indentation. Perltidied: marc21_linking_section.pl Perltidied: marc21_field_007.pl Perltidied: marc21_leader_authorities.pl Perltidied: marc21_leader.pl Perltidied: marc21_leader_book.pl Perltidied: marc21_leader_computerfile.pl Perltidied: marc21_leader_video.pl Test plan: Run perltidy -pro=xt/perltidyrc on marc21_linking_section and compare. Try another one too :) Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Minor differences on comparison :) No errors Created attachment 39372 [details] [review] [SIGNED-OFF] Bug 13437: Preliminary changes for marc21 plugins field 008 Preliminary work on marc21_field_008.pl and 008_authorities.pl. Moving $dateentered en $defaultval to lower scope level. date_entered is a new sub (in a module); sub Field008 has been reduced to one line. Added a trivial unit test for date_entered. Note: the format used in date_entered could be added in DateUtils, moving this logic to a better place. Test plan: Use both plugins in the marc21 editor (biblios/authorities). Run the adjusted unit test. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Both plugins works, no errors Created attachment 39373 [details] [review] [SIGNED-OFF] Bug 13437: Trivial edits on marc21 plugins before conversion marc21_field_003.pl: copy-pasta comment about date removed marc21_field_005.pl: commented use removed marc21_field_006.pl: old/irrelevant pod lines marc21_field_007.pl: old/irrelevant pod lines marc21_field_008.pl: old/irrelevant pod lines, move sub par line twice marc21_field_008_authorities.pl: whitespace, old/irrelevant pod lines marc21_field_040c.pl: two commented lines removed marc21_field_040d.pl: whitespace, commented lines, old/irrelevant pod marc21_field_245h.pl: whitespace marc21_linking_section.pl: relocated some comment lines, and replaced a new CGI object by the one passed in via the plugin launcher (agreed, this may not be so trivial as the other changes) Test plan: These (trivial) changes are hard to test. Pick a few plugins and verify that behavior is not changed in the marc editor. For the brave: Try marc21_linking_section.pl. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Tried marc21_linking_section.pl :) (ling plugin to 773$9, create new record, search for parent and check values inserted) No errors Created attachment 39374 [details] [review] [SIGNED-OFF] Bug 13437: Conversion of marc21 cataloguing plugins to new style This patch converts marc21 plugins to new style by making the following modifications: [1] Replace use strict with use Modern::Perl. This implies that we now re-enabled warnings. There are no redefine warnings anymore, but note that we need to silence some warnings from individual plugins that were covered by disabling the warnings pragma until now. Silencing these individual warnings is outside the scope of this report. [2] Sub plugin_javascript is replaced by an anonymous subroutine $builder. [3] The parameters of $builder are combined in a params hashref. In most cases we only need $params->{id} for the function name. [4] Javascript function Clicxxx is renamed to Clickxxx. [5] The builder does no longer return function_name. [6] Sub plugin is replaced by subroutine $launcher. [7] The parameters of $launcher are combined in a params hashref. We only use $params->{cgi}. Mostly we save that to $input. One exception: $query. [8] The plugins returns a hash with $builder and/or $launcher. Test plan: [1] Run t/db_dependent/FrameworkPlugin.t -incl cataloguing/value_builder/ marc21*.pl. This should catch compile errors and general problems when building or launching these plugins. NOTE: You will see several initialize warnings from individual plugins that were hidden until now by disabling warnings. This is fine; we will be able to address these warnings now on new reports. [2] Check behavior of several plugins in the marc editor. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Checked all marc21 plugins. Attached unused plugins to some field. Some plugins (unused by default) may need some further attention, but also outside the scope of this report. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> New warnigs, but all seems to work. No errors. Created attachment 39375 [details] [review] [SIGNED-OFF] Bug 13437: Replace javascript function parameter in builder The builder contains js functions with a parameter like subfield_managed or i or something similar. This parameter contains the html id of the field corresponding with the plugin. With the functionality of Koha::FrameworkPlugin in place, we can eliminate one js function call and get the same id via the event passed in. Note that this actually makes the function a 'real' event handler. Also note that in many cases this parameter was not used but the id was borrowed from a perl variable like $params->{id}. If the field is not cloned, this is not a problem. But some fields can be cloned and should not use the static perl value but should get it from the event. Test plan: Look for js errors when loading the marc editor. Since the Focus or Click event code has been touched for most marc21 plugins, move your cursor into the field or click on the tag editor button. Verify that the focus event updates the correct field or the click event correctly launches the plugin AND the value comes back into the right field. Bonus: Attach a plugin with popup (like leader) to 040$d. Clone this field. Verify that the two launch buttons operate on the correct value. (This resolves a current bug.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Checked all 15 plugins. Plugin marc21_linking_section seems to work, but I could not get it to pass back something useful into my field. (Same without this patch.) Tested the clone button with leader on 040d. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> All seems to work, no errors Great work! Created attachment 39949 [details] [review] [PASSED QA] Bug 13437: Perltidy some plugin files before conversion Preliminary work before converting to new plugin style. Several files still contained tabs or had strange indentation. Perltidied: marc21_linking_section.pl Perltidied: marc21_field_007.pl Perltidied: marc21_leader_authorities.pl Perltidied: marc21_leader.pl Perltidied: marc21_leader_book.pl Perltidied: marc21_leader_computerfile.pl Perltidied: marc21_leader_video.pl Test plan: Run perltidy -pro=xt/perltidyrc on marc21_linking_section and compare. Try another one too :) Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Minor differences on comparison :) No errors Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 39950 [details] [review] [PASSED QA] Bug 13437: Preliminary changes for marc21 plugins field 008 Preliminary work on marc21_field_008.pl and 008_authorities.pl. Moving $dateentered en $defaultval to lower scope level. date_entered is a new sub (in a module); sub Field008 has been reduced to one line. Added a trivial unit test for date_entered. Note: the format used in date_entered could be added in DateUtils, moving this logic to a better place. Test plan: Use both plugins in the marc21 editor (biblios/authorities). Run the adjusted unit test. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Both plugins works, no errors Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 39951 [details] [review] [PASSED QA] Bug 13437: Trivial edits on marc21 plugins before conversion marc21_field_003.pl: copy-pasta comment about date removed marc21_field_005.pl: commented use removed marc21_field_006.pl: old/irrelevant pod lines marc21_field_007.pl: old/irrelevant pod lines marc21_field_008.pl: old/irrelevant pod lines, move sub par line twice marc21_field_008_authorities.pl: whitespace, old/irrelevant pod lines marc21_field_040c.pl: two commented lines removed marc21_field_040d.pl: whitespace, commented lines, old/irrelevant pod marc21_field_245h.pl: whitespace marc21_linking_section.pl: relocated some comment lines, and replaced a new CGI object by the one passed in via the plugin launcher (agreed, this may not be so trivial as the other changes) Test plan: These (trivial) changes are hard to test. Pick a few plugins and verify that behavior is not changed in the marc editor. For the brave: Try marc21_linking_section.pl. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Tried marc21_linking_section.pl :) (ling plugin to 773$9, create new record, search for parent and check values inserted) No errors Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 39952 [details] [review] [PASSED QA] Bug 13437: Conversion of marc21 cataloguing plugins to new style This patch converts marc21 plugins to new style by making the following modifications: [1] Replace use strict with use Modern::Perl. This implies that we now re-enabled warnings. There are no redefine warnings anymore, but note that we need to silence some warnings from individual plugins that were covered by disabling the warnings pragma until now. Silencing these individual warnings is outside the scope of this report. [2] Sub plugin_javascript is replaced by an anonymous subroutine $builder. [3] The parameters of $builder are combined in a params hashref. In most cases we only need $params->{id} for the function name. [4] Javascript function Clicxxx is renamed to Clickxxx. [5] The builder does no longer return function_name. [6] Sub plugin is replaced by subroutine $launcher. [7] The parameters of $launcher are combined in a params hashref. We only use $params->{cgi}. Mostly we save that to $input. One exception: $query. [8] The plugins returns a hash with $builder and/or $launcher. Test plan: [1] Run t/db_dependent/FrameworkPlugin.t -incl cataloguing/value_builder/ marc21*.pl. This should catch compile errors and general problems when building or launching these plugins. NOTE: You will see several initialize warnings from individual plugins that were hidden until now by disabling warnings. This is fine; we will be able to address these warnings now on new reports. [2] Check behavior of several plugins in the marc editor. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Checked all marc21 plugins. Attached unused plugins to some field. Some plugins (unused by default) may need some further attention, but also outside the scope of this report. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> New warnigs, but all seems to work. No errors. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 39953 [details] [review] [PASSED QA] Bug 13437: Replace javascript function parameter in builder The builder contains js functions with a parameter like subfield_managed or i or something similar. This parameter contains the html id of the field corresponding with the plugin. With the functionality of Koha::FrameworkPlugin in place, we can eliminate one js function call and get the same id via the event passed in. Note that this actually makes the function a 'real' event handler. Also note that in many cases this parameter was not used but the id was borrowed from a perl variable like $params->{id}. If the field is not cloned, this is not a problem. But some fields can be cloned and should not use the static perl value but should get it from the event. Test plan: Look for js errors when loading the marc editor. Since the Focus or Click event code has been touched for most marc21 plugins, move your cursor into the field or click on the tag editor button. Verify that the focus event updates the correct field or the click event correctly launches the plugin AND the value comes back into the right field. Bonus: Attach a plugin with popup (like leader) to 040$d. Clone this field. Verify that the two launch buttons operate on the correct value. (This resolves a current bug.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Checked all 15 plugins. Plugin marc21_linking_section seems to work, but I could not get it to pass back something useful into my field. (Same without this patch.) Tested the clone button with leader on 040d. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> All seems to work, no errors Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Patches pushed to master. Thanks Marcel! |