Bug 37305

Summary: Remove C4::Biblio::prepare_marc_host and use Koha::Biblio->generate_marc_host_field in preference
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: Passed QA --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: flyingendpaper, lucas
Version: unspecified   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 37364    
Bug Blocks: 39545    
Attachments: Bug 37305: Subfield order matters
Bug 37305: Remove C4::Biblio::prepare_host_field
Bug 37305: Subfield order matters
Bug 37305: Remove C4::Biblio::prepare_host_field
Bug 37305: Subfield order matters
Bug 37305: Remove C4::Biblio::prepare_host_field
Bug 37305: Subfield order matters
Bug 37305: Remove C4::Biblio::prepare_host_field
Bug 37305: Subfield order matters
Bug 37305: Remove C4::Biblio::prepare_host_field
Bug 37305: Subfield order matters
Bug 37305: Remove C4::Biblio::prepare_host_field
Bug 37305: (follow-up) Only add control number subfield when defined

Description Martin Renvoize (ashimema) 2024-07-10 13:10:19 UTC
We introduce the new method and add unit tests for it in bug 29560. We should now be able to remove the untested and non-object-oriented method from C4::Biblio.

It would also be nice to start working toward merging UseControlNumber and EasyAnalytics and thus also remove the very similar C4::Biblio::PrepareMarcHost() method.
Comment 1 Martin Renvoize (ashimema) 2024-07-17 10:07:33 UTC
Created attachment 169078 [details] [review]
Bug 37305: Subfield order matters

This patch updates the use of a hash to store and then pass subfields to
the MARC::Field new method to an array with key/value pairs using fat comma's.

This allows us to maintain the integrity check that we're passing string
as the first arguament in each pair whilst also allowing us to ensure
fields are passed in the order they are found.
Comment 2 Martin Renvoize (ashimema) 2024-07-17 10:07:36 UTC
Created attachment 169079 [details] [review]
Bug 37305: Remove C4::Biblio::prepare_host_field

This patch updates the only use of prepare_host_field to use the well
tested Koha::Biblio->generate_marc_host_field method instead and then
removes the aforementioned prepare_host_field method from C4::Biblio.

Test plan
1) Ensure the "enhanced workflow" for creating analytics still functions
   as expected.
Comment 3 Martin Renvoize (ashimema) 2025-05-06 14:24:18 UTC Comment hidden (obsolete)
Comment 4 Martin Renvoize (ashimema) 2025-05-06 14:24:20 UTC Comment hidden (obsolete)
Comment 5 Martin Renvoize (ashimema) 2025-05-06 14:25:14 UTC
Just a quick rebase to confirm this all still applies after the tidy
Comment 6 Martin Renvoize (ashimema) 2025-06-26 14:54:17 UTC
Created attachment 183540 [details] [review]
Bug 37305: Subfield order matters

This patch updates the use of a hash to store and then pass subfields to
the MARC::Field new method to an array with key/value pairs using fat comma's.

This allows us to maintain the integrity check that we're passing string
as the first arguament in each pair whilst also allowing us to ensure
fields are passed in the order they are found.
Comment 7 Martin Renvoize (ashimema) 2025-06-26 14:54:19 UTC
Created attachment 183541 [details] [review]
Bug 37305: Remove C4::Biblio::prepare_host_field

This patch updates the only use of prepare_host_field to use the well
tested Koha::Biblio->generate_marc_host_field method instead and then
removes the aforementioned prepare_host_field method from C4::Biblio.

Test plan
1) Ensure the "enhanced workflow" for creating analytics still functions
   as expected.
Comment 8 Martin Renvoize (ashimema) 2025-06-26 15:15:30 UTC
Created attachment 183545 [details] [review]
Bug 37305: Subfield order matters

This patch updates the use of a hash to store and then pass subfields to
the MARC::Field new method to an array with key/value pairs using fat comma's.

