When creating a child record (New > New child record from Normal view) the information put in field 773 is very basic, sometimes not complete (e.g. only one ISBN, only 245 $a in 773 $t, 773 $w missing 003 of the host record etc.) and put in random order.
Created attachment 180571 [details] [review] Bug 39545: Better generation of 773 field for a child record (MARC21) When creating a child record (New > New child record from Normal view) the information put in field 773 is very basic, sometimes not complete (e.g. only one ISBN, only 245 $a in 773 $t, 773 $w missing 003 of the host record etc.) and put in random order. Test plan: ========== 1. Find any record to be a host record for an analytical record. You can enrich the record with multiple 020, 490+830 fields, make sure that there are 001 and 003 fields present. 2. From upper menu chose New > New child record. 3. See the generated field 773 - the subfields are incomplete and in random order. 4. Apply the patch ; restart_all. 5. Repeat p. 2.-3. See that the generated 773 field contains now much more information, including 773 $7 and also preset leader.
Created attachment 180572 [details] [review] Bug 39545: Unit tests
Created attachment 180600 [details] [review] Bug 39545: Better generation of 773 field for a child record (MARC21) When creating a child record (New > New child record from Normal view) the information put in field 773 is very basic, sometimes not complete (e.g. only one ISBN, only 245 $a in 773 $t, 773 $w missing 003 of the host record etc.) and put in random order. Test plan: ========== 1. Find any record to be a host record for an analytical record. You can enrich the record with multiple 020, 490+830 fields, make sure that there are 001 and 003 fields present. 2. From upper menu chose New > New child record. 3. See the generated field 773 - the subfields are incomplete and in random order. 4. Apply the patch ; restart_all. 5. Repeat p. 2.-3. See that the generated 773 field contains now much more information, including 773 $7 and also preset leader. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Created attachment 180601 [details] [review] Bug 39545: Unit tests Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
We have three functions for this.. I really want to merge them at some point, but I also don't want them to fall further out of sync in the meantime. Can we also look at PrepMarcHost in the same file, and more importantly in my opinion the generate_marc_host_field method in Koha::Biblio?
Created attachment 181876 [details] [review] Bug 39545: Unit tests Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Created attachment 181877 [details] [review] Bug 39545: (follow-up) Apply same logic to Koha::Biblio
A quick addition on a Friday.. I'll try to return to this and update tests.. I'd love to drop the C4 version and jus use the Koha one if we can manage it :)
Hmm.. you remove subfield s handling here.. coming from the 240$a. was this deliberate?
Created attachment 181970 [details] [review] Bug 39545: (follow-up) Apply same logic to Koha::Biblio This patch updates the Koha::Biblio->generate_marc_host_field method to more closely match the updated logic in C4::Biblio::prep_marc_host. We change a number of things including: 1) Constructinging the 773 subfield generation from a hash to an array to preserve field order. 2) Updating main entry handling for 100/110/111 fields to clone the field and then remove relevant subfields as apposed to fetching only a limited set of subfields. 3) Add handling to generate subfield 7 content depending on LDR and Main Entry information 4) Improve handling for title subfield construction from 245 to respect non-filing characters and remove trailing punctuation 5) Remove trailing punctuation from subfield b when generated from 250 6) Remove generation of subfield s from the 240 field entirely 7) Remove trailing punctuation from subfield d when generated from 260 8) Add generation of subfield k from 800-830 fields
I need a cataloguer to weight in here I'm afraid. I've used the operation of syncing this up in the Koha::Biblio method to do a deep dive back into it. I've noted all the changes explicitly in my commit message, and I'm not entirely sure they're all correct. For instance, I'm not really sure about the punctuation handling and I could see that possibly causing problems for others. I'm also not sure about the removal of subfield s handling.. maybe that needs more consideration. Finally.. I also found whilst updating unit tests, that the regular expression used for punctuation handling in the 245 work of the original patch doesn't catch trailing whitespace which doesn't feel entirely appropriate as in my findings 'as_string' adds such space.
OK.. I've remember where all this got to last time around now.. can we please rework this atop the other bugs in the queue already that heavily affect these area's.. I've put them into the dependencies but I've not rebased the changes here yet..
Rebasing this now, we can drop your patch entirely as the dependencies already removed that method. We can then use my patch that implements your changes against the more modern method. However, I think we're doing too much in one bug here still and we need a clear discussion around the changes
Created attachment 181987 [details] [review] Bug 39545: Better generation of 773 field for a child record (MARC21) When creating a child record (New > New child record from Normal view) the information put in field 773 is very basic, sometimes not complete (e.g. only one ISBN, only 245 $a in 773 $t, 773 $w missing 003 of the host record etc.) and put in random order. Test plan: ========== 1. Find any record to be a host record for an analytical record. You can enrich the record with multiple 020, 490+830 fields, make sure that there are 001 and 003 fields present. 2. From upper menu chose New > New child record. 3. See the generated field 773 - the subfields are incomplete and in random order. 4. Apply the patch ; restart_all. 5. Repeat p. 2.-3. See that the generated 773 field contains now much more information, including 773 $7 and also preset leader. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Rebased-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> The rebase removed the majority of this patch as the prepare_host_field method was removed entirely upstream. The follow-up patch re-implements the same but in the newer Koha::Biblio->generate_marc_host_field method instead. We also dropped the Unit tests that were introduced in the subsequent patch.
Created attachment 181988 [details] [review] Bug 39545: (follow-up) Apply same logic to Koha::Biblio This patch updates the Koha::Biblio->generate_marc_host_field method to more closely match the updated logic in C4::Biblio::prep_marc_host. We change a number of things including: 1) Constructinging the 773 subfield generation from a hash to an array to preserve field order. 2) Updating main entry handling for 100/110/111 fields to clone the field and then remove relevant subfields as apposed to fetching only a limited set of subfields. 3) Add handling to generate subfield 7 content depending on LDR and Main Entry information 4) Improve handling for title subfield construction from 245 to respect non-filing characters and remove trailing punctuation 5) Remove trailing punctuation from subfield b when generated from 250 6) Remove generation of subfield s from the 240 field entirely 7) Remove trailing punctuation from subfield d when generated from 260 8) Add generation of subfield k from 800-830 fields Rebased-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> We dropped the upstream aforementioned C4::Biblio method, but this patch faithfully replicates it's functionality within Koha::Biblio->generate_marc_host_field
Retired cataloger here!! Looking at Comment 15 from Martin, all looks good with these caveats/comments: I really must object to "Remove generation of subfield s from the 240 field entirely" because VAST quantities of records have an incredibly valuable 240 that absolutely must be included in the 773, including records for Bibles, books of the Bible, laws and statutes, and translations. The preservation of this string in the 773 is essential, and frequently used--analytic records/child records are, indeed, created for these entities due to their being issued serially, for chapters/parts being used/assigned in academic settings, analytic/child records being cataloged for relevant treaties and/or conventions issued in compilations, etc. Especially for legal materials, the $s and the $t should be included together--it's normal to include both, anyway. These fields are incredibly important for academic libraries, specialized libraries, government libraries, law libraries, etc. There should also be generation of subfield d from 264, since many catalogs have newer records with a 264 while also having older records with a 260. Hope this helps!! --h2