Bug 22439 - Duplicated code in PrepHostMarcField and prepare_host_field
Summary: Duplicated code in PrepHostMarcField and prepare_host_field
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
: 30793 37996 (view as bug list)
Depends on: 39545
Blocks: 34482 37710
  Show dependency treegraph
 
Reported: 2019-03-01 09:06 UTC by paxed
Modified: 2026-01-06 20:04 UTC (History)
8 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 22439: Remove C4::Biblio::PrepHostMarcField and consolidate with Koha::Biblio::generate_marc_host_field (10.31 KB, patch)
2025-07-03 15:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 22439: Add comprehensive tests for generate_marc_host_field item parameter (5.92 KB, patch)
2025-07-03 15:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 22439: Remove C4::Biblio::PrepHostMarcField and consolidate with Koha::Biblio::generate_marc_host_field (10.41 KB, patch)
2025-07-12 14:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 22439: Add comprehensive tests for generate_marc_host_field item parameter (6.11 KB, patch)
2025-07-12 14:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 22439: (QA follow-up) add generate_marc_host_field to api/v1/items (946 bytes, patch)
2026-01-06 19:56 UTC, Arthur Suzuki
Details | Diff | Splinter Review
Bug 22439: (QA follow-up) add generate_marc_host_field to api/v1/items (939 bytes, patch)
2026-01-06 20:04 UTC, Arthur Suzuki
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description paxed 2019-03-01 09:06:13 UTC
The C4/Biblio.pm subroutines PrepHostMarcField and prepare_host_field both do the same thing, so should be merged.
Comment 1 paxed 2019-03-04 08:17:28 UTC
In addition:

1) prepare_host_field: if ( $field = $host->field('100') || $host->field('110') || $host->field('11') ) - the '11' should be '111'

2) prepare_host_field has a 240 -> 773b, which should be 250a -> 773b, as it is in PrepHostMarcField

3) There's a difference in how the 773a is set: prepare_host_field uses 100ab | 110ab | 11ab, but PrepHostMarcField uses 100a | 110a | 111a
Comment 2 Martin Renvoize (ashimema) 2024-07-17 10:13:10 UTC
*** Bug 30793 has been marked as a duplicate of this bug. ***
Comment 3 Martin Renvoize (ashimema) 2024-07-17 10:30:10 UTC
I remove C4::Biblio::prepare_host_field in bug 37305, opting to use the tested method I add to Koha::Biblio as `generate_marc_host_field` in bug 29560.

1) The 111 vs 11 mistake in prepare_marc_host is fixed in generate_marc_host_field already.
2) We set 773b t 250ab and 773s to 240a in generate_marc_host_field
3) We stick to 1XXab for 773a in generate_marc_host_field

The outstanding differences between PrepHostMarcField, for EasyAnalytics, and generate_marc_host_field are:

1) Subfield 0 is set to the host biblionumber
2) Subfield 9 is set to the host itemnumber
3) Subfield o is set to the host item barcode
Comment 4 Martin Renvoize (ashimema) 2025-07-03 15:25:42 UTC Comment hidden (obsolete)
Comment 5 Martin Renvoize (ashimema) 2025-07-03 15:25:44 UTC Comment hidden (obsolete)
Comment 6 Martin Renvoize (ashimema) 2025-07-12 14:18:10 UTC
*** Bug 37996 has been marked as a duplicate of this bug. ***
Comment 7 Martin Renvoize (ashimema) 2025-07-12 14:25:13 UTC
Created attachment 184035 [details] [review]
Bug 22439: Remove C4::Biblio::PrepHostMarcField and consolidate with Koha::Biblio::generate_marc_host_field

This patch removes the PrepHostMarcField function from C4::Biblio and enhances the modern
generate_marc_host_field method in Koha::Biblio to handle item-specific data when needed.

The refactoring eliminates code duplication while maintaining all existing functionality.
Item-specific subfields (barcode, itemnumber, biblionumber) are now only included when
EasyAnalyticalRecords is enabled, ensuring the implementation continues to be compliant
with MARC standards.

Changes:
- Remove PrepHostMarcField function from C4::Biblio
- Enhance generate_marc_host_field to accept optional item parameter
- Update cataloguing/addbiblio.pl to use the modern method
- Update cataloguing/linkitem.pl to use the modern method
- Add EasyAnalyticalRecords preference checking to ensure we only add
  the additional non-standard fields when the preference is enabled.

Test plan:
1. Apply patch
2. Enable EasyAnalyticalRecords system preference
3. Create a parent bibliographic record with items
4. Create a child analytical record:
   - Go to Cataloging > New Record
   - Use "Add child record" option for the parent record
   - Choose a specific item from the parent record
   - Verify the 773 field contains subfields $0 (biblionumber), $9 (itemnumber), $o (barcode)