This allows us to maintain the integrity check that we're passing string
as the first arguament in each pair whilst also allowing us to ensure
fields are passed in the order they are found.

Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 9 Martin Renvoize (ashimema) 2025-06-26 15:15:31 UTC
Created attachment 183546 [details] [review]
Bug 37305: Remove C4::Biblio::prepare_host_field

This patch updates the only use of prepare_host_field to use the well
tested Koha::Biblio->generate_marc_host_field method instead and then
removes the aforementioned prepare_host_field method from C4::Biblio.

Test plan
1) Ensure the "enhanced workflow" for creating analytics still functions
   as expected.

Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 10 Martin Renvoize (ashimema) 2025-06-26 15:16:07 UTC
Tested and moved along as part of bug 39545.
Comment 11 Lucas Gass (lukeg) 2025-06-26 17:03:27 UTC
The QA tool tells me this is not tidy:

Processing files
[RUNNING] C4/Biblio.pm
[RUNNING] Koha/Biblio.pm
[RUNNING] cataloguing/addbiblio.pl
[PASS] cataloguing/addbiblio.pl
[FAIL] Koha/Biblio.pm
[PASS] C4/Biblio.pm
Result:
[PASS] C4/Biblio.pm
[FAIL] Koha/Biblio.pm
   FAIL	  tidiness
		File is not tidy, please run `perl misc/devel/tidy.pl Koha/Biblio.pm`
[PASS] cataloguing/addbiblio.pl
Comment 12 Martin Renvoize (ashimema) 2025-06-27 11:40:38 UTC
Created attachment 183598 [details] [review]
Bug 37305: Subfield order matters

This patch updates the use of a hash to store and then pass subfields to
the MARC::Field new method to an array with key/value pairs using fat comma's.

This allows us to maintain the integrity check that we're passing string
as the first arguament in each pair whilst also allowing us to ensure
fields are passed in the order they are found.

Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 13 Martin Renvoize (ashimema) 2025-06-27 11:40:40 UTC
Created attachment 183599 [details] [review]
Bug 37305: Remove C4::Biblio::prepare_host_field

This patch updates the only use of prepare_host_field to use the well
tested Koha::Biblio->generate_marc_host_field method instead and then
removes the aforementioned prepare_host_field method from C4::Biblio.

Test plan
1) Ensure the "enhanced workflow" for creating analytics still functions
   as expected.

Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 14 Martin Renvoize (ashimema) 2025-06-27 11:41:18 UTC
I've fixed my commit hook now.. appologies.
Comment 15 Lucas Gass (lukeg) 2025-06-30 13:45:50 UTC
I believe that t/db_dependent/Koha/Biblio.t tests are failing with this applied. Please check.
Comment 16 Martin Renvoize (ashimema) 2025-07-03 12:56:39 UTC
Created attachment 183755 [details] [review]
Bug 37305: Subfield order matters

This patch updates the use of a hash to store and then pass subfields to
the MARC::Field new method to an array with key/value pairs using fat comma's.

This allows us to maintain the integrity check that we're passing string
as the first arguament in each pair whilst also allowing us to ensure
fields are passed in the order they are found.

Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 17 Martin Renvoize (ashimema) 2025-07-03 12:56:41 UTC
Created attachment 183756 [details] [review]
Bug 37305: Remove C4::Biblio::prepare_host_field

This patch updates the only use of prepare_host_field to use the well
tested Koha::Biblio->generate_marc_host_field method instead and then
removes the aforementioned prepare_host_field method from C4::Biblio.

Test plan
1) Ensure the "enhanced workflow" for creating analytics still functions
   as expected.

Sponsored-by: Open Fifth <https://openfifth.co.uk>
Signed-off-by: Heather Hernandez <flyingendpaper@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 18 Martin Renvoize (ashimema) 2025-07-03 12:56:43 UTC
Created attachment 183757 [details] [review]
Bug 37305: (follow-up) Only add control number subfield when defined

The previous commit introduced a bug where the control number subfield 'w'
was being added to the MARC field even when undefined, causing test failures.

This patch adds a condition to only push the control number subfield when
the value is actually defined, matching the pattern used for other subfields
in the method.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>