Bug 24294 - Creating an order with ACQ framework using 00x fields doesn't work with default value
Summary: Creating an order with ACQ framework using 00x fields doesn't work with defau...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-12-22 21:48 UTC by Katrin Fischer
Modified: 2020-04-15 23:11 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
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 (3.62 KB, patch)
2020-01-02 11:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24294: Add default value support for control fields in ACQ framework (3.68 KB, patch)
2020-01-02 22:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 24294: Add default value support for control fields in ACQ framework (3.78 KB, patch)
2020-02-25 13:43 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24294: (QA follow-up) Add rollback and 008 default test (1.38 KB, patch)
2020-02-25 13:43 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 Mapley 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?