Description
Ere Maijala
2018-03-19 13:18:51 UTC
*** Bug 2797 has been marked as a duplicate of this bug. *** Created attachment 73703 [details] [review] Initial holdings support Created attachment 73704 [details] [review] Bug 20447: MARC Holdings support Initial development is complete. Here's a full patch for the brave to try out. You will need to run dbix schema update and the database update. Without changing settings there should be no change in behavior. Holdings support is enabled only by setting preference SummaryHoldings to "Use". I consider this quite feature-complete apart from the following: - terminal utilities for importing and exporting holdings are still missing - some tests should be added - existing tests should be verified - look and feel may need refinement Created attachment 76485 [details] [review] Bug 20447: MARC Holdings support Created attachment 76542 [details] [review] Bug 20447: MARC Holdings support Created attachment 76670 [details]
Sample MARCXML file containing biblios with interleaved holdings
Created attachment 76671 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 76672 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 76735 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 76736 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 78178 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 78179 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 78180 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Created attachment 78738 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 78739 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 78740 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Note that what's currently displayed in OPAC from holdings is pretty basic and can be extended as necessary. What's displayed is controlled by the framework, and the fields currently available are public_note, summary, supplements and indexes. Created attachment 78907 [details] [review] Bug 20447: Ignore holdings records in staff UI when staging records Importing holdings is not a typical action and can be done with bulk import if necessary. The patch appies fine. I have been able to import the marxcml file I have been able to export the marcxml file I am able to turn on summary holding on syspref I am able to see the marc holdings on the intraner BUT I am not able to see the marc holdings at the opac I think the marc framework that works with holdings should be 'fixed' review mandatory fields and duplicables fields... I have duplicated 852 and I guett an error DBIx::Class::Schema::source(): Can't find source for HoldingsMetadata at /home/vagrant/kohaclone/Koha/Objects.pm line 341 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 PENDING CHECKING OAI thanks a lot for this patch Hugo, did you run the databse updates (perl installer/data/mysql/updatedatabase.pl) and DBIx class updates (perl misc/devel/update_dbix_class_files.pl ...)? That'll be required for things to work properly. I'm asking because I don't get an error with multiple 852 fields, and that's actually a repeatable field according to the standard. Created attachment 79403 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. Created attachment 79413 [details] [review] Bug 20447: Add missing fields to the holdings framework Adds fields 337, 338, 347, 5xx and 84x. Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 20447: MARC Holdings support Using index info to reconstruct a base tree... M Koha/Schema/Result/Deleteditem.pm M Koha/Schema/Result/Item.pm M installer/data/mysql/kohastructure.sql M koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt Auto-merging installer/data/mysql/kohastructure.sql Auto-merging Koha/Schema/Result/Item.pm CONFLICT (content): Merge conflict in Koha/Schema/Result/Item.pm Auto-merging Koha/Schema/Result/Deleteditem.pm CONFLICT (content): Merge conflict in Koha/Schema/Result/Deleteditem.pm Failed to merge in the changes. Patch failed at 0001 Bug 20447: MARC Holdings support The copy of the patch that failed is found in: /home/vagrant/kohaclone/.git/rebase-apply/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-20447-MARC-Holdings-support-B8w5Re.patch Created attachment 79565 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 79566 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 79567 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Created attachment 79568 [details] [review] Bug 20447: Ignore holdings records in staff UI when staging records Importing holdings is not a typical action and can be done with bulk import if necessary. Created attachment 79569 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. Created attachment 79570 [details] [review] Bug 20447: Add missing fields to the holdings framework Adds fields 337, 338, 347, 5xx and 84x. Updated the patches to remove schema changes. Please run misc/devel/update_dbix_class_files.pl to update the schema files when testing. Created attachment 79812 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 79813 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 79814 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Created attachment 79815 [details] [review] Bug 20447: Ignore holdings records in staff UI when staging records Importing holdings is not a typical action and can be done with bulk import if necessary. Created attachment 79816 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. I consolidated fixes into the patches. Latest patches include the following changes: - Make sure to write 1 or 0 to the suppress column - Take holdings timestamps into account in OAI-PMH Created attachment 79819 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. Created attachment 80320 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 80321 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 80322 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Created attachment 80323 [details] [review] Bug 20447: Ignore holdings records in staff UI when staging records Importing holdings is not a typical action and can be done with bulk import if necessary. Created attachment 80324 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. Latest version contains the following fixes: - Add missing 0xx fields to the holdings framework - Fix the documentation links (question marks) in the MARC editor - Fix title and breadcrumb when modifying a holdings record - Include more fields in the call number to display - Fix Items tab name when holdings are enabled - Handle holdings when moving items from a biblio to another Created attachment 80823 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 80824 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 80825 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Created attachment 80826 [details] [review] Bug 20447: Ignore holdings records in staff UI when staging records Importing holdings is not a typical action and can be done with bulk import if necessary. Created attachment 80827 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. The latest version adds ccode column to holdings for parity with items location fields. Also upgrade script no longer contains framework data duplicated from the default framework sql and after a lot of deliberation branch code has been moved from 852$a to 852$b and location from 852$b to 852$c by default so that 852$a can be reserved for an ISIL code or such. Created attachment 81264 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 81265 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 81266 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Created attachment 81267 [details] [review] Bug 20447: Ignore holdings records in staff UI when staging records Importing holdings is not a typical action and can be done with bulk import if necessary. Created attachment 81268 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. Created attachment 82669 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Created attachment 82670 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 82671 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Created attachment 82672 [details] [review] Bug 20447: Ignore holdings records in staff UI when staging records Importing holdings is not a typical action and can be done with bulk import if necessary. Created attachment 82673 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. Hi Ere, I tested patches and import seem be ok. But Test fails: t/db_dependent/Exporter/Record.t .. DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`holdings`, CONSTRAINT `holdings_ibfk_3` FOREIGN KEY (`holdingbranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) [for Statement "INSERT INTO holdings SET biblionumber = ?, biblioitemnumber = ?, frameworkcode = ?, holdingbranch = ?, location = ?, ccode = ?, callnumber = ?, suppress = ?, datecreated = NOW() " with ParamValues: 0='442', 1=441, 2='', 3="Location", 4=undef, 5=undef, 6="Tb | 1973", 7=0] at /home/vagrant/kohaclone/C4/Holdings.pm line 503. ERROR in _koha_add_holding INSERT INTO holdings SET biblionumber = ?, biblioitemnumber = ?, frameworkcode = ?, holdingbranch = ?, location = ?, ccode = ?, callnumber = ?, suppress = ?, datecreated = NOW() Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`holdings`, CONSTRAINT `holdings_ibfk_3` FOREIGN KEY (`holdingbranch`) REFERENCES `branches` (`branchcode`) ON UPDATE CASCADE) at /home/vagrant/kohaclone/C4/Holdings.pm line 509. DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`holdings_metadata`, CONSTRAINT `holdings_metadata_fk_1` FOREIGN KEY (`holding_id`) REFERENCES `holdings` (`holding_id`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO `holdings_metadata` ( `format`, `holding_id`, `marcflavour`, `metadata`) VALUES ( ?, ?, ?, ? )" with ParamValues: 0='marcxml', 1='442', 2='MARC21', 3="<?xml version="1.0" encoding="UTF-8"?> Can You give me feedback? Thank You Mike Created attachment 83550 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 83551 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 83552 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Created attachment 83553 [details] [review] Bug 20447: Ignore holdings records in staff UI when staging records Importing holdings is not a typical action and can be done with bulk import if necessary. Created attachment 83554 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. Hi Mike, and thanks for testing. I had accidentally broken the tests, should be fixed and improved now. Created attachment 83555 [details] [review] Bug 20447: MARC Holdings support https://bugs.koha-community.org/show_bug.cgi?id=20447 Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83556 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83557 [details] [review] Bug 20447: Include basic summary holdings in the OAI-PMH responses. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 If include_items is set in oai conf, also basic location information from holdings records is included in the returned MARC records. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83558 [details] [review] Bug 20447: Ignore holdings records in staff UI when staging records Importing holdings is not a typical action and can be done with bulk import if necessary. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 83559 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework, and that's disabled in the UI so it must be done in the database. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> So, started the first QA run ;) First of all, I really like this enhancement and would to see it in Koha. But this need some more work I think, first some not critical and easy to fix: - System preference SummaryHoldings is missing in db updates - Holdings marc framework is missing from db update - Schema files should be attached in its own patch - I do not like using subfield "V" as default for linking item to holding, uppercase letters for subfields are not possible in marc21 standard But, the main problem I see here is C4::Holdings module - C4 namespace is deprecated, and should not be used for new modules, you always should add the new modules into Koha namespace and use objects if possible and if it does make sense, of course. You already have Koha::Holdings and Koha::Holdings::Metadatas, so you should add new routines here (in the Koha::Objects(s) manner). See also bug 22144, that's the way the holdings, holding::metadatas and its marc record object should be linked together too. Ok, I'll revise. Created attachment 85364 [details]
Sample MARCXML file with interleaved holdings
Created attachment 85368 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Created attachment 85369 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 85370 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 85371 [details] [review] Bug 20447: DBIX schema changes Almost there. I missed addition of holdings framework to update scripts, so clearing status until I get that fixed. Created attachment 85416 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Created attachment 85417 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 85418 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 85419 [details] [review] Bug 20447: DBIX schema changes All QA comments should now be addressed. I've converted everything to Koha namespace and cleaned up the code heavily. There are still some references to existing code in C4 namespace, but that's difficult to avoid at the moment. I had to come up with a method for updating metadata, and I hope what I came up with is nice and clean, but it can still be adjusted especially when C4::Biblio is converted to keep them as similar as possible. Functional changes from previous versions: - Subfield 'k' is now used for linking from item to holdings record. While its position is not so nice in the item form, it was the only subfield available. - biblioitemnumber has been dropped from the holdings table since it wasn't used. One thing that could be done would be to decouple the MARC editor from addbiblio so that the code could be re-used. I'll see if that's something that could be done with not too high impact. I'll work in Bug 21280 to unify the MARC editors in biblios and authorities. I don't consider it blocking, but if it happened that it would be accepted, I would update the editor here to use the common stuff. Created attachment 85906 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Created attachment 85907 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 85908 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 85909 [details] [review] Bug 20447: DBIX schema changes Created attachment 85994 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 85995 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 85996 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 85997 [details] [review] Bug 20447: DBIX schema changes Why is there deleted_on on in both holdings and holdings_metadata tables? Why is it not enough to have it just in holdings? Comment on attachment 85994 [details] [review] Bug 20447: MARC Holdings support Review of attachment 85994 [details] [review]: ----------------------------------------------------------------- ::: C4/Biblio.pm @@ +1563,5 @@ > } > > + #---- holdings > + if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "holdings" ) { > + my $holding = Koha::Holdings->find( $value ); does the find need "deleted_on => undef" as a parameter in order to not care about deleted holdings? Comment on attachment 85994 [details] [review] Bug 20447: MARC Holdings support Review of attachment 85994 [details] [review]: ----------------------------------------------------------------- ::: C4/Items.pm @@ +1705,5 @@ > + holdingbranch => $oldHolding->{'holdingbranch'}, > + location => $oldHolding->{'location'}, > + callnumber => $oldHolding->{'callnumber'}, > + suppress => $oldHolding->{'suppress'}, > + deleted_on => undef if deleted_on is not needed / is undef by default this can be removed (In reply to Joonas Kylmälä from comment #97) > Why is there deleted_on on in both holdings and holdings_metadata tables? > Why is it not enough to have it just in holdings? Because that's how it's done also for biblios in bug 20271. There's at least one practical upside: it allows one to fetch a metadata record without joining with the base table. (In reply to Joonas Kylmälä from comment #98) > Comment on attachment 85994 [details] [review] [review] > Bug 20447: MARC Holdings support > > Review of attachment 85994 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: C4/Biblio.pm > @@ +1563,5 @@ > > } > > > > + #---- holdings > > + if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "holdings" ) { > > + my $holding = Koha::Holdings->find( $value ); > > does the find need "deleted_on => undef" as a parameter in order to not care > about deleted holdings? It allows one to fetch a holdings record by ID even if it's deleted. I believe it makes sense since otherwise it would be possible, at least in theory, for an item to point to a deleted holdings record without it showing. (In reply to Joonas Kylmälä from comment #99) > Comment on attachment 85994 [details] [review] [review] > Bug 20447: MARC Holdings support > > Review of attachment 85994 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: C4/Items.pm > @@ +1705,5 @@ > > + holdingbranch => $oldHolding->{'holdingbranch'}, > > + location => $oldHolding->{'location'}, > > + callnumber => $oldHolding->{'callnumber'}, > > + suppress => $oldHolding->{'suppress'}, > > + deleted_on => undef > > if deleted_on is not needed / is undef by default this can be removed It tries to find an existing holdings record for the item to be attached to. Deleted holdings do not qualify, so the filter is needed. However, I just realized that the search terms can match multiple records which would cause find() to fail. I need to revise that. (In reply to Ere Maijala from comment #100) > (In reply to Joonas Kylmälä from comment #97) > > Why is there deleted_on on in both holdings and holdings_metadata tables? > > Why is it not enough to have it just in holdings? > > Because that's how it's done also for biblios in bug 20271. There's at least > one practical upside: it allows one to fetch a metadata record without > joining with the base table. #1 rule in my database course was always use the normal form for databases. It feels wrong to have the data in one place: twice the possibility of having bugs and also taking twice as much space. I read quickly https://stackoverflow.com/questions/2623852/why-are-joins-bad-when-considering-scalability and in the end the performance difference might not be that big. I would like you to provide performance benchmarks if we are going to use the non-normalized way. (In reply to Joonas Kylmälä from comment #103) > #1 rule in my database course was always use the normal form for databases. > It feels wrong to have the data in one place: twice the possibility of > having bugs and also taking twice as much space. I read quickly > https://stackoverflow.com/questions/2623852/why-are-joins-bad-when- > considering-scalability and in the end the performance difference might not > be that big. I would like you to provide performance benchmarks if we are > going to use the non-normalized way. Here is a few pages of reasoning why we should use the normal form: https://mariadb.com/kb/en/library/database-normalization/ (unless there is a significant performance gain) I'm aware of that, but I'm not making the call here. I think it would be better to continue discussion about the deleted_on field in the metadata tables in bug 20271. Created attachment 86088 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 86089 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 86090 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 86091 [details] [review] Bug 20447: DBIX schema changes Fixed item relinking and made saving a holdings record reindex its biblio record. Ere, please rebase on master. Thank you. Auto-merging Koha/Schema/Result/Branch.pm CONFLICT (content): Merge conflict in Koha/Schema/Result/Branch.pm Auto-merging Koha/Schema/Result/Biblio.pm CONFLICT (content): Merge conflict in Koha/Schema/Result/Biblio.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 20447: DBIX schema changes (In reply to Michal Denar from comment #112) > Auto-merging Koha/Schema/Result/Branch.pm > CONFLICT (content): Merge conflict in Koha/Schema/Result/Branch.pm > Auto-merging Koha/Schema/Result/Biblio.pm > CONFLICT (content): Merge conflict in Koha/Schema/Result/Biblio.pm > error: Failed to merge in the changes. > Patch failed at 0001 Bug 20447: DBIX schema changes Mike, these files could be easily regeneretad, just omit this one patch and run command 'dbic' on kohadevbox Created attachment 87307 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 87308 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 87309 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 87310 [details] [review] Bug 20447: DBIX schema changes Rebased, but the schema patch is likely to break again.. Hi Ere, tests passed, but I'm not able to import attached files: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 0 MARC bibliographic records and 0 holdings records done in 0.012 seconds Any ideas? Thank you. A couple of simple checks first: check that you have holdings.xml in the current directory and that it's valid xml. bulkmarcimport doesn't have very good error checking and it will just report 0 records if it doesn't find the file or can't read it. For what it's worth, here are my results: $ xmllint -noout holdings.xml $ misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML ..... 5 MARC bibliographic records and 3 holdings records done in 1.711 seconds If you are still unable to make it work, I'll need to recheck with kohadevbox. Hi Ere, sorry my fault., no holdings.xml on devbox :-( I'll next round... Thanks to Josef for kick. Created attachment 87346 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 87347 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 87348 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 87349 [details] [review] Bug 20447: DBIX schema changes Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Comment on attachment 87346 [details] [review] Bug 20447: MARC Holdings support Review of attachment 87346 [details] [review]: ----------------------------------------------------------------- ::: C4/Biblio.pm @@ +2828,5 @@ > > $itemnumbers = [] unless defined $itemnumbers; > > + if ( !$skip_holdings && C4::Context->preference('SummaryHoldings') && !@$itemnumbers ) { > + require Koha::Holdings; You don't need this require ::: C4/Items.pm @@ +1690,5 @@ > AND biblionumber = ? > |, undef, $tobiblioitem, $tobiblio, $itemnumber, $frombiblio ); > if ($return == 1) { > + # Check if the moved item is attached to a holdings record > + my $item = my $item = Koha::Items->find($itemnumber); double "my $item" ::: C4/Search.pm @@ +2252,5 @@ > } > > + # Fetch summary holdings > + if (C4::Context->preference('SummaryHoldings')) { > + $summary_holdings = Koha::Holdings->search({ biblionumber => $oldbiblio->{biblionumber}, deleted_on => undef })->unblessed; why do you unbless here? ::: Koha/Holding.pm @@ +53,5 @@ > + my ($self) = @_; > + > + $self->{_metadata} ||= Koha::Holdings::Metadatas->find({ holding_id => $self->id }); > + > + return $self->{_metadata}; Should be changed, according to bug 22407, see https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL15:_Object-oriented_code_and_the_Koha::_namespace You should use something like my $metadata = $self->_result->metadata; return Koha::Holdings::Metadata->_new_from_dbic($metadata); @@ +111,5 @@ > + my ($self) = @_; > + > + $self->{_items} ||= Koha::Items->search( { holding_id => $self->holding_id() } ); > + > + return wantarray ? $self->{_items}->as_list : $self->{_items}; PERL15 rule applies here too ::: Koha/Template/Plugin/Holdings.pm @@ +71,5 @@ > + my $sth = C4::Context->dbh->prepare($query); > + $sth->execute($holding->{'holdingbranch'}); > + my $b = $sth->fetchrow_hashref(); > + push @parts, $b->{'branchname'} if $b; > + $sth->finish(); Why do you use sql instead of relation method like $holding->library ? ::: catalogue/detail.pl @@ +194,5 @@ > > +# Summary holdings > +my $summary_holdings; > +if (C4::Context->preference('SummaryHoldings')) { > + $summary_holdings = Koha::Holdings->search({ biblionumber => $biblionumber, deleted_on => undef })->unblessed; Why do you unbless? ::: t/db_dependent/Koha/Holding.t @@ +22,5 @@ > +use t::lib::TestBuilder; > + > +use C4::Biblio; > + > +use Koha::BiblioFramework; Singular class is imported through plural class, it is not needed to be used explicitaly. ::: t/db_dependent/Koha/Holdings.t @@ +22,5 @@ > +use t::lib::TestBuilder; > + > +use C4::Biblio; > + > +use Koha::BiblioFramework; Not needed to use singular class Started QA, found some first issues: When editing holding marc record, i am able to change framework to default and acquisition, is it intended? The holdings marc editor need to be upgraded to new style introduce by bug 22045. Could be done in own bug report or on bug 21280 I think. QA test tools report: FAIL C4/Biblio.pm FAIL valid "my" variable $item masks earlier declaration in same statement FAIL C4/ImportBatch.pm FAIL valid "my" variable $item masks earlier declaration in same statement FAIL C4/Items.pm FAIL valid "my" variable $item masks earlier declaration in same statement "my" variable $item masks earlier declaration in same statement FAIL C4/Search.pm FAIL valid "my" variable $item masks earlier declaration in same statement FAIL C4/XSLT.pm FAIL valid "my" variable $item masks earlier declaration in same statement FAIL Koha/Exporter/Record.pm FAIL valid "my" variable $item masks earlier declaration in same statement FAIL Koha/Holdings.pm FAIL valid "my" variable $item masks earlier declaration in same statement FAIL Koha/OAI/Server/ListBase.pm FAIL valid "my" variable $item masks earlier declaration in same statement FAIL Koha/OAI/Server/Repository.pm FAIL valid "my" variable $item masks earlier declaration in same statement FAIL Koha/Template/Plugin/Holdings.pm FAIL valid "my" variable $item masks earlier declaration in same statement FAIL admin/marc_subfields_structure.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL catalogue/detail.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL catalogue/showmarc.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL cataloguing/addholding.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL cataloguing/additem.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL cataloguing/value_builder/marc21_field_008_holdings.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL cataloguing/value_builder/marc21_leader_holdings.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc FAIL filters missing_filter at line 14 ( <li><a id="newholding" href="/cgi-bin/koha/cataloguing/addholding.pl?biblionumber=[% biblionumber %]#addholding">New holdings record</a></li>) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt FAIL filters missing_filter at line 210 ( <a class="btn btn-default btn-xs" href="/cgi-bin/koha/cataloguing/addholding.pl?op=edit&biblionumber=[% holding.biblionumber %]&holding_id=[% holding.holding_id %]#editholding"><i class="fa fa-pencil"></i> Edit</a>) missing_filter at line 210 ( <a class="btn btn-default btn-xs" href="/cgi-bin/koha/cataloguing/addholding.pl?op=edit&biblionumber=[% holding.biblionumber %]&holding_id=[% holding.holding_id %]#editholding"><i class="fa fa-pencil"></i> Edit</a>) missing_filter at line 211 ( <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/cataloguing/addholding.pl?op=delete&biblionumber=[% holding.biblionumber %]&holding_id=[% holding.holding_id %]"><i class="fa fa-eraser"></i> Delete</a>) missing_filter at line 211 ( <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/cataloguing/addholding.pl?op=delete&biblionumber=[% holding.biblionumber %]&holding_id=[% holding.holding_id %]"><i class="fa fa-eraser"></i> Delete</a>) missing_filter at line 212 ( <a class="btn btn-default btn-xs" href="/cgi-bin/koha/cataloguing/additem.pl?biblionumber=[% holding.biblionumber %]&holding_id=[% holding.holding_id %]#additema"><i class="fa fa-plus"></i> Add item</a>) missing_filter at line 212 ( <a class="btn btn-default btn-xs" href="/cgi-bin/koha/cataloguing/additem.pl?biblionumber=[% holding.biblionumber %]&holding_id=[% holding.holding_id %]#additema"><i class="fa fa-plus"></i> Add item</a>) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addholding.tt FAIL filters wrong_html_filter at line 358 ( › Edit <a href="/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=[% biblionumber | html %]">[% title | html %] [% IF ( author ) %] by [% author | html %][% END %] (Record #[% biblionumber | html %])</a>) wrong_html_filter at line 359 ( › <a href="/cgi-bin/koha/cataloguing/addholding.pl?biblionumber=[% biblionumber | html %]">Holdings</a>) wrong_html_filter at line 457 ( <li><a href="#tab[% BIG_LOO.number | html %]XX">[% BIG_LOO.number | html %]</a></li>) missing_filter at line 469 ( <a href="#" tabindex="1" class="tagnum" title="[% innerloo.tag_lib | html %] - Click to Expand this Tag" onclick="ExpandField('tag_[% innerloo.tag | html %]_[% innerloo.index | html %][% innerloo.random | html %]'); return false;">[% innerloo.tag %]</a>) missing_filter at line 471 ( <span class="tagnum" title="[% innerloo.tag_lib %]">[% innerloo.tag | html %]</span>) missing_filter at line 553 ( [% subfield_loo.marc_lib %]) missing_filter at line 578 ( [% mv.javascript %]) missing_filter at line 582 ( <textarea cols="70" rows="4" id="[%- mv.id | html -%]" name="[%- mv.name | html -%]" class="input_marceditor" tabindex="1">[%- mv.value -%]</textarea>) FAIL forbidden patterns forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 11) forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 376) FAIL js_in_body A <script> tag found inside head, must be moved to the body (see bug 17858) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/marc21_field_008_holdings.tt FAIL forbidden patterns forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 167) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/marc21_leader_holdings.tt FAIL forbidden patterns forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 83) FAIL misc/batchRebuildHoldingsTables.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL misc/export_records.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL misc/migration_tools/bulkmarcimport.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL opac/opac-detail.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL t/db_dependent/Exporter/Record.t FAIL valid "my" variable $item masks earlier declaration in same statement FAIL t/db_dependent/Koha/Holding.t FAIL valid "my" variable $item masks earlier declaration in same statement FAIL t/db_dependent/Koha/Holdings.t FAIL valid "my" variable $item masks earlier declaration in same statement FAIL tools/export.pl FAIL valid "my" variable $item masks earlier declaration in same statement FAIL tools/stage-marc-import.pl FAIL valid "my" variable $item masks earlier declaration in same statement Created attachment 88751 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 88752 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 88753 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 88754 [details] [review] Bug 20447: DBIX schema changes Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 88755 [details] [review] Bug 20447: Use blessed objects and fix issues found during review Thanks Josef, I believe I've addressed the comments in the latest patch. I'd rather leave editor unification to bug 21280 or a separate bug. Regarding changing the framework, I had in one version a new column for the framework type so that the selection list could be filtered, but I dropped it to keep the changes to minimum. That can be revisited later. I don't consider it a big issue. This patch set would break cataloguing/merge.pl since there is no patch to move holdings to a different bibliographic record. I have moved this back to "In Discussion" because I think this needs to be fixed before merging this patch set. (In reply to Joonas Kylmälä from comment #134) > This patch set would break cataloguing/merge.pl since there is no patch to > move holdings to a different bibliographic record. I have moved this back to > "In Discussion" because I think this needs to be fixed before merging this > patch set. Hi Joonas, thx for testing. Setting to Failed QA is better in this case. Joonas, was that an assumption or a verified problem? There's code in Items::MoveItemFromBiblio that does its best to handle holdings. A holdings record cannot be moved if an item is moved, but the mechanism tries to make sure there's a suitable holdings record used or added in the destination biblio. Created attachment 91743 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 91744 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 91745 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 91746 [details] [review] Bug 20447: DBIX schema changes Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 91747 [details] [review] Bug 20447: Use blessed objects and fix issues found during review Rebased. Setting back to Signed Off. Please set back and describe the issue in detail if it's real. (In reply to Ere Maijala from comment #136) > Joonas, was that an assumption or a verified problem? There's code in > Items::MoveItemFromBiblio that does its best to handle holdings. A holdings > record cannot be moved if an item is moved, but the mechanism tries to make > sure there's a suitable holdings record used or added in the destination > biblio. I probably noticed the problem only in the code when I commented but try the following test case and you see the problem happens for real: 1) Create new record with item and a holdings record. Don't attach the holding to the item. 2) create another record with no items or holdings records 3) In cataloguing search (More -> Cataloging) make such search that you find both records 4) Select both of them and click "Merge selected". Select record with no items or holdings as there reference record (the one that will be kept) 5) After the merge is done notice that the holdings record didn't transfer to the merged record. Ok, thanks, I'll fix.. Looks like bug 22690 is needed so that merge can be done in a sensible way. Note to self: make sure there's also an index on the timestamp column. (In reply to Ere Maijala from comment #146) > Note to self: make sure there's also an index on the timestamp column. +1000 Created attachment 94539 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 94540 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 94541 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 94542 [details] [review] Bug 20447: DBIX schema changes Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Now rebased and made merging use bug 22690 as the base. Includes the following tweaks: - timestamp column has a key - Holdings tab is shown second to Items tab so that it doesn't interfere with displaying items when there are no holdings records. browser-strings.inc will be removed by bug 23884. Created attachment 97796 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 97797 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 97798 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 97799 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 97800 [details] [review] Bug 20447: DBIX schema changes Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased. Created attachment 97843 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 97844 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 97845 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 97846 [details] [review] Bug 20447: DBIX schema changes Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99143 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99144 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99145 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99146 [details] [review] Bug 20447: DBIX schema changes Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Rebased. Created attachment 99176 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99177 [details] [review] Bug 20447: Add a batch rebuild script for holdings table. This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99178 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99179 [details] [review] Bug 20447: DBIX schema changes Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Fixed a typo introduced during last rebase. Created attachment 99235 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99236 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 99237 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99238 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 99239 [details] [review] Bug 20447: DBIX schema changes Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Added a minimal patch to include holding_id in the item definition of the REST API. I'll defer actual API functionality to a later bug. Please, rebase Comment on attachment 99235 [details] [review] Bug 20447: MARC Holdings support Review of attachment 99235 [details] [review]: ----------------------------------------------------------------- Hi Ere, I read through the code and have some notes/questions Overall, I think it's great enhancement and I really like this to be part of Koha. ::: C4/Search.pm @@ +2242,5 @@ > } > > + # Fetch summary holdings > + if (C4::Context->preference('SummaryHoldings')) { > + $summary_holdings = Koha::Holdings->search({ biblionumber => $oldbiblio->{biblionumber}, deleted_on => undef }); Maybe something like $biblio->holdings? ::: Koha/Biblio.pm @@ +520,5 @@ > + > +sub holdings { > + my ($self) = @_; > + > + $self->{_holdings} ||= Koha::Holdings->search( { biblionumber => $self->biblionumber } ); use relation and new_from_dbic? ::: Koha/Holding.pm @@ +23,5 @@ > +use Carp; > + > +use C4::Charset; > +use C4::Log; > + are all imports needed? @@ +138,5 @@ > + > + return > + wantarray > + ? Koha::Items->_new_from_dbic($items_rs)->as_list > + : Koha::Items->_new_from_dbic($items_rs); do we really need this duality? @@ +166,5 @@ > + my $guard = C4::Context->dbh->{AutoCommit} ? $schema->txn_scope_guard() : undef; > + > + my $result = $self->SUPER::store(); > + > + return $result unless $result; Should it be just 'return unless $result'? ::: Koha/Holdings.pm @@ +25,5 @@ > +use C4::Biblio; > +use C4::Charset; > +use C4::Context; > +use Koha::Database; > +use Koha::Holding; Are all imports needed? ::: Koha/Holdings/Metadata.pm @@ +34,5 @@ > +=cut > + > +=head3 record > + > +my @record = $metadata->record($params); there are no params handled in method 'record' ::: catalogue/detail.pl @@ +432,3 @@ > C4::Search::enabled_staff_search_views, > + materials => $materials_flag, > + show_summary_holdings => C4::Context->preference('SummaryHoldings') ? 1 : 0, You could use template plugin for getting value of a syspref and then you don't need to pass this variable to template ::: cataloguing/addholding.pl @@ +151,5 @@ > +=cut > + > +sub CreateKey { > + return int(rand(1000000)); > +} Maybe Koha::Token would be better to use instead of this sub, but could be leaved to other bug report, as this pattern is in more files now. ::: cataloguing/value_builder/marc21_field_008_holdings.pl @@ +22,5 @@ > +use C4::Auth; > +use CGI qw ( -utf8 ); > +use C4::Context; > + > +use C4::Search; Do you need C4::Search ? @@ +39,5 @@ > + my $function_name = $params->{id}; > + my $dateentered = date_entered(); > + my $res = " > +<script type=\"text/javascript\"> > +//<![CDATA[ type parameter and CDATA should not be used ::: cataloguing/value_builder/marc21_leader_holdings.pl @@ +29,5 @@ > + my ( $params ) = @_; > + my $function_name = $params->{id}; > + my $res = " > +<script type=\"text/javascript\"> > +//<![CDATA[ Type parameter and CDATA should not be used ::: koha-tmpl/intranet-tmpl/prog/css/addholding.css @@ +139,5 @@ > + > +.yui-gf .yui-u { > + width: 79.2%; > +} > + Are there still yui classes? ::: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addholding.tt @@ +6,5 @@ > +[% INCLUDE 'doc-head-close.inc' %] > +[% Asset.js("lib/hc-sticky.js") | $raw %] > +[% Asset.js("js/cataloging.js") | $raw %] > +[% INCLUDE 'strings.inc' %] > +[% Asset.js("js/browser.js") | $raw %] Even javascript included by Asset plugin should be at end of template And you need to set variable footerjs, like: [% SET footerjs = 1 %] (And macro block - see bellow) @@ +287,5 @@ > +</div> > +</div> > +</div> > + > +<script> Scripts should be in enclosed by [% MACRO jsinclude BLOCK %] ::: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/marc21_field_008_holdings.tt @@ +3,5 @@ > +<title>Koha › Holdings › 008 builder</title> > +[% INCLUDE 'doc-head-close.inc' %] > +</head> > +<body id="cat_marc21_field_008_holdings" class="cat" style="padding:1em;"> > +<h3> 008 Fixed-length data elements</h3> There is white space just after <h3> Thanks Josef, I'll address your comments when I'm done rebasing. It proved to be a bit more work than before since the MARC 21 default framework was changed to YAML. Created attachment 105068 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 105069 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 105070 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 105071 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 105072 [details] [review] Bug 20447: DBIX schema changes Rebased, review comments not yet addressed. I just attached the rebased version out of paranoia to make sure a couple of days working on rebasing won't get lost. Created attachment 105259 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 105260 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 105261 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 105262 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 105263 [details] [review] Bug 20447: DBIX schema changes I've now addressed Josef's comments apart from the ones listed below (for a reason): > ::: C4/Search.pm > @@ +2242,5 @@ > > } > > > > + # Fetch summary holdings > > + if (C4::Context->preference('SummaryHoldings')) { > > + $summary_holdings = Koha::Holdings->search({ biblionumber => $oldbiblio->{biblionumber}, deleted_on => undef }); > > Maybe something like $biblio->holdings? I didn't change this since it would require fetching the biblio record for each result, and since we don't currently do that, it would have been extra work required just to get the holdings. I added a comment noting that. > ::: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addholding.tt > @@ +6,5 @@ > > +[% INCLUDE 'doc-head-close.inc' %] > > +[% Asset.js("lib/hc-sticky.js") | $raw %] > > +[% Asset.js("js/cataloging.js") | $raw %] > > +[% INCLUDE 'strings.inc' %] > > +[% Asset.js("js/browser.js") | $raw %] > > Even javascript included by Asset plugin should be at end of template > > And you need to set variable footerjs, like: [% SET footerjs = 1 %] (And > macro block - see bellow) addholding.tt mirrors addbiblio.tt, and addbiblio.tt uses js plugins that don't support footerjs. I updated addholding.tt according to changes in addbiblio.tt, but I left the structure intact so that it's easier to compare and update the files. It might be possible in a follow-up to merge some parts of the separate templates. Created attachment 106147 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 106148 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 106149 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 106150 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 106151 [details] [review] Bug 20447: DBIX schema changes Rebased and added logic to select the holdings tab instead of items tab if the bibliographic record has holdings records but no items. Created attachment 108250 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 108251 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 108252 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 108253 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 108254 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 108255 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Created attachment 108336 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108337 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108338 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108339 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108340 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108341 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108789 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Very trivial string change and new subroutine in test caused conflict. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 108790 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108791 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108792 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108793 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 108794 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Hi, just rebased again the first patch "Bug 20447: MARC Holdings support". There was a very trivial string change in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt caused by the string "Current location" changing in Koha master to "Current library". Also in t/db_dependent/Koha/Item.t a new test was introduced in koha master to the same place where this patch had changed so I simply resolved this by allowing the new test to come through. Both of these were very trivial so I leave this patch still to Signed Off state. I left a note after Michal's Signed-off-by to the patch so in case there is some problems no one can blame Michal but they can ask me to fix things if I broke something (which I don't think since this was very trivial rebase). C4::Biblio::DelBiblio, and some of the code calling it only checks whether items exists before deleting the biblio. We need to check also if holdings records exists in the biblio before it can be deleted. A listing of things to fix (search for DelBiblio): - C4/Acquisition.pm - misc/batchdeletebiblios.pl - svc/bib - misc/cronjobs/delete_records_via_leader.pl Maybe some more might exists but these at least should be fixed before merging. Created attachment 109695 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Very trivial string change, new test and new column in kohastructure.sql caused conflict. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109696 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109697 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109698 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Trivial changes in test to fix rebase conflict Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109699 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Deleteditem.pm and Item.pm schema files had changes, regenerated them with dbic Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109700 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Rebased. The issue with deletion still exists so keeping FQA status. Created attachment 109703 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Very trivial string change, new test and new column in kohastructure.sql caused conflict. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109704 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109705 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109706 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Trivial changes in test to fix rebase conflict Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109707 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Deleteditem.pm and Item.pm schema files had changes, regenerated them with dbic Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109708 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Check for existing holdings has now been added. Created attachment 109711 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109712 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109713 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Trivial changes in test to fix rebase conflict Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109714 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Deleteditem.pm and Item.pm schema files had changes, regenerated them with dbic Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109715 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> mates, first patch from the list was obsoleted and not popped-up again, this "MARC Holdings support", that's for a reason? I didn't found an explanation. Comment on attachment 109703 [details] [review] Bug 20447: MARC Holdings support Unobsoleted the main patch. I'll need to verify one more thing wrt forcing deletion of biblios. Created attachment 109717 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 109718 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109719 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109720 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Trivial changes in test to fix rebase conflict Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109721 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Deleteditem.pm and Item.pm schema files had changes, regenerated them with dbic Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109722 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Fixed acquisitions to consider existing holdings records and Koha::Biblio->holdings to only return non-deleted records. Needs new signoff for the base patch. Created attachment 109741 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 109742 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109743 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109744 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Trivial changes in test to fix rebase conflict Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109745 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Deleteditem.pm and Item.pm schema files had changes, regenerated them with dbic Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 109746 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Latest patches just fix some code style issues detected with QA tools. Note: I believe the boolean/tinyint thing is bogus, and the script block in addholding.tt cannot be moved until the scripts shared with addbiblio.tt are fixed. Created attachment 109765 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109766 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109767 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109768 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Trivial changes in test to fix rebase conflict Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109769 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Deleteditem.pm and Item.pm schema files had changes, regenerated them with dbic Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109770 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109873 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109874 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109875 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109877 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Trivial changes in test to fix rebase conflict Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109878 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Deleteditem.pm and Item.pm schema files had changes, regenerated them with dbic Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 109879 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Fixed a rebase issue that lost a block added in bug 20667 from Koha/Schema/Result/Item.pm. Created attachment 110115 [details] [review] Bug 20447: (follow-up) Prevent internal server error with Attach item feature If one tried to use the "Edit -> Attach item" feature in biblio detail.pl page then the following error would come if the item was not attached to any holdings record: "DBIC result _type isn't of the _type Holding" This fix prevents the error by skipping the creation of Holding object if the result set is empty. Created attachment 110174 [details] [review] Bug 20447: (follow-up) Fix holding_branch getter to handle null This fix avoids an internal server error when holdingbranch column is empty in the database. This could happen e.g. when there's a problem with the framework's Koha field mapping. Created attachment 110313 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110314 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110315 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110316 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Trivial changes in test to fix rebase conflict Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110317 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Deleteditem.pm and Item.pm schema files had changes, regenerated them with dbic Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110318 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110319 [details] [review] Bug 20447: (follow-up) Prevent internal server error with Attach item feature If one tried to use the "Edit -> Attach item" feature in biblio detail.pl page then the following error would come if the item was not attached to any holdings record: "DBIC result _type isn't of the _type Holding" This fix prevents the error by skipping the creation of Holding object if the result set is empty. Created attachment 110320 [details] [review] Bug 20447: (follow-up) Fix holding_branch getter to handle null This fix avoids an internal server error when holdingbranch column is empty in the database. This could happen e.g. when there's a problem with the framework's Koha field mapping. Reverted changes to obsolete nb-NO framework sql. Carried over the sign-offs. Created attachment 110467 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 110468 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110469 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110471 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Trivial changes in test to fix rebase conflict Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110472 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Deleteditem.pm and Item.pm schema files had changes, regenerated them with dbic Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110473 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 110474 [details] [review] Bug 20447: (follow-up) Prevent internal server error with Attach item feature If one tried to use the "Edit -> Attach item" feature in biblio detail.pl page then the following error would come if the item was not attached to any holdings record: "DBIC result _type isn't of the _type Holding" This fix prevents the error by skipping the creation of Holding object if the result set is empty. Created attachment 110475 [details] [review] Bug 20447: (follow-up) Fix holding_branch getter to handle null This fix avoids an internal server error when holdingbranch column is empty in the database. This could happen e.g. when there's a problem with the framework's Koha field mapping. Rebased the patch "Bug 20447: MARC Holdings support" and kept sign-offs given it was really trivial conflict caused by trivial string change in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref from commit a02bd4f71c2 Just noticed also this tiny issue in koha-tmpl/intranet-tmpl/prog/en/modules/tools/export.tt The file is missing [% USE Koha %] from the top and that's why the checkbox to export holdings records doesn't appear. Created attachment 112176 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 112177 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 112179 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 112180 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 112181 [details] [review] Bug 20447: DBIX schema changes Created attachment 112182 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Rebased and added tests and fixes. I had to do enough changes that a new sign-off is needed. Created attachment 112189 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 112190 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 112191 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 112192 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 112193 [details] [review] Bug 20447: DBIX schema changes Created attachment 112194 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 One more round of cleanup and QA fixes. Patch doesn't apply anymore. Created attachment 112570 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Created attachment 112571 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 112572 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 112573 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 112574 [details] [review] Bug 20447: DBIX schema changes Created attachment 112575 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Rebased. However, there are some QA script failures, not sure if everything is caused by these patches but these should be investigated one by one and then after fixing or making sure they are not caused by these patches let's move back to Needs signoff. These patches need also a test plan. FAIL t/lib/TestBuilder.pm OK critic OK forbidden patterns OK git manipulation FAIL pod OK pod coverage OK spelling OK valid FAIL installer/data/mysql/updatedev.pl FAIL critic OK file permissions OK forbidden patterns OK git manipulation OK pod OK spelling OK valid FAIL installer/data/mysql/kohastructure.sql FAIL boolean_vs_tinyint OK charset_collate OK git manipulation FAIL tinyint_has_boolean_flag FAIL Koha/Schema/Result/HoldingsMetadata.pm OK critic OK file permissions OK forbidden patterns OK git manipulation OK pod FAIL pod coverage OK spelling OK valid FAIL Koha/Schema/Result/Holding.pm OK critic OK file permissions OK forbidden patterns OK git manipulation OK pod FAIL pod coverage OK spelling OK valid Adding default indicator field value for example to field 852 through the settings in Home › Administration › MARC frameworks › Default holdings framework (HLD) doesn't work. After setting the default value for indicator and go to a biblio graphic record page to create a new holdings record and go to the tab where 852 is then the indicator value is not populated with the default value I set. Setting of default indicator value works correctly with the bibliographic record editing. Comment on attachment 112570 [details] [review] Bug 20447: MARC Holdings support Review of attachment 112570 [details] [review]: ----------------------------------------------------------------- ::: cataloguing/addholding.pl @@ +686,5 @@ > + author => $biblio->author > +); > + > +$template->param( > + frameworks => Koha::BiblioFrameworks->search({}, { order_by => ['frameworktext'] }), Arrays given as template params needs to be references to the array. This should be something like frameworks => \@frameworks instead (where @frameworks is the result of the search). This caused the cataloguing frameworks not to show up in the MARC holdings editor. This patch set causes following error in error logs: > Use of uninitialized value in concatenation (.) or string at /usr/share/koha/intranet/cgi-bin/cataloguing/additem.pl line 242. > 242 $authorised_lib{$holding->{holding_id}} = $holding->{holding_id} . ' ' . $holding->{holdingbranch} . ' ' . $holding->{location} . ' ' . $holding->{ccode} . ' ' . $holding->{callnumber}; Before using the values in string one should check whether the string is defined. Created attachment 113943 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113944 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113945 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113946 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113947 [details] [review] Bug 20447: DBIX schema changes https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113948 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113952 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113953 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113954 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113955 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113956 [details] [review] Bug 20447: DBIX schema changes https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113957 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 113958 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Rebased and fixed the frameworks not showing up in cataloguing editor. The QA tool failures and indicator issues are still to be fixed. Created attachment 113959 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Currently adding permissions items_batchdel items_batchmod is required if one wants to include piece of JavaScript that will bring up confirmation box for the deletion of holdings records. This also depends on the StaffDetailItemSelection syspref. The librarian without those permissions can already delete holdings so these permissions should not be required to just bring up the confirmation box for deletion. Created attachment 115239 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115240 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115241 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115242 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115243 [details] [review] Bug 20447: DBIX schema changes https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115244 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115245 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Rebased. Created attachment 115654 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115655 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115656 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115657 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115658 [details] [review] Bug 20447: DBIX schema changes https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115659 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115660 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Rebased. But also the main change in installer/data/mysql/kohastructure.sql: since the current file in the main DB re-exported fully from MariaDB 10.5, so all changes and updates also adjusted to be in the new SQL dumper style. Adding regenerated DBIX::Class Schema with proper checksums: Created attachment 115722 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115723 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115724 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115725 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115726 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115727 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 115728 [details] [review] Bug 20447: DBIX schema changes https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 115729 [details] [review] Bug 20447: (Follow-up) Regenerate DBIX schema to match with latest DB state Created attachment 116797 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 116798 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 116799 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 116800 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 116801 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 116802 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 116803 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 116804 [details] [review] Bug 20447: DBIX schema changes https://bugs.koha-community.org/show_bug.cgi?id=22690 Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 116805 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 116806 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 116807 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 116808 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 116809 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 116810 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 116811 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 116812 [details] [review] Bug 20447: DBIX schema changes https://bugs.koha-community.org/show_bug.cgi?id=22690 Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Rebased again to accomodate changes in authority fetching and got the select2 changes from addhbiblio.tt file similarly to addholding.tt (addholding.tt is based on addbiblio.tt) in order to fix MARC editor issue with select input box showing twice. Removed also updatedev.pl file that came from rebase issue in attachment 112189 [details] [review]. The file is not related at all to holdings and probably was something accidentally attached. QA script failures still exists and warnings in addholding.pl when some strings are undefined. I got reports that Advanced editor mode doesn't work with holdings but it works with biblios. Probably should aim for feature parity here. Created attachment 118382 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 118383 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 118385 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 118386 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 118387 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 https://bugs.koha-community.org/show_bug.cgi?id=22690 Created attachment 118388 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 118389 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 118390 [details] [review] Bug 20447: DBIX schema changes https://bugs.koha-community.org/show_bug.cgi?id=22690 Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 118397 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 118398 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 118399 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 118400 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 118401 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Created attachment 118402 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 118403 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 118404 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 118405 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 118406 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 118407 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 118408 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 118409 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Created attachment 118410 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 118411 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 118412 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Rebased. Still the issues mentioned in previous comments remaining. Created attachment 118429 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 118430 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 118431 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 118432 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 118433 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Created attachment 118434 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 118435 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 118436 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Just did a major rebase for addholding.pl and addholding.tt based on the addbiblio.tt and addbiblio.pl. Rebased on top of Bug 21986, 14662, 20477, 9701, 21172, 20128, 22748, 22721, 7882, 8643, 16314, 27578, and 8976. Note: the patch set was already rebased partly to bug 7882 by someone (Maybe Ere) but the part for -foreach my $tag (@tab_data) { +foreach my $tag (sort @tab_data) { was missed. - Bug 8643 I only managed to partly rebase here because the change was so big, so I only did the same changes to addholding.pl but they would need to be done in addholding.tt as well for the warning prompt to show up when saving, now it only higlight the field as important. I think we cannot just copy&paste addbiblio.tt as addholding.tt so we need to get back to this later. - "Bug 25728: Create AV when cataloguing a bibliographic record" was not backported, too complicated. - Would be good to refactor the addholding.{pl,tt}, additem.{pl,tt}, addbiblio.{pl,tt} so the rebases won't be too hard. - I didn't confirm whether the rebase for bug 8976 is working, just added the same changes as there was for addbiblio.pl. The indicator issue is now resolved (because it was due to incorrect rebase earlier). The QA warnings and the warning in error log are still to be resolved. Selecting holding record in http://localhost:8081/cgi-bin/koha/acqui/neworderempty.pl?booksellerid=1&basketno=1&biblionumber=248 page doesn't work. The dropdown menu for "k - Holdings record" is empty. It should be populated with the holdings records in the biblio record we are doing the order for. Created attachment 118527 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 118528 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 118529 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 118530 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 118531 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Created attachment 118532 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 118533 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 118534 [details] [review] Bug 20447: Make holdings suppress column boolean object This enables Koha::Object::TO_JSON to return JSON boolean values. If we need to add supression to other parts of Koha than just OPAC in the future maybe it could have it in its own separate column (instead of using supress int values to represent different levels of supression). So using suppress column just for OPAC should be fine. Created attachment 118535 [details] [review] Bug 20447: Move build_sample_holdings_record() description next to other descriptions This prevents the following QA tool error: WARNING: =head3 without preceding higher level in file t/lib/TestBuilder.pm Created attachment 118536 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> QA tool failures are now fixed. But please note: > FAIL installer/data/mysql/kohastructure.sql > FAIL boolean_vs_tinyint > The new column (suppress) for table holdings is using BOOLEAN as type, must be TINYINT(1) instead, see the SQL12 coding guideline This is false positive, the QA script matches the word "Boolean" in SQL *comment* which says "Boolean indicating whether [...]". and also > FAIL Koha/Schema/Result/Holding.pm > FAIL pod coverage > POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 seems false positive as well? The warning mentioned in comment 314 doesn't happen anymore. Not sure where it got fixed, maybe Koha::Object handling or DBIX stuff changed. Before setting this to NSO I think we want to fix the neworderempty.pl page mentioned in comment 409. Emptying 852$h subfield of holdings record doesn't empty the linked db entry in holdings.callnumber column. Currently when modifying a biblio in koha the old biblio revision is stored in action logs: > logaction( "CATALOGUING", "MODIFY", $biblionumber, "biblio BEFORE=>" . $newrecord->as_formatted ); This does not happen now with the holdings patches here. It used to be included in this patch set but the re-worked version of this (the holdings patches were originally made in C4/Holdings.pm) doesn't do this logging. It only logs "holdings" to the info field: > logaction('CATALOGUING', $action, $self->holding_id(), 'holding') Created attachment 119265 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 119266 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 119267 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 119268 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 119269 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Created attachment 119270 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 119271 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 119272 [details] [review] Bug 20447: Make holdings suppress column boolean object This enables Koha::Object::TO_JSON to return JSON boolean values. If we need to add supression to other parts of Koha than just OPAC in the future maybe it could have it in its own separate column (instead of using supress int values to represent different levels of supression). So using suppress column just for OPAC should be fine. Created attachment 119273 [details] [review] Bug 20447: Move build_sample_holdings_record() description next to other descriptions This prevents the following QA tool error: WARNING: =head3 without preceding higher level in file t/lib/TestBuilder.pm Created attachment 119274 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Rebased. Created attachment 119514 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 119515 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 119516 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 119517 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 119518 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Created attachment 119519 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 119520 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 119521 [details] [review] Bug 20447: Make holdings suppress column boolean object This enables Koha::Object::TO_JSON to return JSON boolean values. If we need to add supression to other parts of Koha than just OPAC in the future maybe it could have it in its own separate column (instead of using supress int values to represent different levels of supression). So using suppress column just for OPAC should be fine. Created attachment 119522 [details] [review] Bug 20447: Move build_sample_holdings_record() description next to other descriptions This prevents the following QA tool error: WARNING: =head3 without preceding higher level in file t/lib/TestBuilder.pm Created attachment 119523 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Rebased. Created attachment 119570 [details] [review] Bug 20447: Return all Koha fields even if the value is empty The database columns are updated using the koha field values returned by this subroutine and if we don't return all of them (even the empty ones) the holdings table's database columns won't get emptied. Created attachment 119571 [details] [review] Bug 20447: Return valid koha field values The legal values for holdings.suppress koha field are 0 and 1. This makes sure that illegal values cannot be returned. Is FQA still correct here ? (In reply to Katrin Fischer from comment #447) > Is FQA still correct here ? Yes. There are some problems in mentioned in the earlier comments, at least the holdings being broken in acquisitions item adding. A new bug found: The holdings editor doesn't load by default the frameworkcode template defined in holdings.frameworkcode column, it always loads HLD / default framework regardless of this value. Created attachment 121216 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 121217 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 121220 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 121221 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 121222 [details] [review] Bug 20447: (follow-up) Add required metadata field when creating holdings Since metadata column is mandatory in the holdings_metadata table we need to define it already when it is first created instead of creating it first and updating the column immediately after. This fixes the following error during holdings record creation: DBIx::Class::Storage::DBI::_dbh_execute(): Field 'metadata' doesn't have a default value at /kohadevbox/koha/Koha/Objects.pm line 114 Created attachment 121223 [details] [review] Bug 20447: (follow-up) Correctly list all holdings frameworks in editor The Template Toolkit expects a ref to array and we passed the array instead. It caused the listing of holdings frameworks not to show when there was more than one holdings framework defined. To test: 1) Apply patch 2) Make 3 copies of the default HLD framework 3) Go to a biblio page and create new holding 4) Click Settings in the editor page and notice all 3 frameworks are now listed in the drop-down menu Created attachment 121226 [details] [review] Bug 20447: Don't use GROUP by when fetching authorised values for editor The syntax is invalid SQL. The original intent was probably to remove duplicates from the holdings record marc editor but it would have actually removed then the other instances of the wanted authorised values from the list. Now we show all the authorised values that should be possible to select and yes, if you set the same description (lib field) for two different authorised values in the same category then you cannot distinguish them from each other. Created attachment 121227 [details] [review] Bug 20447: Make holdings suppress column boolean object This enables Koha::Object::TO_JSON to return JSON boolean values. If we need to add supression to other parts of Koha than just OPAC in the future maybe it could have it in its own separate column (instead of using supress int values to represent different levels of supression). So using suppress column just for OPAC should be fine. Created attachment 121228 [details] [review] Bug 20447: Move build_sample_holdings_record() description next to other descriptions This prevents the following QA tool error: WARNING: =head3 without preceding higher level in file t/lib/TestBuilder.pm Created attachment 121229 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 121230 [details] [review] Bug 20447: Return all Koha fields even if the value is empty The database columns are updated using the koha field values returned by this subroutine and if we don't return all of them (even the empty ones) the holdings table's database columns won't get emptied. Created attachment 121231 [details] [review] Bug 20447: Return valid koha field values The legal values for holdings.suppress koha field are 0 and 1. This makes sure that illegal values cannot be returned. Rebased the first patch "Bug 20447: MARC Holdings support". Could this be a good addition to the road map? I think it would be a candidate for early integration next release. https://annuel.framapad.org/p/koha_21.11_roadmap Katrin, Yes, that'd be great. If it was on the roadmap I'm sure I could allocate a bit more resources to any required cleanup and fixes as required. (In reply to Ere Maijala from comment #464) > Katrin, Yes, that'd be great. If it was on the roadmap I'm sure I could > allocate a bit more resources to any required cleanup and fixes as required. You can put it on the road map :) See Jonathan's email from today on the mailing list. Summary of remaining tasks to do: 1. The holdings editor doesn't load by default the frameworkcode template defined in holdings.frameworkcode column, it always loads HLD / default framework regardless of this value. 2. Holdings selection being broken in acquisitions item adding (the list is not populated). 3. Currently when modifying a biblio in koha the old biblio revision is stored in action logs: > logaction( "CATALOGUING", "MODIFY", $biblionumber, "biblio BEFORE=>" . $newrecord->as_formatted ); This does not happen now with the holdings patches here. It used to be included in this patch set but the re-worked version of this (the holdings patches were originally made in C4/Holdings.pm) doesn't do this logging. It only logs "holdings" to the info field: > logaction('CATALOGUING', $action, $self->holding_id(), 'holding') 4. Verify if we need this or not: "- "Bug 25728: Create AV when cataloguing a bibliographic record" was not backported, too complicated." (from comment #408) 5. Squash some of the patches / follow-ups. Other notes: "Emptying 852$h subfield of holdings record doesn't empty the linked db entry in holdings.callnumber column." from comment #421 is fixed with "Return valid koha field values patch". Created attachment 121379 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 121380 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 121381 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 121382 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 121383 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Squashed my follow-ups and added co-authored-by trailer line. Please refer to comment #466 for list of TODO items for get this out of FQA. Created attachment 121406 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 121407 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 121408 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 121409 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 121410 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Rebased on top of yesterdays changes from bug 22690 It looks like we have the same bug in Koha::Holding::move_to_biblio() as we had in 22690, i.e. the other record not being re-indexed after the move. Here we seem to be missing the re-indexing of the source biblio record meaning the ES index will have outdated items info there. Created attachment 122501 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 122502 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Created attachment 122503 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Created attachment 122504 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Created attachment 122505 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Known issues identified in #466 and #479 should now be fixed. Minor thing, this needs updating to take bug 26703 into account. Created attachment 122851 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122852 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122853 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122854 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122855 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122856 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Wow, there's some serious history here. I'm impressed it's all been maintained and kept working. Minor rebase done and follow-up added to take care of bug 26703 requirements All seems to work as I would expect, but I must admit I'm not a cataloguing expert so can only really speak to the test plan passing and the code looking reasonable. Signing off. I'm also wondering how this bug relates to bug 24857. The 'volumes' level added there appears to be at a similar level, though a totally different approach, to the record level introduced here. I'm wondering if the follow-on work that utilises Volumes could actually be achieved using holdings level records as an alternative.. I'm weary of having two very similar features added around the same time doing nearly the same thing but in entirely different ways.. I'd love to see bug 28715 taken care of once this is pushed.. it feels like a real end goal for many of the competing bugs like this at the moment. Starting a new script (addholding.pl) copying addbiblio.pl was not the best idea ;) Provide a full test plan and release notes please. Created attachment 122891 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122892 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122893 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122894 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122895 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122896 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 122897 [details] [review] Bug 20447: Fixes for bug 17600 OK, Prior to bug 17600 I found this working.. so signed off.. but having rebased it for bug 17600 (so I could work on bug 28715 I couldn't get it working again. I can 1) Enable the new preference 2) Add a holding record to an existing bib 3) Add an item to the bib... But.. I can no longer pick the 'Holding' to relate the item to.. either via editing an existing item or adding a new one. I'm stuck, can't see in the code where this is going wrong. Marking as FQA to get authors attention. Created attachment 123047 [details] [review] Bug 22690: (QA follow-up) Add TrackedLink classes and use them This patch adds Koha::TrackedLink(s) classes based on Koha::Object(s) and then adds the relationship accessor to Koha::Item and uses it within the move_to_biblio method. Tests for new relationship also added to t/db_dependent/Koha/Item.t Created attachment 123048 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123049 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123050 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123051 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123052 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123053 [details] [review] Bug 20447: Fixes for bug 17600 Created attachment 123054 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123055 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123056 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123057 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123058 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123059 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 123060 [details] [review] Bug 20447: Fixes for bug 17600 Created attachment 123061 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Eww... As far as I can see, bug 24857 looks like a subset of proper holdings records. It's a lot of semi-duplicate work... I'll pick up from where Joonas got things. Created attachment 124012 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124013 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124014 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124015 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124016 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124017 [details] [review] Bug 20447: Fixes for bug 17600 Created attachment 124018 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Martin, should be working now. Joonas did most of the work, and I fixed a typo in "Fixes for bug 17600" patch and holdings XML creation for XSLT. ALso rebased to fix expected test counts. (In reply to Ere Maijala from comment #521) > Eww... As far as I can see, bug 24857 looks like a subset of proper holdings > records. It's a lot of semi-duplicate work... > > I'll pick up from where Joonas got things. There are a whole set of bugs around item grouping at the moment.. all doing similar things but not exactly the same. * The volumes one, which has now been renamed to 'Item groups' for clarity is for grouping items within a bib record so you can place a group level hold. * The bundles one is for grouping items from different bibs into a new bib for loaning as a set with automatic withdrawal from normal circulation otherwise. I'm not entirely sure I'm close enough to MARC Holdings to know how/where it is used or how it may overlap, be a superset or subset of the other two bugs. There's also been lots of work to improve our handling of Component Parts and Volume linked records (i.e. 773 and friends) See bug 11175. I'll try to take another look at this soon but my brain is a bit of a mess trying to distinguish between all these features. Sorry.. doesn't apply again.. Koha is moving fast in this area at the moment! Martin, As far as I can see, bundles is a different but useful case. However, it seems that bug 24857 is kind of a subset of what holdings records would be (if properly embraced), though as there's no RFC or much more information on how the item groups work, I can't say for sure. What I can say is that it doesn't make sense to have two methods that allow for grouping items in a bib. The main advantage of holdings records compared to item groups is that they can also exist without items and provide useful summary holdings information about items that have not been individually catalogued. And since holdings records are a standard thing among MARC records, there are other systems that support them as well. Since item groups has passed QA, I assume it's to be merged soon. At this point I'll have to reconsider what to do here. I'm must admit that I'm somewhat discombobulated (yes, I finally got to use the word!) as to how to proceed. It might be possible to make summary holdings work with item groups without making a complete mess, but the relationship between the two will need a lot of work. (In reply to Ere Maijala from comment #532) > Since item groups has passed QA, I assume it's to be merged soon. At this > point I'll have to reconsider what to do here. I'm must admit that I'm > somewhat discombobulated (yes, I finally got to use the word!) as to how to > proceed. It might be possible to make summary holdings work with item groups > without making a complete mess, but the relationship between the two will > need a lot of work. Did you raise your points on that bug too? Without knowing all details, this sounds like a serious argument to postpone pushing that one? > discombobulated I was so confused that I had to lookup that one ;) (In reply to Marcel de Rooy from comment #533) > Did you raise your points on that bug too? Without knowing all details, this > sounds like a serious argument to postpone pushing that one? I've now added a comment to raise awareness. I'd like to keep the discussion in one place, though. And I'm not quite sure how to approach this all. > > discombobulated > > I was so confused that I had to lookup that one ;) Ha! :D Indeed.. this is a very complicated set of features and I agree we certainly need to think about them as a complimentary pair. Tomas is keen to talk and help this area move forward too having taken over the Item groups piece. My hope is we can come up with some way of achieving the best for everyone. I have my doubts as to whether the Item groups stuff will be pushed this cycle.. I couldn't fail it in due conscience as it all works well enough and I couldn't see that it would block this work.. but I can definitely see how they are competitive and thus this needs taking into context before final push. Created attachment 124944 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124945 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124946 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124947 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124948 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 124949 [details] [review] Bug 20447: Fixes for bug 17600 Created attachment 124950 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Rebased and fixed displaying of holdings records in OPAC results when there are no items. (In reply to Martin Renvoize from comment #535) > Indeed.. this is a very complicated set of features and I agree we certainly > need to think about them as a complimentary pair. > > Tomas is keen to talk and help this area move forward too having taken over > the Item groups piece. My hope is we can come up with some way of achieving > the best for everyone. I have my doubts as to whether the Item groups stuff > will be pushed this cycle.. I couldn't fail it in due conscience as it all > works well enough and I couldn't see that it would block this work.. but I > can definitely see how they are competitive and thus this needs taking into > context before final push. Okay, I've now rebased the code (I think this was rebase 32 from me :D) and also fixed a problem displaying the holdings locations in OPAC results list when there are no items (apparently this was broken in one of the earlier rebases). So for this part this would be ready to move on. Created attachment 126204 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126205 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126206 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126207 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126208 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126209 [details] [review] Bug 20447: Fixes for bug 17600 Created attachment 126210 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126211 [details] [review] Bug 20447: (QA follow-up) Unify SQL queries formatting Created attachment 126212 [details] [review] Bug 20447: (QA follow-up) Change 3 pushes to `() x 3` for readability I did rebase because changes in Bug 29135 messed up with this a lot, Ere, please check behind me. Created attachment 126296 [details] [review] Bug 20447: (QA follow-up) SummaryHoldings preferences should be 1/0 Created attachment 126391 [details] [review] Bug 20447: (QA follow-up) Remove warning when frameworkcode is undef Created attachment 126392 [details] [review] Bug 20447: (QA follow-up) Fix framework switching bug Form's parameter never used because $record was always defined, with new or with existing holding, but always. Fixed by adding incoming $holding_id presence check. Created attachment 126394 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126395 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126396 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126397 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126398 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126399 [details] [review] Bug 20447: Fixes for bug 17600 Created attachment 126400 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126401 [details] [review] Bug 20447: (QA follow-up) Unify SQL queries formatting Created attachment 126402 [details] [review] Bug 20447: (QA follow-up) Change 3 pushes to `() x 3` for readability Created attachment 126403 [details] [review] Bug 20447: (QA follow-up) SummaryHoldings preferences should be 1/0 Created attachment 126404 [details] [review] Bug 20447: (QA follow-up) Remove warning when frameworkcode is undef Created attachment 126405 [details] [review] Bug 20447: (QA follow-up) Fix framework switching bug Form's parameter never used because $record was always defined, with new or with existing holding, but always. Fixed by adding incoming $holding_id presence check. Rebase was because of changes in the Bug 29135 and further, now I added patches and fixes, and rebased finally for patches to be consistent on the current master. ... and did some obsoletion for attachment-useless messages. Now it's easy to QA if this will even be (I am sure it will :) ). Also, I think we need to put 11175 as "this 20447 based on 11175", I have too many conflicts when I gluing both together on my preproduction branch, and I know how to glue them so I can make this (rebase on top of 11175). Martin: let's put 11175 as dependency here? Created attachment 126932 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126933 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126934 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126935 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126936 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126937 [details] [review] Bug 20447: Fixes for bug 17600 Created attachment 126938 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 126939 [details] [review] Bug 20447: (QA follow-up) Unify SQL queries formatting Created attachment 126940 [details] [review] Bug 20447: (QA follow-up) Change 3 pushes to `() x 3` for readability Created attachment 126941 [details] [review] Bug 20447: (QA follow-up) SummaryHoldings preferences should be 1/0 Created attachment 126942 [details] [review] Bug 20447: (QA follow-up) Remove warning when frameworkcode is undef Created attachment 126943 [details] [review] Bug 20447: (QA follow-up) Fix framework switching bug Form's parameter never used because $record was always defined, with new or with existing holding, but always. Fixed by adding incoming $holding_id presence check. Bug 11175 in master, so I rebased this one to be aligned with the latest master now. Note for Ere/Martin: additem.pl "sub generate_subfield_form" part with "elsif ( $subfieldlib->{authorised_value} eq "holdings" )" block moved to Koha::UI::Form::Builder::Item Created attachment 127040 [details] [review] Bug 20447: (QA follow-up) Fix full-path call for xml_escape Created attachment 128079 [details] [review] Bug 20447: (QA follow-up) Remove undef warnings and add UX note Use of uninitialized value in concatenation (.) or string at /usr/share/koha/lib/Koha/UI/Form/Builder/Item.pm line 255. Created attachment 128131 [details] [review] Bug 20447: (QA follow-up) wrong include place for Koha::Holdings Created attachment 128134 [details] [review] Bug 20447: (QA follow-up) undefined $mode warning remove When POST /intranet/cataloguing/addholding.pl happens from any /cgi-bin/koha/cataloguing/addholding.pl?op=edit&biblionumber=XXX&holding_id=YYY it has "$mode" undefined but comes to code "$mode ne 'popup'" hence the warning. Andrew, can you update this bug on top of master? Thanks yes, also I have two more small bug fixes related. Will do both things tonight Created attachment 130776 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130777 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130778 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130779 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130780 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130781 [details] [review] Bug 20447: Fixes for bug 17600 Created attachment 130782 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130783 [details] [review] Bug 20447: (QA follow-up) Unify SQL queries formatting Created attachment 130784 [details] [review] Bug 20447: (QA follow-up) Change 3 pushes to `() x 3` for readability Created attachment 130785 [details] [review] Bug 20447: (QA follow-up) SummaryHoldings preferences should be 1/0 Created attachment 130786 [details] [review] Bug 20447: (QA follow-up) Remove warning when frameworkcode is undef Created attachment 130787 [details] [review] Bug 20447: (QA follow-up) Fix framework switching bug Form's parameter never used because $record was always defined, with new or with existing holding, but always. Fixed by adding incoming $holding_id presence check. Created attachment 130788 [details] [review] Bug 20447: (QA follow-up) Fix full-path call for xml_escape Created attachment 130789 [details] [review] Bug 20447: (QA follow-up) Remove undef warnings and add UX note Use of uninitialized value in concatenation (.) or string at /usr/share/koha/lib/Koha/UI/Form/Builder/Item.pm line 255. Created attachment 130790 [details] [review] Bug 20447: (QA follow-up) include Koha::Holdings where needed (and remove uses where don't) Created attachment 130791 [details] [review] Bug 20447: (QA follow-up) undefined $mode warning remove When POST /intranet/cataloguing/addholding.pl happens from any /cgi-bin/koha/cataloguing/addholding.pl?op=edit&biblionumber=XXX&holding_id=YYY it has "$mode" undefined but comes to code "$mode ne 'popup'" hence the warning. Created attachment 130792 [details] [review] Bug 20447: (QA follow-up) typo, framework= should be frameworkcode= we don't have 'framework' param parsed in addholding.pl Created attachment 130793 [details] [review] Bug 20447: (QA follow-up) bugfix: holding_id not transferred down from params and not taken into account Created attachment 130794 [details] [review] Bug 20447: (QA follow-up) bugfix: itemcallnumber pref wasn't taken into account Created attachment 130795 [details] [review] Bug 20447: (QA follow-up) UX improvement: values merge point requested to be invisible by customers, i.e. no pipe separate char between (librarians said they using it also for copy-pasting, and it confuses patrons because they read roman numbers with this Created attachment 130796 [details] [review] Bug 20447: (QA follow-up) Warning suppression, tab might be undefined (sidenote: probably tags->subfields->tabs structures should be defined more) Created attachment 130797 [details] [review] Bug 20447: (QA follow-up) bugfix: undefined holding_id in code after storing to db not allows for interface to go back to newly created holding Created attachment 130798 [details] [review] Bug 20447: (QA follow-up) Warning removal: undef param 'sep' expected to be string in further processing Created attachment 130799 [details] [review] Bug 20447: (QA follow-up) Bugfix: Do not force first item for acqorder from biblionumber, but use framework. Note / bigger bug: this happened only for first added item, all next other items in same basket were added without sending biblionumber as in original Koha code: so, this is very suspicious change without any person to explain why. This rolled-back code looks like unfinished feature or mistake. So I will leave this comment especially for future checks, if extension or this feature development will be needed. Created attachment 130800 [details] [review] Bug 20447: (QA follow-up) typo, framework= should be frameworkcode= we don't have 'framework' param parsed in addholding.pl Created attachment 130801 [details] [review] Bug 20447: (QA follow-up) bugfix: holding_id not sent down from params and was not taken into account Created attachment 130802 [details] [review] Bug 20447: (QA follow-up) bugfix: itemcallnumber pref wasn't taken into account Created attachment 130803 [details] [review] Bug 20447: (QA follow-up) UX improvement: merge point of values should be invisible. This is requested by customers, i.e. no pipe separate char between (librarians said they using it also for copy-pasting, and it confuses patrons because they read roman numbers with this | wrongly) Created attachment 130804 [details] [review] Bug 20447: (QA follow-up) Warning suppression tab might be undefined (sidenote: probably tags->subfields->tabs structures should be defined more) Created attachment 130805 [details] [review] Bug 20447: (QA follow-up) bugfix: undefined holding_id in code on store After saving to DB code didn't get back "last inserted ID" so wasn't able to allow for interface to go back to newly created holding ID Created attachment 130806 [details] [review] Bug 20447: (QA follow-up) Warning removal: undef param 'sep' expected to be string in further processing Created attachment 130807 [details] [review] Bug 20447: (QA follow-up) Bugfix: Do not force first item for acqorder from biblionumber, but use framework. Note / bigger bug: this happened only for first added item, all next other items in same basket were added without sending biblionumber as in original Koha code: so, this is very suspicious change without any person to explain why. This rolled-back code looks like unfinished feature or mistake. So I will leave this comment especially for future checks, if extension or this feature development will be needed. Rebased. Also all recent bugfixes and QA followups I had published. Testing this alongside 24857 I do not think these are conflicting features - there could be more tie in between them, but the use cases are very different. The RFC here says this is largely for serial/multipart items - and it works well for that. It supports MARC for import/export and displays in the results, and adds info, even if no items are present Bug 24857 is largely aimed at monographic records - allowing libraries to internally and easily create groups of items that can then be dealt with as a group for holds. It does not require additional cataloging, nor do they display, they are logical groupings for holds purposes (with bug 24860) I don't believe a library would want to implement both features at the same time, if there was call for that then they could be more integrated (connect holding to group or vice versa) but I think as base features both stand alone Created attachment 131458 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131459 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131460 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131461 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131462 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131463 [details] [review] Bug 20447: Fixes for bug 17600 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131464 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131465 [details] [review] Bug 20447: (QA follow-up) Unify SQL queries formatting Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131466 [details] [review] Bug 20447: (QA follow-up) Change 3 pushes to `() x 3` for readability Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131467 [details] [review] Bug 20447: (QA follow-up) SummaryHoldings preferences should be 1/0 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131468 [details] [review] Bug 20447: (QA follow-up) Remove warning when frameworkcode is undef Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131469 [details] [review] Bug 20447: (QA follow-up) Fix framework switching bug Form's parameter never used because $record was always defined, with new or with existing holding, but always. Fixed by adding incoming $holding_id presence check. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131470 [details] [review] Bug 20447: (QA follow-up) Fix full-path call for xml_escape Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131471 [details] [review] Bug 20447: (QA follow-up) Remove undef warnings and add UX note Use of uninitialized value in concatenation (.) or string at /usr/share/koha/lib/Koha/UI/Form/Builder/Item.pm line 255. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131472 [details] [review] Bug 20447: (QA follow-up) include Koha::Holdings where needed (and remove uses where don't) Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131473 [details] [review] Bug 20447: (QA follow-up) undefined $mode warning remove When POST /intranet/cataloguing/addholding.pl happens from any /cgi-bin/koha/cataloguing/addholding.pl?op=edit&biblionumber=XXX&holding_id=YYY it has "$mode" undefined but comes to code "$mode ne 'popup'" hence the warning. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131474 [details] [review] Bug 20447: (QA follow-up) typo, framework= should be frameworkcode= we don't have 'framework' param parsed in addholding.pl Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131475 [details] [review] Bug 20447: (QA follow-up) bugfix: holding_id not sent down from params and was not taken into account Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131476 [details] [review] Bug 20447: (QA follow-up) bugfix: itemcallnumber pref wasn't taken into account Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131477 [details] [review] Bug 20447: (QA follow-up) UX improvement: merge point of values should be invisible. This is requested by customers, i.e. no pipe separate char between (librarians said they using it also for copy-pasting, and it confuses patrons because they read roman numbers with this | wrongly) Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131478 [details] [review] Bug 20447: (QA follow-up) Warning suppression tab might be undefined (sidenote: probably tags->subfields->tabs structures should be defined more) Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131479 [details] [review] Bug 20447: (QA follow-up) bugfix: undefined holding_id in code on store After saving to DB code didn't get back "last inserted ID" so wasn't able to allow for interface to go back to newly created holding ID Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131480 [details] [review] Bug 20447: (QA follow-up) Warning removal: undef param 'sep' expected to be string in further processing Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131481 [details] [review] Bug 20447: (QA follow-up) Bugfix: Do not force first item for acqorder from biblionumber, but use framework. Note / bigger bug: this happened only for first added item, all next other items in same basket were added without sending biblionumber as in original Koha code: so, this is very suspicious change without any person to explain why. This rolled-back code looks like unfinished feature or mistake. So I will leave this comment especially for future checks, if extension or this feature development will be needed. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 131482 [details] [review] Bug 20447: (follow-up) Get frameworks as list Signed-off-by: Nick Clemens <nick@bywatersolutions.com> At the moment this patch set doesn't apply, but it might make sense to wait with the rebase until 22.05 is out the door and then make a focused effort. Unfortunately we only have a week now until soft feature freeze and I feel this would be too huge to get tested well in this timeframe :( Applying: Bug 20447: MARC Holdings support Using index info to reconstruct a base tree... M C4/Biblio.pm M C4/ImportBatch.pm M C4/Items.pm M C4/Search.pm M C4/XSLT.pm M Koha/Biblio.pm M Koha/Item.pm M catalogue/detail.pl M cataloguing/additem.pl M installer/data/mysql/kohastructure.sql M installer/data/mysql/mandatory/auth_val_cat.sql M installer/data/mysql/mandatory/sysprefs.sql M koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt M koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt M koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt M koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt M koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl M t/db_dependent/Koha/Biblio.t M t/db_dependent/Koha/Item.t M t/lib/TestBuilder.pm M tools/stage-marc-import.pl Falling back to patching base and 3-way merge... Auto-merging tools/stage-marc-import.pl Auto-merging t/lib/TestBuilder.pm Auto-merging t/db_dependent/Koha/Item.t Auto-merging t/db_dependent/Koha/Biblio.t Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt Auto-merging installer/data/mysql/mandatory/sysprefs.sql Auto-merging installer/data/mysql/mandatory/auth_val_cat.sql Auto-merging installer/data/mysql/kohastructure.sql Auto-merging cataloguing/additem.pl Auto-merging catalogue/detail.pl Auto-merging Koha/Item.pm Auto-merging Koha/Biblio.pm CONFLICT (content): Merge conflict in Koha/Biblio.pm Auto-merging C4/XSLT.pm Auto-merging C4/Search.pm Auto-merging C4/Items.pm Auto-merging C4/ImportBatch.pm Auto-merging C4/Biblio.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 20447: MARC Holdings support 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-20447-MARC-Holdings-support-BYfeaD.patch I will rebase, this one is in our production so it's important. I am now doing intermediate rebase, this one will go on tag v22.04.05 (commit hash 75372f012b4ecc321a0658f22ae4f3700502f018). Intermediate, because in the next step after below pre-rebase (pre- because not yet on current master), I plan to squash and re-factor TOO BIG to debug and do future rebasings code, and this will help us to rebase & sign off much cleaner code later. And also, it will include 7 more (!) bug fixes patches over the current state of the things above. Created attachment 139865 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Co-authored-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139866 [details] [review] Bug 20447: Add holding_id to REST API item definition Allows items to be properly returned, including the holding_id field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139867 [details] [review] Bug 20447: Add a batch rebuild script for holdings table This script can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139868 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139869 [details] [review] Bug 20447: (QA follow-up) Update for bug 26703 Reverse the title element content as per updated guidelines. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139870 [details] [review] Bug 20447: Fixes for bug 17600 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139871 [details] [review] Bug 20447: DBIX schema changes Rebased-by: Andrew Nugged <nugged@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139872 [details] [review] Bug 20447: (QA follow-up) SummaryHoldings preferences should be 1/0 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139873 [details] [review] Bug 20447: (QA follow-up) Remove warning when frameworkcode is undef Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139874 [details] [review] Bug 20447: (QA follow-up) Fix framework switching bug Form's parameter never used because $record was always defined, with new or with existing holding, but always. Fixed by adding incoming $holding_id presence check. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139875 [details] [review] Bug 20447: (QA follow-up) Fix full-path call for xml_escape Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139876 [details] [review] Bug 20447: (QA follow-up) Remove undef warnings and add UX note Use of uninitialized value in concatenation (.) or string at /usr/share/koha/lib/Koha/UI/Form/Builder/Item.pm line 255. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139877 [details] [review] Bug 20447: (QA follow-up) include Koha::Holdings where needed (and remove uses where don't) Created attachment 139878 [details] [review] Bug 20447: (QA follow-up) undefined $mode warning remove When POST /intranet/cataloguing/addholding.pl happens from any /cgi-bin/koha/cataloguing/addholding.pl?op=edit&biblionumber=XXX&holding_id=YYY it has "$mode" undefined but comes to code "$mode ne 'popup'" hence the warning. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139879 [details] [review] Bug 20447: (QA follow-up) typo, framework= should be frameworkcode= we don't have 'framework' param parsed in addholding.pl Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139880 [details] [review] Bug 20447: (QA follow-up) bugfix: holding_id not sent down from params and was not taken into account Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139881 [details] [review] Bug 20447: (QA follow-up) bugfix: itemcallnumber pref wasn't taken into account Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139882 [details] [review] Bug 20447: (QA follow-up) UX improvement: merge point of values should be invisible. This is requested by customers, i.e. no pipe separate char between (librarians said they using it also for copy-pasting, and it confuses patrons because they read roman numbers with this | wrongly) Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139883 [details] [review] Bug 20447: (QA follow-up) Warning suppression tab might be undefined (sidenote: probably tags->subfields->tabs structures should be defined more) Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139884 [details] [review] Bug 20447: (QA follow-up) bugfix: undefined holding_id in code on store After saving to DB code didn't get back "last inserted ID" so wasn't able to allow for interface to go back to newly created holding ID Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139885 [details] [review] Bug 20447: (QA follow-up) Warning removal: undef param 'sep' expected to be string in further processing Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139886 [details] [review] Bug 20447: (QA follow-up) Bugfix: Do not force first item for acqorder from biblionumber, but use framework. Note / bigger bug: this happened only for first added item, all next other items in same basket were added without sending biblionumber as in original Koha code: so, this is very suspicious change without any person to explain why. This rolled-back code looks like unfinished feature or mistake. So I will leave this comment especially for future checks, if extension or this feature development will be needed. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139887 [details] [review] Bug 20447: (follow-up) Get frameworks as list Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139888 [details] [review] Bug 20447: (QA follow-up) Fix compatibility to 22.05 Fix to conform with 22.05 Koha::Object's 'search' wantarray removal ( https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29844 ) Created attachment 139889 [details] [review] Bug 20447: (QA follow-up) Fix atomic update to be more idempotent Fix atomic update not to add bad ACQ 952k row into marc_subfield_structure when no ACQ present at all Created attachment 139890 [details] [review] Bug 20447: (QA follow-up) Whitespaces in .css file fixes Created attachment 139891 [details] [review] Bug 20447: (QA follow-up) .css file alignment as in addbiblio.css Created attachment 139892 [details] [review] Bug 20447: (QA follow-up) detail.tt rebase cimpatibility fixes Created attachment 139893 [details] [review] Bug 20447: (QA follow-up) addholding.tt rebase compatibility fixes Created attachment 139894 [details] [review] Bug 20447: (QA follow-up) Show blocking full-screen error if biblio record removed elsewhere (biblio id not found). Rebased-by: Andrew Nugged <nugged@gmail.com> Now I will combine changes into less scattered code and a smaller number of patches (also because I am now co-authoring this task for a while). Because so many patches patch other patches :) and evolutionary old master branch compatibility changes should be just in 1st commit, so this burden is mostly impossible to rebase over the current master (but we need to). This should help. Created attachment 139895 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Smaller notes to task parts (batch rebuild script): --- Includes misc/batchRebuildHoldingsTables.pl: a batch rebuild script for holdings table which can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. --- End of smaller notes Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Co-authored-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 139896 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> All intermediate patches were obsoleted and merged into the final solution, as explained in comment #683 (this is for history to note how it was before): 139865 - Bug 20447: MARC Holdings support 139866 - Bug 20447: Add holding_id to REST API item definition 139867 - Bug 20447: Add a batch rebuild script for holdings table 139868 - Bug 20447: Add import/export support for holdings 139869 - Bug 20447: (QA follow-up) Update for bug 26703 139870 - Bug 20447: Fixes for bug 17600 139871 - Bug 20447: DBIX schema changes 139872 - Bug 20447: (QA follow-up) SummaryHoldings preferences should be 1/0 139873 - Bug 20447: (QA follow-up) Remove warning when frameworkcode is undef 139874 - Bug 20447: (QA follow-up) Fix framework switching bug 139875 - Bug 20447: (QA follow-up) Fix full-path call for xml_escape 139876 - Bug 20447: (QA follow-up) Remove undef warnings and add UX note 139877 - Bug 20447: (QA follow-up) include Koha::Holdings where needed (and remove uses where don't) 139878 - Bug 20447: (QA follow-up) undefined $mode warning remove 139879 - Bug 20447: (QA follow-up) typo, framework= should be frameworkcode= 139880 - Bug 20447: (QA follow-up) bugfix: holding_id not sent down from params 139881 - Bug 20447: (QA follow-up) bugfix: itemcallnumber pref wasn't taken into account 139882 - Bug 20447: (QA follow-up) UX improvement: merge point of values should be invisible. 139883 - Bug 20447: (QA follow-up) Warning suppression 139884 - Bug 20447: (QA follow-up) bugfix: undefined holding_id in code on store 139885 - Bug 20447: (QA follow-up) Warning removal: undef 139886 - Bug 20447: (QA follow-up) Bugfix: Do not force first item for acqorder from biblionumber, but use framework. 139887 - Bug 20447: (follow-up) Get frameworks as list 139888 - Bug 20447: (QA follow-up) Fix compatibility to 22.05 139889 - Bug 20447: (QA follow-up) Fix atomic update to be more idempotent 139890 - Bug 20447: (QA follow-up) Whitespaces in .css file fixes 139891 - Bug 20447: (QA follow-up) .css file alignment as in addbiblio.css 139892 - Bug 20447: (QA follow-up) detail.tt rebase cimpatibility fixes 139893 - Bug 20447: (QA follow-up) addholding.tt rebase compatibility fixes 139894 - Bug 20447: (QA follow-up) Show blocking full-screen error if biblio record removed elsewhere (biblio id not found). The above is still based on tag v22.04.05 (commit hash 75372f012b4ecc321a0658f22ae4f3700502f018). Now I am doing rebase to the latest master. Any news? I will publish this week rebased (many changes & few more bugfixes). I will ask the community to push this ticket to master if possible. + I will put a plan on what is more here that should be improved (there is still a lack of batch processing, but our team in the National Library of Finland will work on this further). It's quite late in the current cycle and we already have a lot of big and small developments in the queues that we are trying to finish for the 23.11 release in time. I believe it would be better to focus on this one right after the release and try to get it pushed early for 24.05. Maybe a dedicated sandbox could also help in getting this tested. Created attachment 157492 [details] [review] Bug 20447: MARC Holdings support Enable by setting "SummaryHoldings" option to "Use". Smaller notes to task parts (batch rebuild script): --- Includes misc/batchRebuildHoldingsTables.pl: a batch rebuild script for holdings table which can be used to update the columns in the holdings table from the MARC record using the rules in the framework. In practice this will only be required if the koha link field is changed in the framework. --- End of smaller notes Sponsored-by: The National Library of Finland Co-authored-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Co-authored-by: Andrew Nugged <nugged@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 157493 [details] [review] Bug 20447: Add import/export support for holdings https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20447 Adds support for importing and exporting biblios with interleaved holdings. Test plan: 1.) Import the MARCXML file attached to the bug with the -holdings parameter: misc/migration_tools/bulkmarcimport.pl -biblios -holdings -file holdings.xml -m MARCXML 2.) Verify that the holdings records were imported. 3.) Export the same records with holdings: misc/export_records.pl --record-type=bibs --starting_biblionumber 103 --ending_biblionumber 104 --filename=holdings_export.xml --format=xml --holdings 4.) Verify that both bibliographic and holding records are exported. 5.) Verify that tests in t/db_dependent/Exporter/Record.t pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> PerlKohaCon.fi talk by this topic: " ... about Holdings what are those and how they are used when items are not enough: in our case in the National Library in Finland. Why do we need to have Holdings and then something about Holdings in Koha in general, and what kind of improvements we do need to use those better ones, ... " is here: https://www.youtube.com/watch?v=d2i1dW9CKAs&list=PLa9kvrI3VLf7ZvLvJk9EtlnF2OVETvddm&index=20 (In reply to Michal Denar from comment #688) Michal, can you check this: https://wiki.koha-community.org/wiki/Road_map_24.05#Project_proposals -> Project proposals -> Add support for MARC holdings records. Others who are also interested in this feature appear: we need more supporters in one week there, so if there is enough support, this ticket will be included in the 24.05 plan. Please sign in there. merge conflicts in applying in ktd |