Bug 24294

Summary: Creating an order with ACQ framework using 00x fields doesn't work with default value
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: AcquisitionsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: 1joynelson, hayleypelham, jonathan.druart, lucas, m.de.rooy, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21316
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24134
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.05, 19.05.10
Attachments: Bug 24294: Add default value support for control fields in ACQ framework
Bug 24294: Add default value support for control fields in ACQ framework
Bug 24294: Add default value support for control fields in ACQ framework
Bug 24294: (QA follow-up) Add rollback and 008 default test

Description Katrin Fischer 2019-12-22 21:48:43 UTC
When trying to add an order using the ACQ framework with a 008@ tag, Koha explodes:

Control fields (generally, just tags below 010) do not have subfields, use data() at /home/vagrant/kohaclone/C4/Acquisition.pm line 3272.
 at /usr/share/perl/5.24/Carp.pm line 167
Comment 1 Jonathan Druart 2019-12-23 15:00:21 UTC
I do not recreate. Do you have a default value in ACQ for 008?
Comment 2 Katrin Fischer 2019-12-31 09:58:12 UTC
Hi Jonathan, sorry, running out of time to retest right now - yes, I was using a default value as I ran into this testing the new placeholder (bug 24134)
Comment 3 Jonathan Druart 2020-01-02 11:16:37 UTC
Created attachment 96732 [details] [review]
Bug 24294: Add default value support for control fields in ACQ framework

When trying to add an order using the ACQ framework with a 008@ tag,
Koha explodes:

Control fields (generally, just tags below 010) do not have subfields,
use data() at /home/vagrant/kohaclone/C4/Acquisition.pm line 3272.

Test plan:
Set a default value for a control field in the ACQ framework
Turn on UseACQFrameworkForBiblioRecords
Create a new order from a new record
The default value should be displayed
Save
=> No crash
Comment 4 Katrin Fischer 2020-01-02 22:44:55 UTC
Created attachment 96777 [details] [review]
Bug 24294: Add default value support for control fields in ACQ framework

When trying to add an order using the ACQ framework with a 008@ tag,
Koha explodes:

Control fields (generally, just tags below 010) do not have subfields,
use data() at /home/vagrant/kohaclone/C4/Acquisition.pm line 3272.

Test plan:
Set a default value for a control field in the ACQ framework
Turn on UseACQFrameworkForBiblioRecords
Create a new order from a new record
The default value should be displayed
Save
=> No crash

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Marcel de Rooy 2020-01-10 10:04:25 UTC
            '008' => {
                x => { defaultvalue => 'xxx' },
            },
This works too, telling me that the test does not verify it..
Comment 6 Jonathan Druart 2020-01-10 10:16:38 UTC
(In reply to Marcel de Rooy from comment #5)
>             '008' => {
>                 x => { defaultvalue => 'xxx' },
>             },
> This works too, telling me that the test does not verify it..

I do not understand what you mean. The test fails without the change to FillWithDefaultValues.
Comment 7 Jonathan Druart 2020-01-10 10:18:48 UTC
Ok got it.

Here 008 has a default value, and the record has a 008. So the default value will not be used.
Comment 8 Jonathan Druart 2020-02-04 11:55:04 UTC
Switching back to NQA as the QA question has been answered.
Comment 9 Marcel de Rooy 2020-02-25 13:06:36 UTC
Will have a look
Comment 10 Marcel de Rooy 2020-02-25 13:43:22 UTC
Created attachment 99582 [details] [review]
Bug 24294: Add default value support for control fields in ACQ framework

When trying to add an order using the ACQ framework with a 008@ tag,
Koha explodes:

Control fields (generally, just tags below 010) do not have subfields,
use data() at /home/vagrant/kohaclone/C4/Acquisition.pm line 3272.

Test plan:
Set a default value for a control field in the ACQ framework
Turn on UseACQFrameworkForBiblioRecords
Create a new order from a new record
The default value should be displayed
Save
=> No crash

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2020-02-25 13:43:27 UTC
Created attachment 99583 [details] [review]
Bug 24294: (QA follow-up) Add rollback and 008 default test

Default for 008 was not tested yet.
Only wondering if we should insert default values only when we meet an
undefined value. Or should we also add if we meet an empty string? Is the
latter not more realistic in MARC?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Katrin Fischer 2020-02-25 14:51:17 UTC
Thx Marcel!
Comment 13 Martin Renvoize 2020-02-26 20:43:19 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 14 Joy Nelson 2020-03-31 22:27:54 UTC
backported to 19.11.x branch for 19.11.05
Comment 15 Lucas Gass 2020-04-08 15:39:38 UTC
backported to 19.05.x for 19.05.10
Comment 16 Hayley Pelham 2020-04-15 23:11:10 UTC
I am having trouble recreating, is somebody able to confirm that this patch works for 18.11.x?