5. Test the linkitem.pl functionality:
   - Create an analytical record without using "Add child record"
   - Go to cataloguing/linkitem.pl with appropriate parameters
   - Link an item using its barcode
   - Verify the 773 field is properly populated
6. Disable EasyAnalyticalRecords system preference
7. Repeat steps 4-5 and verify that subfields $0, $9, and $o are NOT added to the 773 field
8. Verify that other 773 subfields (title, author, etc.) are still properly populated
9. Test with both MARC21 and UNIMARC if available
10. Run tests: prove t/db_dependent/Koha/Biblio.t

Sponsored-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 8 Martin Renvoize (ashimema) 2025-07-12 14:25:16 UTC
Created attachment 184036 [details] [review]
Bug 22439: Add comprehensive tests for generate_marc_host_field item parameter

This patch adds test coverage for the new item parameter functionality in
generate_marc_host_field, ensuring proper behavior with the EasyAnalyticalRecords
system preference.

Test coverage includes:
- MARC21 item functionality with EasyAnalyticalRecords enabled/disabled
- UNIMARC item functionality with proper biblionumber precedence
- Item-specific subfields: $0 (biblionumber), $9 (itemnumber), $o (barcode)
- Verification that item subfields are only added when EasyAnalyticalRecords is enabled
- Backward compatibility when no item parameter is provided

The tests verify that biblionumber takes precedence over control number in
UNIMARC when using item-specific data, matching the original PrepHostMarcField
behavior and ensuring compliance with the easy analytics workflow.

To test:
prove t/db_dependent/Koha/Biblio.t
Comment 9 Chris Cormack 2025-11-23 00:56:33 UTC
Automated patch testing results:

[PASS] ALL PATCHES APPLY CLEANLY

All patches apply successfully to the current codebase.

**Patch Results:**

[PASS] Bug-22439-Add-comprehensive-tests-for-generatemarc.patch (ID: 184036)
   Status: SUCCESS
   Dependencies failed: 39545 (failed to apply)
   + Patch applies cleanly

[PASS] Bug-22439-Remove-C4BiblioPrepHostMarcField-and-con.patch (ID: 184035)
   Status: SUCCESS
   Dependencies failed: 39545 (failed to apply)
   + Patch applies cleanly

**Dependencies:** 39545


---
This is an automated message from the patch testing system.
Tested against branch: main
Comment 10 Martin Renvoize (ashimema) 2025-12-08 15:20:35 UTC
Noted
Comment 11 Arthur Suzuki 2026-01-06 17:41:06 UTC
Hi Martin,
On KTD,
In the test plan step 4:
it says to use "Add child record" but I couldn't find this button, only "link to host record" could be found (either I'm dumb, or it was removed?).

Step 5 worked as a charm.

after step 6 and EasyAnalyticalRecords have been disabled, I don't have access to the button "Link to host record" so I cannot repeat step 5.

Maybe it's just the test plan which needs to be updated?
If that is so then I'm happy to signoff!
Unit tests are fine/not complaining!
Best,
Arthur
Comment 12 Arthur Suzuki 2026-01-06 18:49:02 UTC
(In reply to Arthur Suzuki from comment #11)
> Hi Martin,
> On KTD,
> In the test plan step 4:
> it says to use "Add child record" but I couldn't find this button, only
> "link to host record" could be found (either I'm dumb, or it was removed?).
> 
> Step 5 worked as a charm.
> 
> after step 6 and EasyAnalyticalRecords have been disabled, I don't have
> access to the button "Link to host record" so I cannot repeat step 5.
> 
> Maybe it's just the test plan which needs to be updated?
> If that is so then I'm happy to signoff!
> Unit tests are fine/not complaining!
> Best,
> Arthur

After doing this from BZ37710 : 
2 - Create a bundle on an item by changing the 7th character of the leader to c (go to the marc, click on the button at the right of the field 000 and change the bibliographical level to collection).
I was able to access the "Add child record" button from the parent record.
I think that is worth documenting!
Comment 13 Arthur Suzuki 2026-01-06 19:36:25 UTC
Also,
Just found out that using the "manage bundle" button from the holdings table to add items to the bundle doesn't properly creates the subfields.
This is because it uses the API and the marc field logic is not properly implemented there.

See Koha/REST/V1/Items.pm L324 -> generate_marc_host_field should be called before calling link_marc_host.
Comment 14 Arthur Suzuki 2026-01-06 19:56:27 UTC
Created attachment 190955 [details] [review]
Bug 22439: (QA follow-up) add generate_marc_host_field to api/v1/items
Comment 15 Arthur Suzuki 2026-01-06 20:04:22 UTC
Created attachment 190956 [details] [review]
Bug 22439: (QA follow-up) add generate_marc_host_field to api/v1/items