1) I think we should adjust to also take RDA records using 264 into account here. At least for us these are now the majority of records. + # Publication + if ( $host_field = $marc_host->field('260') ) { + my $s = $host_field->as_string('abc'); + if ($s) { + $sfd{d} = $s; + } + } 773$d is not repeatable, so we should avoid to create 2 entries. Maybe check for existence of 264 first and then fallback to 260 if the first is not found? 260 and 264 are both repeatable. I assume this either uses the first or last entry - can you tell? Ideally it might be good to prefer the entry with first indicator = 3 (current/latest), if it exists: https://www.loc.gov/marc/bibliographic/bd264.html 2) This should take $n$p into account as well, as otherwise a multi-part work will not be distinguishable from the title: + # Title + if ( $host_field = $marc_host->field('245') ) { + my $s = $host_field->as_string('ab'); + if ($s) { + $sfd{t} = $s; + } + } I think this could just be changed to: abnp for the subfield list.
> 773$d is not repeatable, so we should avoid to create 2 entries. Maybe check > for existence of 264 first and then fallback to 260 if the first is not > found? This should work! > 260 and 264 are both repeatable. I assume this either uses the first or last > entry - can you tell? Yes--both the 260 and the 264 would have first indicator = 3 for "current, latest." > Ideally it might be good to prefer the entry with first indicator = 3 > (current/latest), if it exists... Yes--this would work. Examples are here, on p.43: https://www.loc.gov/aba/pcc/conser/CCM/module35.pdf E.g.: 264 #1 $3 1990-1994: $a Salem, N.H. : $b Butterworth Legal Publishers 264 21 $3 1995-<1996>: $a Charlottesville, VA : $b Michie 264 21 $3 <1998>-1999: $a Charlottesville, VA : $b LEXIS Law Pub. 264 31 $3 2000- : $a Huntington, NY : $b Juris Pub., $c c1990- > 2) This should take $n$p into account as well, as otherwise a multi-part > work will not be distinguishable from the title: Yes!!! > I think this could just be changed to: abnp for the subfield list. This order can be different in the 245, so IMO should be preserved in the 773, otherwise the title will make no sense, e.g. this 245's order is apnpb (from the MARC21 standard): 245 00$aDeutsche Bibliographie.$pWöchentliches Verzeichnis.$nReihe B,$pBeilage, Erscheinungen ausserhalb des Verlagsbuchhandels :$bAmtsblatt der Deutschen Bibliothek. So can the order of the subfields in the 245 be preserved in the created 773? --h2
Hi Heather, I forgot that n and p are also repeatable... Not sure if keeping sequence is hard/easy, but we can at least ask for it :)
I've got a patch for the 260/264 stuff and I'm working on the 245 now. Order could be fun, need to also take punctuation into consideration given recent chat's. I can maintain order fairly trivially I think, though repeats and punctuation add a little extra challenge.
I think as Koha at the moment expects the punctuation to be there, you only need to make sure that we have a space between the subfields and otherwise the string can be unchanged.
Ooohh.. ordering is already correct: > as_string( [$subfields] [, $delimiter] ) >Returns a string of all subfields run together. A space is added to the result between each subfield, unless the delimiter parameter is passed. The tag number and subfield character are not included. >Subfields appear in the output string in the order in which they occur in the field. >If $subfields is specified, then only those subfields will be included.
Created attachment 169075 [details] [review] Bug 37364: Add requested RDA handling to generate_marc_host_field This patch adds handling for the 264 field in precedence to the 260 for the generate_marc_host_field method and also adds subfields n and p to the 245 handling.
Created attachment 169076 [details] [review] Bug 37364: Add requested RDA handling to generate_marc_host_field This patch adds handling for the 264 field in precedence to the 260 for the generate_marc_host_field method and also adds subfields n and p to the 245 handling.
Created attachment 169077 [details] [review] Bug 37364: Unit tests
Created attachment 181978 [details] [review] Bug 37364: Add requested RDA handling to generate_marc_host_field This patch adds handling for the 264 field in precedence to the 260 for the generate_marc_host_field method and also adds subfields n and p to the 245 handling.
Created attachment 181979 [details] [review] Bug 37364: Unit tests
I really wish people would come back to things when you immediately work on them for them ;) It's been nearly a year and a bunch of other people have now done work in the same area's and this bug got no attention at all.
# Personal Note First of all, I'd like to say I'm sorry that your work was ignored for so long. That's definitely not fun. I am new to this, but I still tried to take a look at it. To be honest, I am not at all familiar with the MARC format, so it's hard for me to sign off on the feature, but I did run your unit tests and I've provided the results below. If I may, I have one small piece of advice that might help: I recommend you provide a clear test plan. Otherwise, it's difficult to know how to test the patch/what needs to be tested, which usually limits potential testers to either people who are already involved in the patch or people who have the time and expertise to read and understand the entire discussion. # Test Results Unit tests do not pass. # Test Execution 1. Apply patch 2. Run `prove ./t/db_dependent/Koha/Biblio.t`. 3. Test fails. See [Prove Output] below. 4. Remove patch with `git reset --hard origin/main`. 5. Run `prove ./t/db_dependent/Koha/Biblio.t`. 6. All tests pass. # Prove Output prove ./t/db_dependent/Koha/Biblio.t ./t/db_dependent/Koha/Biblio.t .. 39/41 # Failed test 'no warnings' # at /usr/local/share/perl/5.38.2/Test/Builder.pm line 187. # There were 4 warning(s) # Previous test 0 '' # Argument " " isn't numeric in numeric eq (==) at /inlibro/git/koha-master-dev-linwei/Koha/Biblio.pm line 1960. # at /inlibro/git/koha-master-dev-linwei/Koha/Biblio.pm line 1960. # Koha::Biblio::generate_marc_host_field(Koha::Biblio=HASH(0x60810b9c8cc8)) called at ./t/db_dependent/Koha/Biblio.t line 1022 # main::__ANON__() called at /usr/local/share/perl/5.38.2/Test/Builder.pm line 368 # eval {...} called at /usr/local/share/perl/5.38.2/Test/Builder.pm line 368 # Test::Builder::subtest(Test::Builder=HASH(0x60810057c040), "generate_marc_host_field", CODE(0x6081003df578)) called at /usr/local/share/perl/5.38.2/Test/More.pm line 831 # Test::More::subtest("generate_marc_host_field", CODE(0x6081003df578)) called at ./t/db_dependent/Koha/Biblio.t line 1099 # # ---------- # Previous test 10 'MARC::Field->subfield(w) returns undef when "UseControlNumber" is disabled' # Argument " " isn't numeric in numeric eq (==) at /inlibro/git/koha-master-dev-linwei/Koha/Biblio.pm line 1960. # at /inlibro/git/koha-master-dev-linwei/Koha/Biblio.pm line 1960. # Koha::Biblio::generate_marc_host_field(Koha::Biblio=HASH(0x60810b9c8cc8)) called at ./t/db_dependent/Koha/Biblio.t line 1036 # main::__ANON__() called at /usr/local/share/perl/5.38.2/Test/Builder.pm line 368 # eval {...} called at /usr/local/share/perl/5.38.2/Test/Builder.pm line 368 # Test::Builder::subtest(Test::Builder=HASH(0x60810057c040), "generate_marc_host_field", CODE(0x6081003df578)) called at /usr/local/share/perl/5.38.2/Test/More.pm line 831 # Test::More::subtest("generate_marc_host_field", CODE(0x6081003df578)) called at ./t/db_dependent/Koha/Biblio.t line 1099 # # ---------- # Previous test 11 'MARC::Field->subfield(w) returns content from 003 and 001 when "UseControlNumber" is enabled' # Argument " " isn't numeric in numeric eq (==) at /inlibro/git/koha-master-dev-linwei/Koha/Biblio.pm line 1960. # at /inlibro/git/koha-master-dev-linwei/Koha/Biblio.pm line 1960. # Koha::Biblio::generate_marc_host_field(Koha::Biblio=HASH(0x60810b95d570)) called at ./t/db_dependent/Koha/Biblio.t line 1048 # main::__ANON__() called at /usr/local/share/perl/5.38.2/Test/Builder.pm line 368 # eval {...} called at /usr/local/share/perl/5.38.2/Test/Builder.pm line 368 # Test::Builder::subtest(Test::Builder=HASH(0x60810057c040), "generate_marc_host_field", CODE(0x6081003df578)) called at /usr/local/share/perl/5.38.2/Test/More.pm line 831 # Test::More::subtest("generate_marc_host_field", CODE(0x6081003df578)) called at ./t/db_dependent/Koha/Biblio.t line 1099 # # ---------- # Previous test 12 'MARC::Field->subfield(d) returns content from 264 in prefernce to 260' # Argument " " isn't numeric in numeric eq (==) at /inlibro/git/koha-master-dev-linwei/Koha/Biblio.pm line 1960. # at /inlibro/git/koha-master-dev-linwei/Koha/Biblio.pm line 1960. # Koha::Biblio::generate_marc_host_field(Koha::Biblio=HASH(0x60810ba88e68)) called at ./t/db_dependent/Koha/Biblio.t line 1060 # main::__ANON__() called at /usr/local/share/perl/5.38.2/Test/Builder.pm line 368 # eval {...} called at /usr/local/share/perl/5.38.2/Test/Builder.pm line 368 # Test::Builder::subtest(Test::Builder=HASH(0x60810057c040), "generate_marc_host_field", CODE(0x6081003df578)) called at /usr/local/share/perl/5.38.2/Test/More.pm line 831 # Test::More::subtest("generate_marc_host_field", CODE(0x6081003df578)) called at ./t/db_dependent/Koha/Biblio.t line 1099 # # Looks like you failed 1 test of 41. ./t/db_dependent/Koha/Biblio.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/41 subtests Test Summary Report ------------------- ./t/db_dependent/Koha/Biblio.t (Wstat: 256 (exited 1) Tests: 41 Failed: 1) Failed test: 41 Non-zero exit status: 1 Files=1, Tests=41, 17 wallclock secs ( 0.03 usr 0.01 sys + 11.19 cusr 1.87 csys = 13.10 CPU) Result: FAIL
Created attachment 182393 [details] [review] Bug 37364: Unit tests This patch adds unit tests to confirm that the 773 record created by Koha::Biblio now takes account of RDA updates and subfield ordering. Test plan 1. Apply patch 2. Run `prove ./t/db_dependent/Koha/Biblio.t`. 3. Test fails. 4. Apply next patch in series 5. Run `prove ./t/db_dependent/Koha/Biblio.t`. 6. All tests pass.
Created attachment 182394 [details] [review] Bug 37364: Add requested RDA handling to generate_marc_host_field This patch adds handling for the 264 field in precedence to the 260 for the generate_marc_host_field method and also adds subfields n and p to the 245 handling. Test plan 1) The routine is currently only called during bundle linking so... 1a) Creating a new bundle * Add a new bib record * Mark the bib record as a 'collection' type by setting leader position 7 to 'c' * Add a new item to this bib record * You should see a new 'Manage bundle' button available in the 'Actions' column of the Holdings table. * Clicking 'Manage bundle' should expand the table to include a new row directly beneath this one. * Use the new 'Add to bundle' button that appears in this row to trigger a modal that allows entering the barcode of items you wish to add to the bundle * Ensure the 'Add MARC link' checkbox is checked to create 773 links and trigger the code above 2) Inspect the MARC record for the host biblio and confirm the 773 has been built. 3) Run unit tests to confirm they now pass
Thanks for taking a look Lin Wei. My apologies for the curtness.. not my best day. The comment was really for Katrin as she asked for the follow-up here in another bug and I did the work on the same day hoping she'd take a look. Anyway, I should know better and apologise. As for the bug itself, I've improved the test plan and fixed a small bug you highlighted, thanks! Perl is always fun when it comes to `==` vs `eq` especially when MARC::Field->indicator() returns a string even though the content is an integer. All should be working now.
No worries, glad I could help :) # Test Results - Unit tests pass. - Test plan was executed successfully. -> Technically, there is an issue with adding records created via Fast Cataloging to a Bundle/Collection. -> But, Fast Cataloging should not be used, so I am not going to disregard this issue. -> See below for details. # Test Execution 1. Apply 182393. 2. Run `prove ./t/db_dependent/Koha/Biblio.t`. 3. Tests fails (see [Prove Output] below). -> I realize this is normal as the tests are fixed in the following commit. 4. Reset with `git reset --hard origin/main`. 5. Run `prove ./t/db_dependent/Koha/Biblio.t`. 6. Tests pass. 7. Apply both 182393 and 182394. 8. Run `prove ./t/db_dependent/Koha/Biblio.t`. 9. Tests pass. -> Here is where I realize it. 10. Create new 'collection' record. 11. Add new item to this bib record. 12. Holdings > Item I just created > Manage bundle. 13. Add to bundle (Add MARC link = true): 1234, 0001, 0002, 0003, 0004 (barcodes of my test records). 14. Inspect MARC records: - 1234: missing 773 t - 0001: 773 t [title of my collection] - 0002: missing 773 t - 0003: 773 t [title of my collection] -> see [Additional Notes] below regarding the missing MARC tags 15. Create new test record. 16. Create new item for it with barcode = 0004. 17. Add to bundle (Add MARC link = true): 0004. 18: Inspect MARC record: - 0004: 773 t [title of my collection] # Additional Notes Test books are fake books created for testing and have a lot of data missing. Real books are, well, real books with real data. 1234: test book, created via Fast Cataloging 0001: real book, imported from Library of Congress 0002: test book, created via Fast Cataloging 0003: real book, imported from Library of Congress 0004: test book, created via New Record Note that the results, when considering these Additional Notes, show that the patch behaves as expected, except for records created via Fast Cataloging: - These records created via Fast Cataloging show up under: Holdings > Manage bundle. - But, their MARC do not contain a 773 tag. I was informed that users should never use Fast Cataloging, so I am considering this discrepancy as "irrelevant" since the feature works both on real data and properly created fake data. In conclusion, I will be signing off on this patch. Good work! # Prove Output prove ./t/db_dependent/Koha/Biblio.t ./t/db_dependent/Koha/Biblio.t .. 15/41 # Failed test 'MARC::Field->subfield(d) returns content from 264 in preference to 260' # at ./t/db_dependent/Koha/Biblio.t line 1049. # got: 'Publication 260' # expected: 'Publication 264' # Failed test 'MARC::Field->subfield(d) returns content from 264 with indicator 1 = 3 in preference to 264 without' # at ./t/db_dependent/Koha/Biblio.t line 1061. # got: 'Publication 260' # expected: 'Publication 264 Preferred' # Looks like you failed 2 tests of 24. # Failed test 'generate_marc_host_field' # at ./t/db_dependent/Koha/Biblio.t line 1099. ./t/db_dependent/Koha/Biblio.t .. 40/41 # Looks like you failed 1 test of 41. ./t/db_dependent/Koha/Biblio.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/41 subtests Test Summary Report ------------------- ./t/db_dependent/Koha/Biblio.t (Wstat: 256 (exited 1) Tests: 41 Failed: 1) Failed test: 17 Non-zero exit status: 1 Files=1, Tests=41, 18 wallclock secs ( 0.04 usr 0.01 sys + 12.20 cusr 1.91 csys = 14.16 CPU) Result: FAIL
Created attachment 182398 [details] [review] Bug 37364: Unit tests This patch adds unit tests to confirm that the 773 record created by Koha::Biblio now takes account of RDA updates and subfield ordering. Test plan 1. Apply patch 2. Run `prove ./t/db_dependent/Koha/Biblio.t`. 3. Test fails. 4. Apply next patch in series 5. Run `prove ./t/db_dependent/Koha/Biblio.t`. 6. All tests pass. Signed-off-by: Lin Wei Li <lin-wei.li@inLibro.com>
Created attachment 182399 [details] [review] Bug 37364: Add requested RDA handling to generate_marc_host_field This patch adds handling for the 264 field in precedence to the 260 for the generate_marc_host_field method and also adds subfields n and p to the 245 handling. Test plan 1) The routine is currently only called during bundle linking so... 1a) Creating a new bundle * Add a new bib record * Mark the bib record as a 'collection' type by setting leader position 7 to 'c' * Add a new item to this bib record * You should see a new 'Manage bundle' button available in the 'Actions' column of the Holdings table. * Clicking 'Manage bundle' should expand the table to include a new row directly beneath this one. * Use the new 'Add to bundle' button that appears in this row to trigger a modal that allows entering the barcode of items you wish to add to the bundle * Ensure the 'Add MARC link' checkbox is checked to create 773 links and trigger the code above 2) Inspect the MARC record for the host biblio and confirm the 773 has been built. 3) Run unit tests to confirm they now pass Signed-off-by: Lin Wei Li <lin-wei.li@inLibro.com>