[Bug 20551] New: Add option for including deleted records in export_records.pl
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Bug ID: 20551 Summary: Add option for including deleted records in export_records.pl Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: glasklas@gmail.com QA Contact: testopia@bugs.koha-community.org CC: robin@catalyst.net.nz Add an option "--include_deleted" to also include possibly deleted biblios in export. This option can only be used together with --date, and --record-type=biblio. The rational behind this is that use case does not makes much sense (if you are doing a full export, why would you include deleted biblios) and thus some extra complexity can be avoided. Superficially related to https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17693, not really quite different as this includes the whole biblio-record (with record status set to "d" in leader) in the regular export, not just biblio ids. This is a requirement for synchronizing with the discovery-system we are currently using for example and imho is a more standard and MARC-compliant way of supporting export of record deletions. -- 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=20551 --- Comment #1 from David Gustafsson <glasklas@gmail.com> --- Created attachment 73916 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73916&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d". How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` Sponsored-by: Gothenburg University Library -- 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=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73916|0 |1 is obsolete| | --- Comment #2 from David Gustafsson <glasklas@gmail.com> --- Created attachment 73923 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73923&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library -- 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=20551 --- Comment #3 from David Gustafsson <glasklas@gmail.com> --- As some might need it, also added an '--deleted_only' option, to export only the deleted biblios. -- 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=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |glasklas@gmail.com |ity.org | -- 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=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73923|0 |1 is obsolete| | --- Comment #4 from David Gustafsson <glasklas@gmail.com> --- Created attachment 75113 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75113&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #5 from David Gustafsson <glasklas@gmail.com> --- Rebased against Koha master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75113|0 |1 is obsolete| | --- Comment #6 from David Gustafsson <glasklas@gmail.com> --- Created attachment 82843 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82843&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #7 from David Gustafsson <glasklas@gmail.com> --- Rebase against master (untested, will test later and verify that still works). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #8 from Owen Leonard <oleonard@myacpl.org> --- The functionality of this works as described in the test plan. However, the QA tools raise this warning: FAIL Koha/Exporter/Record.pm FAIL critic # Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s). And t/db_dependent/Exporter/Record.t returns this error: t/db_dependent/Exporter/Record.t .. 1/6 # No tests run! # Failed test 'No tests run for subtest "export xml"' # at t/db_dependent/Exporter/Record.t line 187. # No tests run! # Failed test 'No tests run for subtest "export iso2709"' # at t/db_dependent/Exporter/Record.t line 219. # Looks like you failed 2 tests of 6. t/db_dependent/Exporter/Record.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/6 subtests Test Summary Report ------------------- t/db_dependent/Exporter/Record.t (Wstat: 512 Tests: 6 Failed: 2) Failed tests: 2-3 Non-zero exit status: 2 Files=1, Tests=6, 3 wallclock secs ( 0.02 usr 0.01 sys + 1.41 cusr 0.69 csys = 2.13 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82843|0 |1 is obsolete| | --- Comment #9 from David Gustafsson <glasklas@gmail.com> --- Created attachment 82914 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82914&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #10 from David Gustafsson <glasklas@gmail.com> --- Found the culprit (next in map that accidentally got there in rebase, where should just return empty list), now tests should pass. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #11 from David Gustafsson <glasklas@gmail.com> --- Forgot to fix the perl critic issue, will do. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82914|0 |1 is obsolete| | --- Comment #12 from David Gustafsson <glasklas@gmail.com> --- Created attachment 82918 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82918&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frank.hansen@ub.lu.se -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #13 from Frank Hansen <frank.hansen@ub.lu.se> --- Patch didn't apply for me. I got: CONFLICT (content): Merge conflict in misc/export_records.pl Failed to merge in the changes. Patch failed at 0001 Bug 20551: Add option for including deleted records in export_records.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |katrin.fischer@bsz-bw.de --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Same for me, marking "Patch doesn't apply". Frnak, you can set that status too if you run into this the next time! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82918|0 |1 is obsolete| | --- Comment #15 from David Gustafsson <glasklas@gmail.com> --- Created attachment 90184 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90184&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #16 from David Gustafsson <glasklas@gmail.com> --- Rebased against master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |23009 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23009 [Bug 23009] Add -deleted_marc_conditions argument to export_records script -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #17 from David Gustafsson <glasklas@gmail.com> --- Created attachment 90190 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90190&action=edit Bug 20551: Fix option name in pod -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #18 from David Gustafsson <glasklas@gmail.com> --- Created attachment 90220 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90220&action=edit Bug 20551: Rename "marcflavour" field to "schema" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |eric.begin@inLibro.com --- Comment #19 from Eric Bégin <eric.begin@inLibro.com> --- Test during hackfest 2019 error: sha1 information is lacking or useless (Koha/Exporter/Record.pm). error: could not build fake ancestor Patch failed at 0001 Bug 20551: Rename "marcflavour" field to "schema" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90184|0 |1 is obsolete| | --- Comment #20 from David Gustafsson <glasklas@gmail.com> --- Created attachment 93619 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93619&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90190|0 |1 is obsolete| | --- Comment #21 from David Gustafsson <glasklas@gmail.com> --- Created attachment 93620 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93620&action=edit Bug 20551: Fix option name in pod -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90220|0 |1 is obsolete| | --- Comment #22 from David Gustafsson <glasklas@gmail.com> --- Created attachment 93621 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93621&action=edit Bug 20551: Rename "marcflavour" field to "schema" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #23 from David Gustafsson <glasklas@gmail.com> --- Should now be rebased against master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #24 from Frank Hansen <frank.hansen@ub.lu.se> --- (In reply to David Gustafsson from comment #23)
Should now be rebased against master.
The patch itself applies and works fine for me. However, the test doesn't pass: t/db_dependent/Exporter/Record.t .. 1/6 # Failed test 'Deleted record has the correct leader value' # at t/db_dependent/Exporter/Record.t line 186. # got: '00134dam a22000617a 4500' # expected: '00136dam a22000617a 4500' # Looks like you failed 1 test of 4. # Failed test 'export xml' # at t/db_dependent/Exporter/Record.t line 187. # Failed test 'Deleted record has the correct leader value' # at t/db_dependent/Exporter/Record.t line 218. # got: '00134dam a22000617a 4500' # expected: '00136dam a22000617a 4500' # Looks like you failed 1 test of 4. # Failed test 'export iso2709' # at t/db_dependent/Exporter/Record.t line 219. t/db_dependent/Exporter/Record.t .. 5/6 # Looks like you failed 2 tests of 6. t/db_dependent/Exporter/Record.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/6 subtests Test Summary Report ------------------- t/db_dependent/Exporter/Record.t (Wstat: 512 Tests: 6 Failed: 2) Failed tests: 2-3 Non-zero exit status: 2 Files=1, Tests=6, 4 wallclock secs ( 0.02 usr 0.00 sys + 2.94 cusr 0.28 csys = 3.24 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #25 from Frank Hansen <frank.hansen@ub.lu.se> --- The test failed because I ran the test on an empty database. I will now sign off the patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #26 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 1. Doesn't it make sense to have this for authority records as well? 2. Why is --date mandatory? 3. POD does not tell that --deleted_only and --include_deleted is only available for bibs 4. --id_list_file does not work with --deleted_only 5. Items are not exported, I guess that's expected but maybe we should have it noted somewhere? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |theod@lib.auth.gr --- Comment #27 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- *** Bug 17693 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #28 from Frank Hansen <frank.hansen@ub.lu.se> --- (In reply to Jonathan Druart from comment #26)
1. Doesn't it make sense to have this for authority records as well?
The authority records are stored in auth_header table. But deleted records is not kept anywhere else. As I can see, this would require to keep deleted authority records in an another table like deletedauth_header. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #29 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Frank, you are right, we do not keep the deleted authorities, so that takes care of 1. I think some of the other questions could still be answered/improved: (In reply to Jonathan Druart from comment #26)
1. Doesn't it make sense to have this for authority records as well?
Because we don't keep them ;)
2. Why is --date mandatory?
Not sure about this one, can you explain?
3. POD does not tell that --deleted_only and --include_deleted is only available for bibs
Easy fix to documentation I think and it could help people who are not aware of database structure.
4. --id_list_file does not work with --deleted_only
Cannot tell about this one, but sounds worth looking into.
5. Items are not exported, I guess that's expected but maybe we should have it noted somewhere?
Maybe also a note in the docs? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #30 from David Gustafsson <glasklas@gmail.com> --- (In reply to Jonathan Druart from comment #26)
1. Doesn't it make sense to have this for authority records as well?
2. Why is --date mandatory?
3. POD does not tell that --deleted_only and --include_deleted is only available for bibs
4. --id_list_file does not work with --deleted_only
5. Items are not exported, I guess that's expected but maybe we should have it noted somewhere?
2) I think I just assumed it was required since included in the SQL-query, also before the patch. Did not realize that "SELECT * FROM biblio_metadata WHERE timestamp > ''" is equivalent with "SELECT * FROM biblio_metadata" if every timestamp entry has a value. So with that in mind an empty timestamp is ok, though the code is a real eyesore :) 3) I think it's already pretty clear since will produce an error message is --included_deleted or --deleted_only is used in conjunction with record_type other than 'bibs'. But I clarified this in the usage documentation for said options. 4) Did not work since have to add special case for @deleted_record_ids, fixed this. 5). There actually is a "deleteditems" table so I guess it would be possible to include these, even though it's hard to imagine why you would want to do this. Since the default is to include items perhaps it would be best to leave this as is and never include items for deleted biblios, or you will get more data than you probably want by default. It would also increase code complexity, probably with little benefit. If someone requests this it can be fixed later perhaps. Will point it out in the documentation that items are not included. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #31 from David Gustafsson <glasklas@gmail.com> --- Created attachment 105403 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105403&action=edit Bug 20551: Improve documentation and make compatible with more options -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arthur.suzuki@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> 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=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93619|0 |1 is obsolete| | --- Comment #32 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 116408 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116408&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library Great job! Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93620|0 |1 is obsolete| | --- Comment #33 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 116409 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116409&action=edit Bug 20551: Fix option name in pod Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93621|0 |1 is obsolete| | --- Comment #34 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 116410 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116410&action=edit Bug 20551: Rename "marcflavour" field to "schema" Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105403|0 |1 is obsolete| | --- Comment #35 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 116411 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116411&action=edit Bug 20551: Improve documentation and make compatible with more options Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #36 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
5). There actually is a "deleteditems" table so I guess it would be possible to include these, even though it's hard to imagine why you would want to do this. Since the default is to include items perhaps it would be best to leave this as is and never include items for deleted biblios, or you will get more data than you probably want by default. It would also increase code complexity, probably with little benefit. If someone requests this it can be fixed later perhaps. Will point it out in the documentation that items are not included.
I think introducing the export of deleted records and deleted items separately would be a good idea. Let's start with this well defined feature and think about the deleted items some more. While records have a flag to say "deleted", we don't have the same for items. So we could start exporting deleted items that belonged to deleted records, but what about other deleted items on existing records? How to "mark" them as deleted? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #37 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am sorry, but tests are failing :( kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Exporter/Record.t t/db_dependent/Exporter/Record.t .. 1/6 Use of uninitialized value in concatenation (.) or string at /usr/share/perl5/MARC/File/XML.pm line 397, <$fh> chunk 7. # Failed test 'Deleted record has the correct leader value' # at t/db_dependent/Exporter/Record.t line 179. # got: '00134dam a22000617a 4500' # expected: '00136dam a22000617a 4500' # Looks like you failed 1 test of 4. # Failed test 'export xml' # at t/db_dependent/Exporter/Record.t line 180. # Failed test 'Deleted record has the correct leader value' # at t/db_dependent/Exporter/Record.t line 211. # got: '00134dam a22000617a 4500' # expected: '00136dam a22000617a 4500' # Looks like you failed 1 test of 4. # Failed test 'export iso2709' # at t/db_dependent/Exporter/Record.t line 212. # Looks like you failed 2 tests of 6. t/db_dependent/Exporter/Record.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/6 subtests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #38 from David Gustafsson <glasklas@gmail.com> --- Created attachment 123099 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123099&action=edit Bug 20551: fix tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123099|0 |1 is obsolete| | --- Comment #39 from David Gustafsson <glasklas@gmail.com> --- Created attachment 123101 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123101&action=edit Bug 20551: Add missing subroutine import -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #40 from David Gustafsson <glasklas@gmail.com> --- Created attachment 123102 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123102&action=edit Bug 20551: Fix tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #41 from David Gustafsson <glasklas@gmail.com> --- (In reply to Katrin Fischer from comment #36)
5). There actually is a "deleteditems" table so I guess it would be possible to include these, even though it's hard to imagine why you would want to do this. Since the default is to include items perhaps it would be best to leave this as is and never include items for deleted biblios, or you will get more data than you probably want by default. It would also increase code complexity, probably with little benefit. If someone requests this it can be fixed later perhaps. Will point it out in the documentation that items are not included.
I think introducing the export of deleted records and deleted items separately would be a good idea. Let's start with this well defined feature and think about the deleted items some more.
While records have a flag to say "deleted", we don't have the same for items. So we could start exporting deleted items that belonged to deleted records, but what about other deleted items on existing records? How to "mark" them as deleted?
Now the tests are fixed. Regarding export of deleted items from a first glance I can see how you would expect this just as matter of symmetry with the regular export, but both conceptually and for lots of other reasons it makes no sense at all. 1. Exporting deleted records is probably never done to save the record information somewhere else, but to use data in records to match a record somewhere else to remove. I cannot imagine a case where this data would be found on an item-level. 2. It would require us to refactor C4::Biblio::EmbedItemsInMarcBiblio just for this specific case (which is an edge case that in practice will probably never be utilized), further increasing the burden of code maintenance and risk of introducing new bugs. If I'm incorrect and this feature is requested by many users it's always possible to implement later, but I would be very surprised if this turns out to be the case. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Cab Vinton <bibliwho@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bibliwho@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116408|0 |1 is obsolete| | --- Comment #42 from David Gustafsson <glasklas@gmail.com> --- Created attachment 125211 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125211&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116409|0 |1 is obsolete| | --- Comment #43 from David Gustafsson <glasklas@gmail.com> --- Created attachment 125212 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125212&action=edit Bug 20551: Fix option name in pod Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116410|0 |1 is obsolete| | --- Comment #44 from David Gustafsson <glasklas@gmail.com> --- Created attachment 125213 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125213&action=edit Bug 20551: Rename "marcflavour" field to "schema" Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116411|0 |1 is obsolete| | --- Comment #45 from David Gustafsson <glasklas@gmail.com> --- Created attachment 125214 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125214&action=edit Bug 20551: Improve documentation and make compatible with more options Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123101|0 |1 is obsolete| | --- Comment #46 from David Gustafsson <glasklas@gmail.com> --- Created attachment 125215 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125215&action=edit Bug 20551: Add missing subroutine import -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123102|0 |1 is obsolete| | --- Comment #47 from David Gustafsson <glasklas@gmail.com> --- Created attachment 125216 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125216&action=edit Bug 20551: Fix tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #48 from David Gustafsson <glasklas@gmail.com> --- Rebase against master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> 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=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125211|0 |1 is obsolete| | --- Comment #49 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 128895 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128895&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125212|0 |1 is obsolete| | --- Comment #50 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 128896 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128896&action=edit Bug 20551: Fix option name in pod Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125213|0 |1 is obsolete| | --- Comment #51 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 128897 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128897&action=edit Bug 20551: Rename "marcflavour" field to "schema" Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125214|0 |1 is obsolete| | --- Comment #52 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 128898 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128898&action=edit Bug 20551: Improve documentation and make compatible with more options Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125215|0 |1 is obsolete| | --- Comment #53 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 128899 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128899&action=edit Bug 20551: Add missing subroutine import -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Frank Hansen <frank.hansen@ub.lu.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125216|0 |1 is obsolete| | --- Comment #54 from Frank Hansen <frank.hansen@ub.lu.se> --- Created attachment 128900 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128900&action=edit Bug 20551: Fix tests Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Works fine for me. Signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #55 from Cab Vinton <bibliwho@gmail.com> --- Will this enhancement be available from the staff interface? i.e., Tools > Export data (.../cgi-bin/koha/tools/export.pl) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #56 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Cab Vinton from comment #55)
Will this enhancement be available from the staff interface? i.e., Tools > Export data (.../cgi-bin/koha/tools/export.pl)
From looking at the patch set this will only appear in the command line script. But as this has been waiting a while now, I suggest we file a separate report for making this available on GUI side. What do you think?
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #57 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + my $marc_flavour = C4::Context->preference('marcflavour'); + my $biblio_metadata = $resultset->find({ + 'biblionumber' => $biblionumber, + 'format' => 'marcxml', + 'marcflavour' => $marc_flavour + }); + my $marc_xml = $biblio_metadata->metadata; + $marc_xml = StripNonXmlChars($marc_xml); + + my $record = eval { + MARC::Record::new_from_xml($marc_xml, 'UTF-8', $marc_flavour) + }; + if (!$record) { You test $record, but you should have tested $biblio_metadata too ? Otherwise you already crash on ->metadata.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #58 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + return unless (@{$record_ids} || @{$deleted_record_ids} && $format ne 'csv'); Parentheses here would be more helpful inside. The outer ones are not needed. But in order to read easier, you could do A || ( B && C ) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #59 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- sub export binmode $fh, ':encoding(UTF-8)' unless $format eq 'csv'; } else { binmode STDOUT, ':encoding(UTF-8)' unless $format eq 'csv'; What happens if we choose csv? Are we not doing UTF-8 ? Later we do: print marc2csv( $record_ids, $csv_profile_id, $itemnumbers ); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #60 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #58)
+ return unless (@{$record_ids} || @{$deleted_record_ids} && $format ne 'csv');
Parentheses here would be more helpful inside. The outer ones are not needed. But in order to read easier, you could do A || ( B && C )
And functionally: why not deleted records in csv ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #61 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + my ($record_type) = @_; + my $validator = $encoding_validator->('Record'); + $validator = $encoding_validator->('Deleted record'); Deleted flag ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #62 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- - my $record = _get_record_for_export( { %$params, record_id => $record_id } ); + for my $record (@records, @deleted_records) { + print MARC::File::XML::record($record); Noting that get_record_for_export does a lot more and we seem to loose that now? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #63 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Ok, not all my questions have the same importance and I realize that this dev is already too long in the pipeline, but I like to have feedback now. Changing status to reflect that -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #64 from Cab Vinton <bibliwho@gmail.com> --- (In reply to Katrin Fischer from comment #56)
From looking at the patch set this will only appear in the command line script. But as this has been waiting a while now, I suggest we file a separate report for making this available on GUI side. What do you think?
I missed this earlier, but Yes, a separate report for the GUI side makes sense. I would wait until this one is fully signed off, but others are welcome to proceed if they wish. Not sure how prevalent this is, but I believe quite a few union catalogs still ask contributing members to regularly send updates w/ lists of records added & deleted. So this feature would come in handy for all the folks who don't have access to the command line. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #65 from David Gustafsson <glasklas@gmail.com> --- (In reply to Marcel de Rooy from comment #57)
+ my $marc_flavour = C4::Context->preference('marcflavour'); + my $biblio_metadata = $resultset->find({ + 'biblionumber' => $biblionumber, + 'format' => 'marcxml', + 'marcflavour' => $marc_flavour + }); + my $marc_xml = $biblio_metadata->metadata; + $marc_xml = StripNonXmlChars($marc_xml); + + my $record = eval { + MARC::Record::new_from_xml($marc_xml, 'UTF-8', $marc_flavour) + }; + if (!$record) {
You test $record, but you should have tested $biblio_metadata too ? Otherwise you already crash on ->metadata..
Hi, thanks for the feedback. There is no definite right or wrong, but personally try to avoid defensive programming where things are very unlikely to fail. Ids should be valid since have been previously fetched from database, in case of the XML-decoding it's highly unlikely to go wrong, but I can imagine cases where it could. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #66 from David Gustafsson <glasklas@gmail.com> --- (In reply to Marcel de Rooy from comment #57)
+ my $marc_flavour = C4::Context->preference('marcflavour'); + my $biblio_metadata = $resultset->find({ + 'biblionumber' => $biblionumber, + 'format' => 'marcxml', + 'marcflavour' => $marc_flavour + }); + my $marc_xml = $biblio_metadata->metadata; + $marc_xml = StripNonXmlChars($marc_xml); + + my $record = eval { + MARC::Record::new_from_xml($marc_xml, 'UTF-8', $marc_flavour) + }; + if (!$record) {
You test $record, but you should have tested $biblio_metadata too ? Otherwise you already crash on ->metadata..
Hi, thanks for the feedback. There is no definite right or wrong, but personally try to avoid defensive programming where things are very unlikely to fail. Ids should be valid since have been previously fetched from database, in case of the XML-decoding it's highly unlikely to go wrong, but I can imagine cases where it could.(In reply to Marcel de Rooy from comment #58)
+ return unless (@{$record_ids} || @{$deleted_record_ids} && $format ne 'csv');
Parentheses here would be more helpful inside. The outer ones are not needed. But in order to read easier, you could do A || ( B && C )
Yes,they could be removed. Personally I think it's more confusing, or at least a bit unsightly, to use parenthesis for anything else than changing operator precedence. At least with "||" and "&&" where there really should be no confusion. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #67 from David Gustafsson <glasklas@gmail.com> --- (In reply to Marcel de Rooy from comment #59)
sub export
binmode $fh, ':encoding(UTF-8)' unless $format eq 'csv'; } else { binmode STDOUT, ':encoding(UTF-8)' unless $format eq 'csv';
What happens if we choose csv? Are we not doing UTF-8 ? Later we do:
print marc2csv( $record_ids, $csv_profile_id, $itemnumbers );
I had a look at this and don't think this block is part of the patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #68 from David Gustafsson <glasklas@gmail.com> --- (In reply to Marcel de Rooy from comment #59)
sub export
binmode $fh, ':encoding(UTF-8)' unless $format eq 'csv'; } else { binmode STDOUT, ':encoding(UTF-8)' unless $format eq 'csv';
What happens if we choose csv? Are we not doing UTF-8 ? Later we do:
print marc2csv( $record_ids, $csv_profile_id, $itemnumbers );
marc2csv uses GetMarcBiblio to load biblios that does not support loading deleted biblios which resides in another table. marc2csv would have to be carefully reviewed and refactored to ensure it can handle deleted biblios, this would increase the scope of this patch and I just did not think it was worth the effort. A separate issues could perhaps be created in case there is an interest implementing support for this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #69 from David Gustafsson <glasklas@gmail.com> --- Sorry, the last comment was meant to be a response to:
And functionally: why not deleted records in csv ?
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #70 from David Gustafsson <glasklas@gmail.com> --- (In reply to Marcel de Rooy from comment #61)
+ my ($record_type) = @_;
+ my $validator = $encoding_validator->('Record');
+ $validator = $encoding_validator->('Deleted record');
Deleted flag ?
I don't think I understand. The string is correct, but looking at the code I wonder why creating a closure instead of just providing the argument further down, I should probably change this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #71 from David Gustafsson <glasklas@gmail.com> --- (In reply to Marcel de Rooy from comment #62)
- my $record = _get_record_for_export( { %$params, record_id => $record_id } );
+ for my $record (@records, @deleted_records) { + print MARC::File::XML::record($record);
Noting that get_record_for_export does a lot more and we seem to loose that now?
Yes, right now I can't think of any reason why deleted records should not be a new record type in _get_record_for_export and processed in the same way as the others (authorities and biblios). I will look into this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #72 from David Gustafsson <glasklas@gmail.com> --- (_get_record_for_export is still called to fetch the other records though, so no functionality has been removed there, but reading what it does don't se how most of the same operations should not work also on deleted biblios) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #73 from David Gustafsson <glasklas@gmail.com> --- Created attachment 132936 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=132936&action=edit Bug 20551: Fetch deleted biblios through _get_record_export -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #74 from David Gustafsson <glasklas@gmail.com> --- Created attachment 132937 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=132937&action=edit Bug 20551: Refactor unmotivated partially applied validator function -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #75 from David Gustafsson <glasklas@gmail.com> --- Thanks again for the feedback, I think at least the most important issues should now be fixed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |pedro.amorim@ptfs-europe.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #76 from David Gustafsson <glasklas@gmail.com> --- Must have forgotten to change status, setting to Needs Signoff. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Baptiste <baptiste.bayche@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |baptiste.bayche@inlibro.com Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #77 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- (In reply to David Gustafsson from comment #76)
Must have forgotten to change status, setting to Needs Signoff.
Hi David, The patches do not apply anymore but I'd be happy to signoff if you rebase it on current main :) Best, Arthur -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128895|0 |1 is obsolete| | Attachment #128896|0 |1 is obsolete| | Attachment #128897|0 |1 is obsolete| | Attachment #128898|0 |1 is obsolete| | Attachment #128899|0 |1 is obsolete| | Attachment #128900|0 |1 is obsolete| | Attachment #132936|0 |1 is obsolete| | Attachment #132937|0 |1 is obsolete| | --- Comment #78 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 176623 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176623&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #79 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 176624 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176624&action=edit Bug 20551: Fix option name in pod Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #80 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 176625 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176625&action=edit Bug 20551: Rename "marcflavour" field to "schema" Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #81 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 176626 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176626&action=edit Bug 20551: Improve documentation and make compatible with more options Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #82 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 176627 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176627&action=edit Bug 20551: Add missing subroutine import -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #83 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 176628 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176628&action=edit Bug 20551: Fix tests Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Works fine for me. Signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #84 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 176629 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176629&action=edit Bug 20551: Fetch deleted biblios through _get_record_export -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #85 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 176630 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176630&action=edit Bug 20551: Refactor unmotivated partially applied validator function -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #86 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 176631 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176631&action=edit Bug 20551: (QA follow-up) perltidy and test fixes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #87 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Just made a rebase a small patch to make the QA scripts happy. Arthur -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #88 from David Gustafsson <glasklas@gmail.com> --- Created attachment 177536 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177536&action=edit Bug 20551: Move some code that might be reused into functions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #89 from Fridolin Somers <fridolin.somers@biblibre.com> --- Ohhhh there is an issue when biblio.biblionumber is on control field like 001 : my ($id_tag, $id_subfield) = GetMarcFromKohaField('biblio.biblionumber', ''); my $record_id = $record->subfield($id_tag, $id_subfield); Fails with : Control fields (generally, just tags below 010) do not have subfields, use data() at /usr/share/perl5/MARC/Record.pm line 240. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #90 from Fridolin Somers <fridolin.somers@biblibre.com> --- Maybe when record can not be encoded we should not try to read its content -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176623|0 |1 is obsolete| | --- Comment #91 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178343 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178343&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176624|0 |1 is obsolete| | --- Comment #92 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178344 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178344&action=edit Bug 20551: Fix option name in pod Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176625|0 |1 is obsolete| | --- Comment #93 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178345 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178345&action=edit Bug 20551: Rename "marcflavour" field to "schema" Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176626|0 |1 is obsolete| | --- Comment #94 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178346 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178346&action=edit Bug 20551: Improve documentation and make compatible with more options Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176627|0 |1 is obsolete| | --- Comment #95 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178347 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178347&action=edit Bug 20551: Add missing subroutine import -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176628|0 |1 is obsolete| | --- Comment #96 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178348 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178348&action=edit Bug 20551: Fix tests Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Works fine for me. Signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176629|0 |1 is obsolete| | --- Comment #97 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178349 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178349&action=edit Bug 20551: Fetch deleted biblios through _get_record_export -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176630|0 |1 is obsolete| | --- Comment #98 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178350 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178350&action=edit Bug 20551: Refactor unmotivated partially applied validator function -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176631|0 |1 is obsolete| | --- Comment #99 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178351 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178351&action=edit Bug 20551: (QA follow-up) perltidy and test fixes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177536|0 |1 is obsolete| | --- Comment #100 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178352 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178352&action=edit Bug 20551: Move some code that might be reused into functions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #101 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 178353 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178353&action=edit Bug 20551: (QA follow-up) don't read record content if there is an error during decoding -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #102 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- just made a rebase using autorebase script + small patch in reply to fridolin's comment -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #103 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178378 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178378&action=edit Bug 20551: Get biblionumber safly in validator function -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178378|0 |1 is obsolete| | --- Comment #104 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178379&action=edit Bug 20551: Get biblionumber safely in validator function -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #105 from David Gustafsson <glasklas@gmail.com> --- (In reply to Arthur Suzuki from comment #102)
just made a rebase using autorebase script + small patch in reply to fridolin's comment
Thanks for the rebase. As it's useful to still get the biblionumber and not that hard to fix I restored including it in the error message, but now done so will not crash if not stored in a subfield. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #106 from Fridolin Somers <fridolin.somers@biblibre.com> --- I think for performance, this call should be outside validator : my ( $id_tag, $id_code ) = GetMarcFromKohaField( 'biblio.biblionumber', '' ); Because validator is called in the loop. We have some use of export more that 1 million records :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #107 from Fridolin Somers <fridolin.somers@biblibre.com> --- Arg I've been using this feature on an a big export and DB server crashed. I see the output file is not filled little by little. I think it comes from this change : + @records = map { + my $record = _get_record_for_export( { %{$params}, record_id => $_ } ); + $record ? $record : (); + } @{$record_ids}; For a huge export the @records object will take too much RAM. Without this path the records are build one by one. Please make some tests to confirm. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #108 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178575 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178575&action=edit Bug 20551: Use iterator for retrieving records to save memory -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178575|0 |1 is obsolete| | --- Comment #109 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178617 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178617&action=edit Bug 20551: Use iterator for retrieving records to save memory -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #110 from David Gustafsson <glasklas@gmail.com> --- (In reply to Fridolin Somers from comment #107)
Arg I've been using this feature on an a big export and DB server crashed. I see the output file is not filled little by little.
I think it comes from this change :
+ @records = map { + my $record = _get_record_for_export( { %{$params}, record_id => $_ } ); + $record ? $record : (); + } @{$record_ids};
For a huge export the @records object will take too much RAM.
Without this path the records are build one by one.
Please make some tests to confirm.
Good catch, didn't think about that aspect, I fixed this by using an iterator instead. I don't think we need to think about the performance cost of running GetMarcFromKohaField multiple times as it's only called for records with encoding errors, which should be a very small proportion of the total records. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178617|0 |1 is obsolete| | --- Comment #111 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178618 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178618&action=edit Bug 20551: Use iterator for retrieving records to save memory -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178618|0 |1 is obsolete| | --- Comment #112 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178619 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178619&action=edit Bug 20551: Use iterator for retrieving records to save memory -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #113 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Hello, In some condition the code could fail : in the sub _get_deleted_biblio_for_export : ---- # Creating schema is expensive, allow caller to # pass it so don't have to recreate for each call my $resultset = $params->{resultset} || Koha::Database->new()->schema()->resultset('DeletedbiblioMetadata'); my $marc_flavour = C4::Context->preference('marcflavour'); my $biblio_metadata = $resultset->find( { 'biblionumber' => $biblionumber, 'format' => 'marcxml', 'schema' => $marc_flavour } ); my $marc_xml = $biblio_metadata->metadata; $marc_xml = StripNonXmlChars($marc_xml); ---- It seems that an empty result set could lead to querying an empty $biblio_metadata object, resulting in a crash. Probably should warn about "no deleted records" and early return in case of empty resultset? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #114 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Also I had some errors in the record_iterator sub. Fixed by explicitely using $deleted_record_id instead of $_ : my $records_iterator = sub { if (my $record_id = pop(@record_ids)) { return _get_record_for_export( { %{$params}, record_id => $record_id } ); } elsif (my $deleted_record_id = pop(@deleted_record_ids)) { return _get_record_for_export( { %{$params}, record_type => 'deleted_bibs', - record_id => $_, + record_id => $deleted_record_id, resultset => $deleted_resultset } ); } return; }; I guess the previous error was due to no biblionumber being passed to the _get_record_for_export sub. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178619|0 |1 is obsolete| | --- Comment #115 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178679&action=edit Bug 20551: Use iterator for retrieving records to save memory -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #116 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178680&action=edit Bug 20551: Cache resultset instead of passing to sub -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #117 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178681 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178681&action=edit Bug 20551: Warn and return if deleted biblio not found -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #118 from David Gustafsson <glasklas@gmail.com> --- Created attachment 178682 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178682&action=edit Bug 20551: Use timetstamp of deletedbiblio_metadata instead of deletedbibio -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #119 from David Gustafsson <glasklas@gmail.com> --- (In reply to Arthur Suzuki from comment #114)
Also I had some errors in the record_iterator sub. Fixed by explicitely using $deleted_record_id instead of $_ :
my $records_iterator = sub { if (my $record_id = pop(@record_ids)) { return _get_record_for_export( { %{$params}, record_id => $record_id } ); } elsif (my $deleted_record_id = pop(@deleted_record_ids)) { return _get_record_for_export( { %{$params}, record_type => 'deleted_bibs', - record_id => $_, + record_id => $deleted_record_id, resultset => $deleted_resultset } ); } return; };
I guess the previous error was due to no biblionumber being passed to the _get_record_for_export sub.
I don't think that could occur as the deleted biblionumber is always retrieved from the database before and thus always exists. But I agree it should be added to be consistent with (In reply to Arthur Suzuki from comment #113)
Hello, In some condition the code could fail : in the sub _get_deleted_biblio_for_export : ---- # Creating schema is expensive, allow caller to
# pass it so don't have to recreate for each call
my $resultset = $params->{resultset} || Koha::Database->new()->schema()->resultset('DeletedbiblioMetadata'); my $marc_flavour = C4::Context->preference('marcflavour'); my $biblio_metadata = $resultset->find( { 'biblionumber' => $biblionumber, 'format' => 'marcxml', 'schema' => $marc_flavour } ); my $marc_xml = $biblio_metadata->metadata; $marc_xml = StripNonXmlChars($marc_xml); ----
It seems that an empty result set could lead to querying an empty $biblio_metadata object, resulting in a crash. Probably should warn about "no deleted records" and early return in case of empty resultset?
I don't think that could occur as the deleted biblionumber is always retrieved from the database before and thus always exists. But I agree it should be added to be consistent with _get_biblio_for_export and future proofing the code. You are right it was triggered by me forgetting to replace $_ with $deleted_record_id in the previous patch. I have fixed this now. Also changed the table deleted biblios is selected from in export_records.pl from deletedbiblio to deletedbiblio_metadata as biblio_metadata is used for non deleted biblios. The deleted biblios resultset is now also cached instead of passed as an argument. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #120 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Tried to apply patches to current master but got the tests failing : ''' kohadev-koha@kohadevbox:koha(main)$ prove t/db_dependent/Exporter/Record.t t/db_dependent/Exporter/Record.t .. 1/7 # Failed test 'Export XML: 3 records should have been exported' # at t/db_dependent/Exporter/Record.t line 200. # got: '1' # expected: '3' # Looks like you planned 4 tests but ran 2. # Looks like you failed 1 test of 2 run. # Failed test 'export xml' # at t/db_dependent/Exporter/Record.t line 211. Can't call method "subfield" on an undefined value at t/db_dependent/Exporter/Record.t line 202, <$fh> line 3. # Looks like your test exited with 11 just after 3. t/db_dependent/Exporter/Record.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 5/7 subtests Test Summary Report ------------------- t/db_dependent/Exporter/Record.t (Wstat: 2816 (exited 11) Tests: 3 Failed: 1) Failed test: 2 Non-zero exit status: 11 Parse errors: Bad plan. You planned 7 tests but ran 3. Files=1, Tests=3, 4 wallclock secs ( 0.03 usr 0.01 sys + 3.58 cusr 0.33 csys = 3.95 CPU) Result: FAIL ''' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #121 from David Gustafsson <glasklas@gmail.com> --- Created attachment 179748 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179748&action=edit Bug 20551: Return 0 instead of undef from _get_record_for_export not to break iterator loop -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #122 from David Gustafsson <glasklas@gmail.com> --- (In reply to Arthur Suzuki from comment #120)
Tried to apply patches to current master but got the tests failing : ''' kohadev-koha@kohadevbox:koha(main)$ prove t/db_dependent/Exporter/Record.t t/db_dependent/Exporter/Record.t .. 1/7 # Failed test 'Export XML: 3 records should have been exported' # at t/db_dependent/Exporter/Record.t line 200. # got: '1' # expected: '3' # Looks like you planned 4 tests but ran 2. # Looks like you failed 1 test of 2 run.
# Failed test 'export xml' # at t/db_dependent/Exporter/Record.t line 211. Can't call method "subfield" on an undefined value at t/db_dependent/Exporter/Record.t line 202, <$fh> line 3. # Looks like your test exited with 11 just after 3. t/db_dependent/Exporter/Record.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 5/7 subtests
Test Summary Report ------------------- t/db_dependent/Exporter/Record.t (Wstat: 2816 (exited 11) Tests: 3 Failed: 1) Failed test: 2 Non-zero exit status: 11 Parse errors: Bad plan. You planned 7 tests but ran 3. Files=1, Tests=3, 4 wallclock secs ( 0.03 usr 0.01 sys + 3.58 cusr 0.33 csys = 3.95 CPU) Result: FAIL '''
Thanks for spotting this! The issue should now be fixed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #123 from David Gustafsson <glasklas@gmail.com> --- Created attachment 179749 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179749&action=edit Bug 20551: Return 0 instead of undef from _get_deleted_record_for_export -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> 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=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178343|0 |1 is obsolete| | --- Comment #124 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180648 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180648&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178344|0 |1 is obsolete| | --- Comment #125 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180649 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180649&action=edit Bug 20551: Fix option name in pod Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178345|0 |1 is obsolete| | --- Comment #126 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180650 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180650&action=edit Bug 20551: Rename "marcflavour" field to "schema" Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178346|0 |1 is obsolete| | --- Comment #127 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180651 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180651&action=edit Bug 20551: Improve documentation and make compatible with more options Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178347|0 |1 is obsolete| | --- Comment #128 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180652 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180652&action=edit Bug 20551: Add missing subroutine import Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178348|0 |1 is obsolete| | --- Comment #129 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180653 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180653&action=edit Bug 20551: Fix tests Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Works fine for me. Signing off Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178349|0 |1 is obsolete| | --- Comment #130 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180654 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180654&action=edit Bug 20551: Fetch deleted biblios through _get_record_export Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178350|0 |1 is obsolete| | --- Comment #131 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180655 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180655&action=edit Bug 20551: Refactor unmotivated partially applied validator function Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178351|0 |1 is obsolete| | --- Comment #132 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180656 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180656&action=edit Bug 20551: (QA follow-up) perltidy and test fixes Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178352|0 |1 is obsolete| | --- Comment #133 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180657 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180657&action=edit Bug 20551: Move some code that might be reused into functions Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178353|0 |1 is obsolete| | --- Comment #134 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180658 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180658&action=edit Bug 20551: (QA follow-up) don't read record content if there is an error during decoding Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178379|0 |1 is obsolete| | --- Comment #135 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180659 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180659&action=edit Bug 20551: Get biblionumber safely in validator function Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178679|0 |1 is obsolete| | --- Comment #136 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180660 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180660&action=edit Bug 20551: Use iterator for retrieving records to save memory Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178680|0 |1 is obsolete| | --- Comment #137 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180661 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180661&action=edit Bug 20551: Cache resultset instead of passing to sub Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178681|0 |1 is obsolete| | --- Comment #138 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180662 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180662&action=edit Bug 20551: Warn and return if deleted biblio not found Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #178682|0 |1 is obsolete| | --- Comment #139 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180663 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180663&action=edit Bug 20551: Use timetstamp of deletedbiblio_metadata instead of deletedbibio Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #179748|0 |1 is obsolete| | --- Comment #140 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180664 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180664&action=edit Bug 20551: Return 0 instead of undef from _get_record_for_export not to break iterator loop Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #179749|0 |1 is obsolete| | --- Comment #141 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 180665 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180665&action=edit Bug 20551: Return 0 instead of undef from _get_deleted_record_for_export Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |andrew@bywatersolutions.com |y.org | CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|andrew@bywatersolutions.com |testopia@bugs.koha-communit | |y.org --- Comment #142 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Bugzilla tells me I was the one who signed this off back in April. Removing myself as QA contact. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply --- Comment #143 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi David, this no longer applies cleanly, could you please have a go at a rebase? Thank you! pply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 20551: Add option for including deleted records in export_records.pl Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 20551: Fix option name in pod Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 20551: Rename "marcflavour" field to "schema" Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 20551: Improve documentation and make compatible with more options Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 20551: Add missing subroutine import Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 20551: Fix tests Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 20551: Fetch deleted biblios through _get_record_export Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 20551: Refactor unmotivated partially applied validator function Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 20551: (QA follow-up) perltidy and test fixes Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. Applying: Bug 20551: Move some code that might be reused into functions error: sha1 information is lacking or useless (Koha/Exporter/Record.pm). error: could not build fake ancestor Patch failed at 0001 Bug 20551: Move some code that might be reused into functions hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-20551-Move-some-code-that-might-be-reused-into-0dievlan.patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180648|0 |1 is obsolete| | --- Comment #144 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193636 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193636&action=edit Bug 20551: Add option for including deleted records in export_records.pl Add option "--include_deleted" to include deleted biblios in export_records.pl as marc records with record status (in leader) set to "d" and "--deleted_only" to export only deleted biblios. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass 3) Delete a biblio record in Koha 4) Run the export script as: `export_records.pl --date=<date-just-before-deletion> --include_deleted` 5) Open the generated koha.mrc, and verify that contains the recently deleted record, and that the record header has record status "d". 6) Perform the same check with: `export_records.pl --date=<date-just-before-deletion> --include_deleted --format=xml` 7) Run: `export_records.pl --date=<date-just-before-deletion> --deleted_only --format=xml` and verify that only the deleted biblio was included in the export. Sponsored-by: Gothenburg University Library Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180649|0 |1 is obsolete| | --- Comment #145 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193637 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193637&action=edit Bug 20551: Fix option name in pod Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180650|0 |1 is obsolete| | --- Comment #146 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193638 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193638&action=edit Bug 20551: Rename "marcflavour" field to "schema" Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180651|0 |1 is obsolete| | --- Comment #147 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193639&action=edit Bug 20551: Improve documentation and make compatible with more options Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180652|0 |1 is obsolete| | --- Comment #148 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193640 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193640&action=edit Bug 20551: Add missing subroutine import Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180653|0 |1 is obsolete| | --- Comment #149 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193641 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193641&action=edit Bug 20551: Fix tests Signed-off-by: Frank Hansen <frank.hansen@ub.lu.se> Works fine for me. Signing off Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180654|0 |1 is obsolete| | --- Comment #150 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193642 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193642&action=edit Bug 20551: Fetch deleted biblios through _get_record_export Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180655|0 |1 is obsolete| | --- Comment #151 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193643 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193643&action=edit Bug 20551: Refactor unmotivated partially applied validator function Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180656|0 |1 is obsolete| | --- Comment #152 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193644 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193644&action=edit Bug 20551: (QA follow-up) perltidy and test fixes Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180657|0 |1 is obsolete| | --- Comment #153 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193645 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193645&action=edit Bug 20551: Move some code that might be reused into functions Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180658|0 |1 is obsolete| | --- Comment #154 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193646 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193646&action=edit Bug 20551: (QA follow-up) don't read record content if there is an error during decoding Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180659|0 |1 is obsolete| | --- Comment #155 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193647 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193647&action=edit Bug 20551: Get biblionumber safely in validator function Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180660|0 |1 is obsolete| | --- Comment #156 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193648 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193648&action=edit Bug 20551: Use iterator for retrieving records to save memory Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180661|0 |1 is obsolete| | --- Comment #157 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193649 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193649&action=edit Bug 20551: Cache resultset instead of passing to sub Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180662|0 |1 is obsolete| | --- Comment #158 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193650 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193650&action=edit Bug 20551: Warn and return if deleted biblio not found Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180663|0 |1 is obsolete| | --- Comment #159 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193651 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193651&action=edit Bug 20551: Use timetstamp of deletedbiblio_metadata instead of deletedbibio Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180664|0 |1 is obsolete| | --- Comment #160 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193652 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193652&action=edit Bug 20551: Return 0 instead of undef from _get_record_for_export not to break iterator loop Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180665|0 |1 is obsolete| | --- Comment #161 from David Gustafsson <glasklas@gmail.com> --- Created attachment 193653 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193653&action=edit Bug 20551: Return 0 instead of undef from _get_deleted_record_for_export Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #162 from David Gustafsson <glasklas@gmail.com> --- Rebased against main. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 --- Comment #163 from Fridolin Somers <fridolin.somers@biblibre.com> --- This is signed-off by Andrew Needs another SO or can it be QA ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Cab Vinton <bibliwho@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|bibliwho@gmail.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alexbuckley@catalyst.net.nz Keywords| |rel_26_11_candidate Priority|P5 - low |P4 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org