Bug 25006 - Koha::Item->as_marc_field generates undef subfields
Summary: Koha::Item->as_marc_field generates undef subfields
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 23807
Blocks: 23247
  Show dependency treegraph
 
Reported: 2020-03-27 19:35 UTC by Tomás Cohen Arazi
Modified: 2020-05-11 19:26 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06


Attachments
Bug 25006: Regression tests (1.97 KB, patch)
2020-03-27 19:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25006: Make Koha::Item->as_marc_field skip undefined subfields (1.39 KB, patch)
2020-03-27 19:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25006: Regression tests (2.02 KB, patch)
2020-03-29 09:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 25006: Make Koha::Item->as_marc_field skip undefined subfields (1.44 KB, patch)
2020-03-29 09:03 UTC, David Nind
Details | Diff | Splinter Review
Bug 25006: Regression tests (2.09 KB, patch)
2020-03-30 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25006: Make Koha::Item->as_marc_field skip undefined subfields (1.51 KB, patch)
2020-03-30 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2020-03-27 19:35:45 UTC
Trying to use the EmbedItems RecordProcessor filter for an endpoint, I noticed it got really noisy, and found it is generating subfields even for undefined mapped attributes (on the framework).
Comment 1 Tomás Cohen Arazi 2020-03-27 19:53:01 UTC
Created attachment 102016 [details] [review]
Bug 25006: Regression tests

This patch introduces regression tests for as_marc_field. It makes sure
there are empty subfields and then uses List::MoreUtils' all to verify
all generated MARC subfields contain something (i.e. the undef ones are
skipped).
Comment 2 Tomás Cohen Arazi 2020-03-27 19:53:06 UTC
Created attachment 102017 [details] [review]
Bug 25006: Make Koha::Item->as_marc_field skip undefined subfields

This patch makes as_marc_field skip subfields that don't have a value or
contains an empty string (replicating C4::Items:1021 logic).

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Item.t
=> FAIL: Tests fail because the generated MARC::Field contains undef
subfields.
3. Apply this patch
4. Repeat 2.
=> SUCCESS: Tests pass! Undefined and empty subfields are skipped!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 David Nind 2020-03-29 09:02:59 UTC
Created attachment 102051 [details] [review]
Bug 25006: Regression tests

This patch introduces regression tests for as_marc_field. It makes sure
there are empty subfields and then uses List::MoreUtils' all to verify
all generated MARC subfields contain something (i.e. the undef ones are
skipped).

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2020-03-29 09:03:02 UTC
Created attachment 102052 [details] [review]
Bug 25006: Make Koha::Item->as_marc_field skip undefined subfields

This patch makes as_marc_field skip subfields that don't have a value or
contains an empty string (replicating C4::Items:1021 logic).

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Item.t
=> FAIL: Tests fail because the generated MARC::Field contains undef
subfields.
3. Apply this patch
4. Repeat 2.
=> SUCCESS: Tests pass! Undefined and empty subfields are skipped!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Jonathan Druart 2020-03-30 10:41:19 UTC
Created attachment 102077 [details] [review]
Bug 25006: Regression tests

This patch introduces regression tests for as_marc_field. It makes sure
there are empty subfields and then uses List::MoreUtils' all to verify
all generated MARC subfields contain something (i.e. the undef ones are
skipped).

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Jonathan Druart 2020-03-30 10:41:23 UTC
Created attachment 102078 [details] [review]
Bug 25006: Make Koha::Item->as_marc_field skip undefined subfields

This patch makes as_marc_field skip subfields that don't have a value or
contains an empty string (replicating C4::Items:1021 logic).

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Item.t
=> FAIL: Tests fail because the generated MARC::Field contains undef
subfields.
3. Apply this patch
4. Repeat 2.
=> SUCCESS: Tests pass! Undefined and empty subfields are skipped!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Martin Renvoize 2020-04-03 13:24:28 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 8 Joy Nelson 2020-04-28 20:38:00 UTC
backported to 19.11.x branch for 19.11.06
Comment 9 Lucas Gass 2020-05-11 19:26:42 UTC
missing dependencies for 19.05.x, no backport