Bug 35993

Summary: AddBiblio should add 005 just like ModBiblio updates it
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: MARC Bibliographic data supportAssignee: Marcel de Rooy <m.de.rooy>
Status: Pushed to main --- QA Contact: Martin Renvoize <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: kebliss, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5374
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35994
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36054
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00
Attachments: Bug 35993: Add SimpleMarc::update_last_transaction_time
Bug 35993: Apply new routine to ModBiblioMarc
Bug 35993: Add SimpleMarc::update_last_transaction_time
Bug 35993: Apply new routine to ModBiblioMarc
Bug 35993: Apply new routine to ModBiblioMarc
Bug 35993: Apply new routine to ModBiblioMarc
Bug 35993: Add SimpleMarc::update_last_transaction_time
Bug 35993: Apply new routine to ModBiblioMarc
Bug 35993: Add SimpleMarc::update_last_transaction_time
Bug 35993: Apply new routine to ModBiblioMarc
Bug 35993: Add SimpleMarc::update_last_transaction_time
Bug 35993: Apply new routine to ModBiblioMarc
Bug 35993: Add SimpleMarc::update_last_transaction_time
Bug 35993: Apply new routine to ModBiblioMarc
Bug 35993: (follow-up) Fix Merge.t
Bug 35993: (follow-up) Fix MarcOverlayRules.t too
Bug 35993: (follow-up) Another fix for MarcOverlayRules.t

Description Marcel de Rooy 2024-02-05 13:09:11 UTC
Records added via Acquisition do not contain a 005 tag. This is inconsistent.
Comment 1 Marcel de Rooy 2024-02-05 14:16:04 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2024-02-05 14:16:06 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2024-02-06 13:16:03 UTC
Created attachment 161776 [details] [review]
Bug 35993: Add SimpleMarc::update_last_transaction_time

Note: This routine does not check field existence like
ModBiblioMarc does/did (see next patch). So it inserts
a field 005 if it is not present.

Test plan:
Run t/SimpleMARC.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2024-02-06 13:16:05 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2024-02-06 13:19:57 UTC
Created attachment 161778 [details] [review]
Bug 35993: Apply new routine to ModBiblioMarc

Now both AddBiblio and ModBiblio will insert 005 if not present.

Test plan:
Run t/db_dependent/Biblio.t
Add biblio record via acquistion order. Check 005 in the table
biblio_metadata.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2024-02-07 14:14:05 UTC
Created attachment 161805 [details] [review]
Bug 35993: Apply new routine to ModBiblioMarc

Now both AddBiblio and ModBiblio will insert 005 if not present.

Test plan:
Run t/db_dependent/Biblio.t
Add biblio record via acquistion order. Check 005 in the table
biblio_metadata.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Phil Ringnalda 2024-02-09 18:25:27 UTC
Created attachment 161992 [details] [review]
Bug 35993: Add SimpleMarc::update_last_transaction_time

Note: This routine does not check field existence like
ModBiblioMarc does/did (see next patch). So it inserts
a field 005 if it is not present.

Test plan:
Run t/SimpleMARC.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 8 Phil Ringnalda 2024-02-09 18:25:30 UTC
Created attachment 161993 [details] [review]
Bug 35993: Apply new routine to ModBiblioMarc

Now both AddBiblio and ModBiblio will insert 005 if not present.

Test plan:
Run t/db_dependent/Biblio.t
Add biblio record via acquistion order. Check 005 in the table
biblio_metadata.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 9 Lucas Gass 2024-02-09 18:31:50 UTC
Created attachment 161994 [details] [review]
Bug 35993: Add SimpleMarc::update_last_transaction_time

Note: This routine does not check field existence like
ModBiblioMarc does/did (see next patch). So it inserts
a field 005 if it is not present.

Test plan:
Run t/SimpleMARC.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 10 Lucas Gass 2024-02-09 18:31:52 UTC
Created attachment 161995 [details] [review]
Bug 35993: Apply new routine to ModBiblioMarc

Now both AddBiblio and ModBiblio will insert 005 if not present.

Test plan:
Run t/db_dependent/Biblio.t
Add biblio record via acquistion order. Check 005 in the table
biblio_metadata.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 11 Marcel de Rooy 2024-02-12 06:39:09 UTC
> Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Thx Phil and Lucas !
Comment 12 Marcel de Rooy 2024-04-22 11:43:57 UTC
Created attachment 165271 [details] [review]
Bug 35993: Add SimpleMarc::update_last_transaction_time

Note: This routine does not check field existence like
ModBiblioMarc does/did (see next patch). So it inserts
a field 005 if it is not present.

Test plan:
Run t/SimpleMARC.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 13 Marcel de Rooy 2024-04-22 11:43:59 UTC
Created attachment 165272 [details] [review]
Bug 35993: Apply new routine to ModBiblioMarc

Now both AddBiblio and ModBiblio will insert 005 if not present.

Test plan:
Run t/db_dependent/Biblio.t
Add biblio record via acquistion order. Check 005 in the table
biblio_metadata.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 14 Marcel de Rooy 2024-04-22 11:44:22 UTC
Rebased (Biblio.t)
Comment 15 Martin Renvoize 2024-04-23 10:23:52 UTC
Created attachment 165370 [details] [review]
Bug 35993: Add SimpleMarc::update_last_transaction_time

Note: This routine does not check field existence like
ModBiblioMarc does/did (see next patch). So it inserts
a field 005 if it is not present.

Test plan:
Run t/SimpleMARC.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2024-04-23 10:23:55 UTC
Created attachment 165371 [details] [review]
Bug 35993: Apply new routine to ModBiblioMarc

Now both AddBiblio and ModBiblio will insert 005 if not present.

Test plan:
Run t/db_dependent/Biblio.t
Add biblio record via acquistion order. Check 005 in the table
biblio_metadata.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2024-04-23 10:24:18 UTC
Clean code, well tested, works as described.

Passing QA
Comment 18 Katrin Fischer 2024-04-29 07:25:00 UTC
Fixed number of tests in Biblio.t (22 -> 23)
Comment 19 Katrin Fischer 2024-04-29 07:38:29 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 20 Marcel de Rooy 2024-04-29 14:15:20 UTC
Causes Merge.t to fail. Fix coming
Comment 21 Marcel de Rooy 2024-04-29 14:17:25 UTC
Created attachment 165746 [details] [review]
Bug 35993: (follow-up) Fix Merge.t

The change for 005 makes some field compares fail. We should
exclude 005 in the local compare_fields sub.

We do NOT need the compare_fields from AuthoritiesMarc. Removing.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 22 Katrin Fischer 2024-04-29 14:25:42 UTC
Follow-up will be pushed to main.
Comment 23 Marcel de Rooy 2024-04-29 14:36:40 UTC
Created attachment 165748 [details] [review]
Bug 35993: (follow-up) Fix MarcOverlayRules.t too

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Marcel de Rooy 2024-05-02 11:44:14 UTC
Created attachment 166063 [details] [review]
Bug 35993: (follow-up) Another fix for MarcOverlayRules.t

We should not assume comparing within the same second. Now just
remove 005 before comparing.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 25 Katrin Fischer 2024-05-02 13:28:47 UTC
Pushed for 24.05!

Well done everyone, thank you!