Bug 23807

Summary: Add Koha::Item->as_marc_field
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: ere.maijala, glasklas, jonathan.druart, katrin.fischer, kyle, martin.renvoize, nick
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00
Bug Depends on:    
Bug Blocks: 23793, 25006, 27007    
Attachments: Bug 23807: Add Koha::Item->as_marc_field
Bug 23807: Unit tests
Bug 23807: Add Koha::Item->as_marc_field
Bug 23807: Unit tests
Bug 23807: Add Koha::Item->as_marc_field
Bug 23807: Unit tests

Description Tomás Cohen Arazi 2019-10-11 17:11:56 UTC
It will be handy to have an OO method to generate a MARC::Field object out of a Koha::Item object. All the required data is in place and it could aid in performance as well.
Comment 1 Tomás Cohen Arazi 2019-10-11 17:44:35 UTC
Created attachment 94029 [details] [review]
Bug 23807: Add Koha::Item->as_marc_field
Comment 2 Tomás Cohen Arazi 2019-10-11 17:49:24 UTC
This is a POC. It uses the default FW when it shouldn't, as each FW might have a different mapping for items.itemnumber.
Comment 3 Tomás Cohen Arazi 2019-10-11 18:26:12 UTC
(In reply to Tomás Cohen Arazi from comment #2)
> This is a POC. It uses the default FW when it shouldn't, as each FW might
> have a different mapping for items.itemnumber.

Well, not POC. It is completely funcional and the default framework is a reasonable fallback. I will try to submit the unit tests, but please consider testing it along with the filter on the performance patchsets.
Comment 4 Katrin Fischer 2019-10-12 08:28:43 UTC
(In reply to Tomás Cohen Arazi from comment #3)
> (In reply to Tomás Cohen Arazi from comment #2)
> > This is a POC. It uses the default FW when it shouldn't, as each FW might
> > have a different mapping for items.itemnumber.
> 
> Well, not POC. It is completely funcional and the default framework is a
> reasonable fallback. I will try to submit the unit tests, but please
> consider testing it along with the filter on the performance patchsets.

I think not only a fallback, if I understand correctly, the default framework defines the mappings now:
Bug 19096 - Koha to MARC mappings (Part 2): Make Default authoritative
Comment 5 Tomás Cohen Arazi 2019-10-14 11:34:57 UTC
Created attachment 94094 [details] [review]
Bug 23807: Unit tests

This patch introduces tests for the Koha::Item->as_marc_field method. It
tries to cover all the use cases.

To test:
1. Apply this patchset
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Item.t
=> SUCCESS; Tests pass!
3. Sign off :-D
Comment 6 Jan Kolátor 2019-10-16 11:50:29 UTC
Created attachment 94274 [details] [review]
Bug 23807: Add Koha::Item->as_marc_field

Signed-off-by: Jan Kolator <kolatorj@email.cz>
Comment 7 Jan Kolátor 2019-10-16 11:50:33 UTC
Created attachment 94275 [details] [review]
Bug 23807: Unit tests

This patch introduces tests for the Koha::Item->as_marc_field method. It
tries to cover all the use cases.

To test:
1. Apply this patchset
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Item.t
=> SUCCESS; Tests pass!
3. Sign off :-D

Signed-off-by: Jan Kolator <kolatorj@email.cz>
Comment 8 Josef Moravec 2019-10-16 12:06:47 UTC
Created attachment 94279 [details] [review]
Bug 23807: Add Koha::Item->as_marc_field

Signed-off-by: Jan Kolator <kolatorj@email.cz>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 9 Josef Moravec 2019-10-16 12:06:52 UTC
Created attachment 94280 [details] [review]
Bug 23807: Unit tests

This patch introduces tests for the Koha::Item->as_marc_field method. It
tries to cover all the use cases.

To test:
1. Apply this patchset
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Item.t
=> SUCCESS; Tests pass!
3. Sign off :-D

Signed-off-by: Jan Kolator <kolatorj@email.cz>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Martin Renvoize 2019-10-17 14:07:55 UTC
Nice work!

Pushed to master for 19.11.00
Comment 11 Jonathan Druart 2020-11-13 08:30:27 UTC
Any good reasons to have called GetMarcSubfieldStructure with the unsafe flag set?

% grep unsafe t/db_dependent/Koha/Item.t
    my $mss = C4::Biblio::GetMarcSubfieldStructure( '', { unsafe => 1 } );
    $mss = C4::Biblio::GetMarcSubfieldStructure( '', { unsafe => 0 } );

Unsafe from tests don't feel safe :)

I am seeing some tests failing (when run twice) because of it.
Comment 12 Jonathan Druart 2020-11-13 08:38:01 UTC
I've opened bug 27007.