https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=12412 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | Patch complexity|--- |Small patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65595 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65595&action=edit Bug 19049: Restore import with to_marc plugin Still needs some polishing and testing. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65656 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65656&action=edit Bug 19049: Tests -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Recover the Arbitrary file |Fix regression on |type on stage-marc-import |stage-marc-import with |with plugin to_marc |to_marc plugin -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65708 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65708&action=edit Bug 19049: Testing RecordsFromMarcPlugin with a to_marc plugin This patch adds a simple to_marc plugin in t/Koha/Plugin that is used in the added subtest in ImportBatch.t. Test plan: [1] Run t/db_dependent/ImportBatch.t [2] Copy the to_marc test plugin from t to your plugin directory. Under Debian packages, you should do something like: mkdir -p /var/lib/koha/master/plugins/Koha/Plugin/ cp [yourclone]/t/Koha/Plugin/MarcFieldValues.pm /var/lib/koha/master/plugins/Koha/Plugin/ [3] Check if you see this plugin on plugins/plugins-home.pl [4] Create a text file with some fields like: (Note: The plugin needs an empty line between both "records".) 100,a = Test Author 1 245,a = Title One 100,a = Author 2 245,a = Title Two [5] Go to stage-marc-import.pl. Upload the created file. Select the plugin in the format combo and proceed. Did you create two records ? Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65656|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de, | |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Increasing Severity -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65797 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65797&action=edit Bug 19049: [Follow-up] Make sure that GetPlugins finds the plugin We have to mock_config the pluginsdir before Plugins is loaded, and we should pass an absolute path (not a relative one). If you did not install the test to_marc plugin, this test would fail. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65708|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 65802 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65802&action=edit [SIGNED OFF] Bug 19049: Fix regression on stage-marc-import with to_marc plugin Bug 12412 added the use of to_marc plugins allowing arbitrary file formats in stage-marc-import (as long as the plugins can handle them). The feature was not very visible in the code, and when bug 10407 added the marcxml format, it made some changes that broke the use of to_marc. This patch restores the functionality by: [1] Adding a sub RecordsFromMarcPlugin to ImportBatch.pm, specifically addressing the conversion from arbitrary formats to MARC::Record. The original to_marc interface is used: pass it the file contents, and it returns a string consisting of a number of MARC blobs separated by \x1D. Consequently, the call of to_marc is removed from routine BatchStageMarcRecords where it did not belong. The to_marc_plugin parameter is removed and two calls are adjusted accordingly. [2] Instead of a separate combo with plugins, the format combo contains MARC, MARCXML and optionally some plugin formats. [3] The code in stage-marc-import.pl now clearly shows the three main format types: MARC, MARCXML or plugin based. Note: This patch restores more or less the situation after bug 12412, but I would actually recommend to have the to_marc plugins return MARC::Record objects instead of large text strings. In the second example I added a to_marc plugin that actually converts MARC record objects to string format, while RecordsFromMarcPlugin reconverts them to MARC::Records. Test plan: See second patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 65804 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65804&action=edit [SIGNED OFF] Bug 19049: [Follow-up] Make sure that GetPlugins finds the plugin We have to mock_config the pluginsdir before Plugins is loaded, and we should pass an absolute path (not a relative one). If you did not install the test to_marc plugin, this test would fail. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65802|0 |1 is obsolete| | Attachment #65803|0 |1 is obsolete| | Attachment #65804|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 65807 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65807&action=edit Bug 19049: Fix regression on stage-marc-import with to_marc plugin Bug 12412 added the use of to_marc plugins allowing arbitrary file formats in stage-marc-import (as long as the plugins can handle them). The feature was not very visible in the code, and when bug 10407 added the marcxml format, it made some changes that broke the use of to_marc. This patch restores the functionality by: [1] Adding a sub RecordsFromMarcPlugin to ImportBatch.pm, specifically addressing the conversion from arbitrary formats to MARC::Record. The original to_marc interface is used: pass it the file contents, and it returns a string consisting of a number of MARC blobs separated by \x1D. Consequently, the call of to_marc is removed from routine BatchStageMarcRecords where it did not belong. The to_marc_plugin parameter is removed and two calls are adjusted accordingly. [2] Instead of a separate combo with plugins, the format combo contains MARC, MARCXML and optionally some plugin formats. [3] The code in stage-marc-import.pl now clearly shows the three main format types: MARC, MARCXML or plugin based. Note: This patch restores more or less the situation after bug 12412, but I would actually recommend to have the to_marc plugins return MARC::Record objects instead of large text strings. In the second example I added a to_marc plugin that actually converts MARC record objects to string format, while RecordsFromMarcPlugin reconverts them to MARC::Records. Test plan: See second patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 65809 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65809&action=edit Bug 19049: [Follow-up] Make sure that GetPlugins finds the plugin We have to mock_config the pluginsdir before Plugins is loaded, and we should pass an absolute path (not a relative one). If you did not install the test to_marc plugin, this test would fail. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Verified patch is compatible with original KitchenSink to_marc plugin -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Marcel, -my $format = $input->param('format') || 'ISO2709'; +my $format = $input->param('format'); Then later: + } else { + die "No format specified"; + } I would prefer to avoid the die and default to "MARC" if format is not passed, especially if we backport this change. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #20)

t/db_dependent/ImportBatch.t .. 1/14 # No tests run!

# Failed test 'No tests run for subtest "RecordsFromMarcPlugin"' # at t/db_dependent/ImportBatch.t line 199. Can't call method "GetPlugins" on an undefined value at t/db_dependent/ImportBatch.t line 190.

Must skip tests if enable_plugins is not set in koha-conf.xml or pass it in arg.

It has to be mocked IMHO. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65876 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65876&action=edit Bug 19049: [QA Follow-up] Mock config, default format As requested by QA: [1] Mock_config enable_plugins in the test. [2] Fallback to MARC when format is empty. Remove die statement. Added: [3] Remove $marc. This variable got obsolete during development. [4] Add if to the Handler call and add if around processing $text. No need to split undef if somehow plugin was not found, etc. If the routine returns an empty arrayref, stage-marc-import will do fine. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65876|0 |1 is obsolete| | --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65877 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65877&action=edit Bug 19049: [QA Follow-up] Mock config, default format As requested by QA: [1] Mock_config enable_plugins in the test. [2] Fallback to MARC when format is empty. Remove die statement. Added: [3] Remove $marc. This variable got obsolete during development. [4] Add test on $input_file and $plugin_class. Test $text before calling Handler or processing $text. No need to split undef if somehow Handler returned undef, etc. If the routine returns an empty arrayref, stage-marc-import will do fine. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Wrong bug! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19163 --- Comment #28 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- This causes a critical bug. See fix on bug 19163 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19163 [Bug 19163] Critical typo in stage-marc-import process -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19049 --- Comment #30 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- These patches have been pushed to 16.11.x and will be in 16.11.12. Fridolin, please take a look at the dependent bug 19163 - it fixes a critical problem with this patch set! -- You are receiving this mail because: You are watching all bug changes.