Bug 10855 as a whole does not apply at the moment (may need a little rebase ?), so I tried to test this patch with Koha/AdditionalField.pm alone extracted from 10855. Found some problems:
1) if order line is deleted, additional fields for this order will stay in the database
2) in case order gets transferred to another basket, additional fields will remain associated with initial order record (now "cancelled" after the transfer); shouldn't they rather got re-associated with the newly created order record (the clone of the original) instead?
3) non-latin character encoding: similar issues as in 10855 (see also Bug 11302 - Template::output should deal with object).

Regarding 3), I have working solution for bug 10855 (essentially, it's a more conservative take than Jonathan patch in 11302 - should be less regression-prone; still needs some testing), but it doesn't quite work for this particular patch. I guess it's because new sub GetValues() doesn't encode it's output, which introduces yet another (different kind of) encoding problem in cases where there are non-latin characters present e.g. in authorized values descriptions (?).

I've tested this patch and it is working. You can add easily new fields linked to a order line. This function could be very useful to add specific fields to manage your collections.

It is true that if a order line is being deleted, additional fields for this order will be stored further in the database, precisely in the table "additional_field_values" but it's a non-blocking feature. Of course, it would be better to have these additional fields deleted too when the order line is being deleted.

The encoding problem should be solved. I have created additional fields in french with accented letters and the display is not ok.

If the two last issues are fixed, I think that this patch will improve the acquisitions module.

Changing status to Failed QA, as it requires some work

(In reply to Jacek Ablewicz from comment #3)
> 1) if order line is deleted, additional fields for this order will
> stay in the database

Order lines are never removed from database, so additional fields data should remain too.

> 3) non-latin character encoding: similar issues as in 10855 (see also
> Bug 11302 - Template::output should deal with object).

I think this has been fixed by bug 11944 or bug 10855 because I can't reproduce the problem.


Remaining issue:
> 2) in case order gets transferred to another basket, additional fields
> will remain associated with initial order record (now "cancelled"
> after the transfer); shouldn't they rather got re-associated with the
> newly created order record (the clone of the original) instead?

Created attachment 45758 [details]
picture of warning for AV and Marcfield

When I try to set up parameters for test case :

d/ a AV category, a MARC field not linked to AV category

then I have a blocking warning (see picture attached).
Am I doing wrong ?

Patch tested with a sandbox, by Harold Dramer <harold.dramer@nyls.edu>

Since I am the author of bug 10855, it would be better to see someone else QA this new enhancement.

Jesse, any chances to see you QA this one?

Just some first notes on this:

1) New "add order line fields" page

a) Test plan calls for adding fields with AV category and MARC field, but this always results in this error message for me:
You cannot select an authorised value category and a marcfield

b) why have the datatable search field below the table instead of above? I know we have other places where it's like this, but it always irritates me - the search is most useful for a big table and this will require you to scroll all the way to the end of the table first.

c) this looks like a tiny translation issue:
'aLengthMenu': [[10, 20, 50, 100, -1], [10, 20, 50, 100, "All"]],

d) it would be nice, if the table could use the new buttons we for Edit and Delete as introduced for a lot of other tables recently to match the new design pattern.

e) Icon icon-plus and others should be FA icons.

Otherwise this seems to work ok. :)

I am wondering what the next step is - this patch will allow to see and manipulate the fields on adding an order, but later, once the basket is closed you won't be able to see or search for them. So we should try and get bug 11870 sorted and maybe think about a way to show the fields on receive and on the basket summary page (in separate bugs).

(In reply to Katrin Fischer from comment #19)
> Just some first notes on this:
> 
> 1) New "add order line fields" page
> 
> a) Test plan calls for adding fields with AV category and MARC field, but
> this always results in this error message for me:
> You cannot select an authorised value category and a marcfield

Yes, the test plan is wrong: you cannot define both.

> b) why have the datatable search field below the table instead of above? I
> know we have other places where it's like this, but it always irritates me -
> the search is most useful for a big table and this will require you to
> scroll all the way to the end of the table first.

It's the same behaviour as for serials, so I'd say it's expected.

> d) it would be nice, if the table could use the new buttons we for Edit and
> Delete as introduced for a lot of other tables recently to match the new
> design pattern.

Could be done by bug 15774 which create a new page to configure all of these additional fields.

>> b) why have the datatable search field below the table instead of above? I
>> know we have other places where it's like this, but it always irritates me -
>> the search is most useful for a big table and this will require you to
>> scroll all the way to the end of the table first.

>It's the same behaviour as for serials, so I'd say it's expected.

Still doesn't make sense :) But not a blocker. 
I think Jesse wanted to take a look at this - so I didn't test further.

Created attachment 53614 [details] [review]
Bug 11844: Use additional fields for order lines

This patch makes use of new module Koha::AdditionalField to provide
additional fields to order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, then
value from biblio record is retrieved at order line creation. When
saving order line (at creation or modification), values in additional
fields are saved into biblio record.
If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for a AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category
   b/ no AV category, a MARC field linked to AV category
   c/ a AV category, no MARC field
   d/ a AV category, a MARC field not linked to AV category
   e/ a AV category, a MARC field linked to another AV category
   In case of (e), the additional AV category should be used, not the AV
   category linked to MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e) (check this is the right AV category for (e))
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (a), (b),
    (d) and (e)
11/ modify the same order line, check that values you've filled is
    correctly retrieved.
12/ modify all values, save, and check biblio once again
13/ create a new order line on the same biblio used for previous order
    line. check that values are correctly retrieved from biblio

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>

Created attachment 53615 [details] [review]
Bug 11844: Copy additional fields values when transfering order

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>

Created attachment 53616 [details] [review]
Bug 11844: Fix additional field values when creating a new order line

ordernumber was not correctly retrieved

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>

Created attachment 53619 [details] [review]
Bug 11844: (followup) use new unified additional field screen

Global symbol "$additional_fields" requires explicit package name (did you forget to declare "my $additional_fields"?) at acqui/neworderempty.pl line 313.

Created attachment 55529 [details] [review]
Bug 11844: Fix syntax errors

Blocked by bug 15774.

Created attachment 75332 [details] [review]
Bug 11844: Use additional fields for order lines

This patch allows to create additional fields for order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, there are two possible
scenario:
- MARC field mode = get: The field cannot be modified and its value is
  retrieved from the bibliographic record (current behaviour)
- MARC field mode = set: The field can be modified and its value is
  saved to the bibliographic record (new behaviour)

If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for an AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set
   e/ an AV category, no MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e). Fields (a) and (c) should be disabled.
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (b) and
    (d), but not for (a) and (c)
11/ modify the same order line, check that values you've filled are
    correctly retrieved and that values for (a) and (c) were correctly
    retrieved from the bibliographic record
12/ modify all values, save, and check biblio once again

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>

Rebased and squashed patch on top of recent changes of bug 15774.

It introduces a new parameter for additional fields (MARC field mode) which allows additional field value modifications to be reported in the biblio. The current behaviour of retrieving the value from the biblio is still the default.

Created attachment 75362 [details] [review]
Bug 11844: Update kohastructure.sql

Hi Julian, we would be interested in this feature and judging from the CC list, others as well. :) Are there any plans to pick this up again?

*** Bug 28219 has been marked as a duplicate of this bug. ***

Created attachment 141116 [details] [review]
Bug 11844: Use additional fields for order lines

This patch allows to create additional fields for order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, there are two possible
scenario:
- MARC field mode = get: The field cannot be modified and its value is
  retrieved from the bibliographic record (current behaviour)
- MARC field mode = set: The field can be modified and its value is
  saved to the bibliographic record (new behaviour)

If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for an AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set
   e/ an AV category, no MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e). Fields (a) and (c) should be disabled.
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (b) and
    (d), but not for (a) and (c)
11/ modify the same order line, check that values you've filled are
    correctly retrieved and that values for (a) and (c) were correctly
    retrieved from the bibliographic record
12/ modify all values, save, and check biblio once again

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>

Patch rebased on master

(In reply to Julian Maurice from comment #36)
> Patch rebased on master

Hey Julian,

I'm very sorry, I tried to test it in the ByWater Sandbox and always get an error message "The field could not be inserted. Perhaps the name already exists?" when trying to create an new field.

I had the same issue. After applying the patch, you need to run updatedatabase.pl, update_dbix_class_files.pl, and then restart Koha.

(In reply to Julian Maurice from comment #38)
> I had the same issue. After applying the patch, you need to run
> updatedatabase.pl, update_dbix_class_files.pl, and then restart Koha.

Thanks!

Everthing was ok except case c (no AV category, MARC field 942$c linked to AV category itemtypes, MARC field mode = get)

The value was empty in the additional field in the order line although 942$c was not empty (value=Books)

This is not supposed to work with items field (what should happen when you have more than one item ?). I should have said that in the test plan, sorry.

I'm surprised it works when MARC field mode is set. Which item is updated ? The first one ?

(In reply to Julian Maurice from comment #40)
> This is not supposed to work with items field (what should happen when you
> have more than one item ?). I should have said that in the test plan, sorry.
> 
> I'm surprised it works when MARC field mode is set. Which item is updated ?
> The first one ?

Hi Julian, I think you misread: 942 not 952, so the record level non-repeatable field was meant here.

Oh... ok then I'll check what's wrong :)
Thanks

(In reply to Julian Maurice from comment #42)
> Oh... ok then I'll check what's wrong :)
> Thanks

Thanks :) Would be great to get this into 22.11! 

I was wondering, could the problem stem from the itemtypes being not a 'real' AV? You set it up like one, but it's actually drawn from the itemtypes table.

Created attachment 145517 [details] [review]
Bug 11844: Use additional fields for order lines

This patch allows to create additional fields for order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, there are two possible
scenario:
- MARC field mode = get: The field cannot be modified and its value is
  retrieved from the bibliographic record (current behaviour)
- MARC field mode = set: The field can be modified and its value is
  saved to the bibliographic record (new behaviour)

If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for an AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set
   e/ an AV category, no MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e). Fields (a) and (c) should be disabled.
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (b) and
    (d), but not for (a) and (c)
11/ modify the same order line, check that values you've filled are
    correctly retrieved and that values for (a) and (c) were correctly
    retrieved from the bibliographic record
12/ modify all values, save, and check biblio once again

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>

I rebased, there are also some not too hard looking QA script fails:

 FAIL	Koha/AdditionalField.pm
   FAIL	  pod coverage
		POD is missing for 'effective_authorised_value_category'

 OK	Koha/Object/Mixin/AdditionalFields.pm
 OK	Koha/Schema/Result/Aqorder.pm
 OK	acqui/addorder.pl
 OK	acqui/neworderempty.pl
 OK	admin/additional-fields.pl
 FAIL	installer/data/mysql/atomicupdate/bug-11844.pl
   FAIL	  file permissions
		File must have the exec flag

 OK	installer/data/mysql/kohastructure.sql
 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/additional-fields-entry.inc
   FAIL	  filters
		missing_filter at line 35 (                    This value will be saved to the [% field.marcfield %] subfield of the selected biblio.)

 OK	koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/additional-fields.tt
   FAIL	  forbidden patterns
		forbidden pattern: trailing space char (line 127)

Created attachment 145535 [details] [review]
Bug 11844: Use additional fields for order lines

This patch allows to create additional fields for order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, there are two possible
scenario:
- MARC field mode = get: The field cannot be modified and its value is
  retrieved from the bibliographic record (current behaviour)
- MARC field mode = set: The field can be modified and its value is
  saved to the bibliographic record (new behaviour)

If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for an AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set
   e/ an AV category, no MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e). Fields (a) and (c) should be disabled.
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (b) and
    (d), but not for (a) and (c)
11/ modify the same order line, check that values you've filled are
    correctly retrieved and that values for (a) and (c) were correctly
    retrieved from the bibliographic record
12/ modify all values, save, and check biblio once again

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>

Created attachment 145536 [details] [review]
Bug 11844: Take into account itemtypes, branches, and cn_source

MARC subfields can be linked to "authorised values" that are not real
authorised values: itemtypes, branches, and cn_source.
Those were not taken into account. This patch fixes that

Test plan:
1. Create additional fields for order lines with a MARC subfield that is
   linked to one of those "fake" authorised values list
2. Follow the same steps as in the main test plan

Created attachment 145540 [details] [review]
Bug 11844: Fix QA issues (POD, TT filters, file exec flag)

(In reply to Katrin Fischer from comment #43)
> I was wondering, could the problem stem from the itemtypes being not a
> 'real' AV? You set it up like one, but it's actually drawn from the
> itemtypes table.

Yes, that was it.
It should be ok now.

(In reply to Julian Maurice from comment #49)
> (In reply to Katrin Fischer from comment #43)
> > I was wondering, could the problem stem from the itemtypes being not a
> > 'real' AV? You set it up like one, but it's actually drawn from the
> > itemtypes table.
> 
> Yes, that was it.
> It should be ok now.


When trying to add a new Additional Field an error message appears : "The field could not be inserted. Perhaps the name already exists?"

(In reply to Michaela Sieber from comment #50) 
> When trying to add a new Additional Field an error message appears : "The
> field could not be inserted. Perhaps the name already exists?"

See comment 38

(In reply to Julian Maurice from comment #38)
> I had the same issue. After applying the patch, you need to run
> updatedatabase.pl, update_dbix_class_files.pl, and then restart Koha.

I tried that on the sandboxes running "Refresh schema" and "Restart services", but the problem remained. See: http://sandboxes.ptfs-europe.co.uk/

I don't know what's wrong on sandboxes, but I just retried it locally: I reproduced the error, then updatedbix + restart starman worked.
marcfield_mode column exist on the sandbox so updatedabase was correctly run.

(In reply to Julian Maurice from comment #53)
> I don't know what's wrong on sandboxes, but I just retried it locally: I
> reproduced the error, then updatedbix + restart starman worked.
> marcfield_mode column exist on the sandbox so updatedabase was correctly run.

Yes, that's odd, the output of the "refresh schema" looked like it finished ok. Could you maybe attach the 'DBIC don't push' patch here?

I did some tests in biblibre and ptfs sandboxes

Test results:

Case 1: no AV category and no MARC field  --> works perfect
Case 2: AV category and no MARC field  --> works perfect


Unfortunately Additional Fields with MARC fields did not work:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set


In get mode nothing happens, the values are not displayed
In set mode an error message appears:
"Tag "942h" is not a valid tag. at /kohadevbox/koha/Koha/Object/Mixin/AdditionalFields.pm line 72.
 at /usr/lib/x86_64-linux-gnu/perl-base/Carp.pm line 289" .....



And I think the entry in the left menu is missing, see testplan
2/ In the left menu, click on "Add order line fields"

Trying to see if I can help out here...

Created attachment 146497 [details] [review]
Bug 11844: Use additional fields for order lines

This patch allows to create additional fields for order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, there are two possible
scenario:
- MARC field mode = get: The field cannot be modified and its value is
  retrieved from the bibliographic record (current behaviour)
- MARC field mode = set: The field can be modified and its value is
  saved to the bibliographic record (new behaviour)

If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for an AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set
   e/ an AV category, no MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e). Fields (a) and (c) should be disabled.
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (b) and
    (d), but not for (a) and (c)
11/ modify the same order line, check that values you've filled are
    correctly retrieved and that values for (a) and (c) were correctly
    retrieved from the bibliographic record
12/ modify all values, save, and check biblio once again

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 146500 [details] [review]
Bug 11844: Take into account itemtypes, branches, and cn_source

MARC subfields can be linked to "authorised values" that are not real
authorised values: itemtypes, branches, and cn_source.
Those were not taken into account. This patch fixes that

Test plan:
1. Create additional fields for order lines with a MARC subfield that is
   linked to one of those "fake" authorised values list
2. Follow the same steps as in the main test plan

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 146501 [details] [review]
Bug 11844: Fix QA issues (POD, TT filters, file exec flag)

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 146502 [details] [review]
Bug 11844: [DO NOT PUSH] Schema changes

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 146503 [details] [review]
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record

This is about the note displaying after an additional field
that is linked to a MARC field.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 146504 [details] [review]
Bug 11844: (follow-up) Add standard note to database update

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 146505 [details] [review]
Bug 11844: (follow-up) Add new option to acq navigation

The navigation on the left in the acq module has links to
the additional fields for baskets and invoices. This also
adds a link for the new order lines feature.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Created attachment 146506 [details] [review]
Bug 11844: (follow-up) Add format hint to the additional fields admin page

The MARC field needs to be added with a $ between field and subfield
for things to work right. So this adds a hint about the expected format.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

I've run some more tests here:

* Applied patches
* Ran dbic
* Ran restart_all

1) Administration > Additional fields

* Can add edit and delete additional fields for order lines.
* Additional fields for baskets don't have the MARC options, which is correct.
* Additional fields for subscriptions offer the MARC option and have the save/display selection now. In difference to orders existing MARC information is not displayed when a new subscription is added, only when you edit. This was already the case before and I think we should improve it separately. Saving values works as expected.

> And I think the entry in the left menu is missing, see testplan
> 2/ In the left menu, click on "Add order line fields"

Fixed this in a follow-up.

2) Functionality in acquisitions

I have added 4 different additional fields:

* Text field, no auth value or MARC
* MARC field with get: 245$a
* MARC field with set and link to a pseudo AV in the frameworks: 942$c itemtypes
* Pull down, AV controlled

All fields have behaved as expected when adding or modifying an order.

I think maybe Michaela was missing the $ in configuration from the error message she saw:
> "Tag "942h" is not a valid tag.

I am proposing adding a hint to the field with the proper format in a follow-up. I've also filed bug 32944.

3) Follow-ups

I have done several really small follow-ups. Please feel free to drop or squash in QA as needed.

This generally looks great.. but there's a few last unit tests missing.

Could we have some tests for the new Koha::AdditionalField::effective_authorised_value_category method please.

I'm afraid I'm also not seeing any additions for the C4::Acquisition::TransferOrder changes either.

Finally, the changes in Koha/Object/Mixin/AdditionalFields.pm don't appear to be tested either.

Sorry Julian, this is great work and I wouldn't want it accidentally undone in a future change. Failing QA for now.

I get your point, Martin. Sadly, I think writing those tests is a bit too much for me. :( Hope Julian will be able to spend some more time on this, it would be sad, as you said, to see it stuck again. We are so close!

Created attachment 148889 [details] [review]
Bug 11844: Add tests for Koha::AdditionalField

Created attachment 148895 [details] [review]
Bug 11844: Add tests for TransferOrder

Created attachment 148897 [details] [review]
Bug 11844: Add tests for marcfield_mode

I think I covered everything... Back to signed off!

Created attachment 149081 [details] [review]
Bug 11844: Use additional fields for order lines

This patch allows to create additional fields for order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, there are two possible
scenario:
- MARC field mode = get: The field cannot be modified and its value is
  retrieved from the bibliographic record (current behaviour)
- MARC field mode = set: The field can be modified and its value is
  saved to the bibliographic record (new behaviour)

If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for an AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set
   e/ an AV category, no MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e). Fields (a) and (c) should be disabled.
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (b) and
    (d), but not for (a) and (c)
11/ modify the same order line, check that values you've filled are
    correctly retrieved and that values for (a) and (c) were correctly
    retrieved from the bibliographic record
12/ modify all values, save, and check biblio once again

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149082 [details] [review]
Bug 11844: Take into account itemtypes, branches, and cn_source

MARC subfields can be linked to "authorised values" that are not real
authorised values: itemtypes, branches, and cn_source.
Those were not taken into account. This patch fixes that

Test plan:
1. Create additional fields for order lines with a MARC subfield that is
   linked to one of those "fake" authorised values list
2. Follow the same steps as in the main test plan

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149083 [details] [review]
Bug 11844: Fix QA issues (POD, TT filters, file exec flag)

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149084 [details] [review]
Bug 11844: [DO NOT PUSH] Schema changes

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149085 [details] [review]
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record

This is about the note displaying after an additional field
that is linked to a MARC field.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149086 [details] [review]
Bug 11844: (follow-up) Add standard note to database update

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149087 [details] [review]
Bug 11844: (follow-up) Add new option to acq navigation

The navigation on the left in the acq module has links to
the additional fields for baskets and invoices. This also
adds a link for the new order lines feature.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149088 [details] [review]
Bug 11844: (follow-up) Add format hint to the additional fields admin page

The MARC field needs to be added with a $ between field and subfield
for things to work right. So this adds a hint about the expected format.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149089 [details] [review]
Bug 11844: Add tests for Koha::AdditionalField

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149090 [details] [review]
Bug 11844: Add tests for TransferOrder

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149091 [details] [review]
Bug 11844: Add tests for marcfield_mode

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Created attachment 149682 [details] [review]
Bug 11844: Use additional fields for order lines

This patch allows to create additional fields for order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, there are two possible
scenario:
- MARC field mode = get: The field cannot be modified and its value is
  retrieved from the bibliographic record (current behaviour)
- MARC field mode = set: The field can be modified and its value is
  saved to the bibliographic record (new behaviour)

If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for an AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set
   e/ an AV category, no MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e). Fields (a) and (c) should be disabled.
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (b) and
    (d), but not for (a) and (c)
11/ modify the same order line, check that values you've filled are
    correctly retrieved and that values for (a) and (c) were correctly
    retrieved from the bibliographic record
12/ modify all values, save, and check biblio once again

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149683 [details] [review]
Bug 11844: Take into account itemtypes, branches, and cn_source

MARC subfields can be linked to "authorised values" that are not real
authorised values: itemtypes, branches, and cn_source.
Those were not taken into account. This patch fixes that

Test plan:
1. Create additional fields for order lines with a MARC subfield that is
   linked to one of those "fake" authorised values list
2. Follow the same steps as in the main test plan

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149684 [details] [review]
Bug 11844: Fix QA issues (POD, TT filters, file exec flag)

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149685 [details] [review]
Bug 11844: [DO NOT PUSH] Schema changes

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149686 [details] [review]
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record

This is about the note displaying after an additional field
that is linked to a MARC field.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149687 [details] [review]
Bug 11844: (follow-up) Add standard note to database update

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149688 [details] [review]
Bug 11844: (follow-up) Add new option to acq navigation

The navigation on the left in the acq module has links to
the additional fields for baskets and invoices. This also
adds a link for the new order lines feature.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149689 [details] [review]
Bug 11844: (follow-up) Add format hint to the additional fields admin page

The MARC field needs to be added with a $ between field and subfield
for things to work right. So this adds a hint about the expected format.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149690 [details] [review]
Bug 11844: Add tests for Koha::AdditionalField

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149691 [details] [review]
Bug 11844: Add tests for TransferOrder

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 149692 [details] [review]
Bug 11844: Add tests for marcfield_mode

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Thanks for adding all the tests.. this works great and is now covered nicely.

Passing QA

RM note.. the QA tool threw two issues.. POD coverage which is a false positive here I believe and a permissions issue.. but the permissions all look correct to me.. not sure what's going on there.

(In reply to Martin Renvoize from comment #94)
> RM note.. the QA tool threw two issues.. POD coverage which is a false
> positive here I believe and a permissions issue.. but the permissions all
> look correct to me.. not sure what's going on there.

 FAIL   t/db_dependent/Koha/AdditionalField.pm
   FAIL   file permissions
                File must not have the exec flag

Well, this is valid:
-rwxr-xr-x 1 jonathan jonathan 2506 mai    2 11:51 t/db_dependent/Koha/AdditionalField.pm

Actually I guess it should be renamed to .t

Created attachment 150537 [details] [review]
Bug 11844: Use additional fields for order lines

This patch allows to create additional fields for order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, there are two possible
scenario:
- MARC field mode = get: The field cannot be modified and its value is
  retrieved from the bibliographic record (current behaviour)
- MARC field mode = set: The field can be modified and its value is
  saved to the bibliographic record (new behaviour)

If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for an AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set
   e/ an AV category, no MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e). Fields (a) and (c) should be disabled.
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (b) and
    (d), but not for (a) and (c)
11/ modify the same order line, check that values you've filled are
    correctly retrieved and that values for (a) and (c) were correctly
    retrieved from the bibliographic record
12/ modify all values, save, and check biblio once again

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150538 [details] [review]
Bug 11844: Take into account itemtypes, branches, and cn_source

MARC subfields can be linked to "authorised values" that are not real
authorised values: itemtypes, branches, and cn_source.
Those were not taken into account. This patch fixes that

Test plan:
1. Create additional fields for order lines with a MARC subfield that is
   linked to one of those "fake" authorised values list
2. Follow the same steps as in the main test plan

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150539 [details] [review]
Bug 11844: Fix QA issues (POD, TT filters, file exec flag)

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150540 [details] [review]
Bug 11844: [DO NOT PUSH] Schema changes

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150541 [details] [review]
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record

This is about the note displaying after an additional field
that is linked to a MARC field.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150542 [details] [review]
Bug 11844: (follow-up) Add standard note to database update

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150543 [details] [review]
Bug 11844: (follow-up) Add new option to acq navigation

The navigation on the left in the acq module has links to
the additional fields for baskets and invoices. This also
adds a link for the new order lines feature.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150544 [details] [review]
Bug 11844: (follow-up) Add format hint to the additional fields admin page

The MARC field needs to be added with a $ between field and subfield
for things to work right. So this adds a hint about the expected format.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150545 [details] [review]
Bug 11844: Add tests for Koha::AdditionalField

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150546 [details] [review]
Bug 11844: Add tests for TransferOrder

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150547 [details] [review]
Bug 11844: Add tests for marcfield_mode

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 150548 [details] [review]
Bug 11844: (QA follow-up) Fix unit test file ending

t/db_dependent/Koha/AdditionalField.pm -> t/db_dependent/Koha/AdditionalField.t

(In reply to Jonathan Druart from comment #95)
> (In reply to Martin Renvoize from comment #94)
> > RM note.. the QA tool threw two issues.. POD coverage which is a false
> > positive here I believe and a permissions issue.. but the permissions all
> > look correct to me.. not sure what's going on there.
> 
>  FAIL   t/db_dependent/Koha/AdditionalField.pm
>    FAIL   file permissions
>                 File must not have the exec flag
> 
> Well, this is valid:
> -rwxr-xr-x 1 jonathan jonathan 2506 mai    2 11:51
> t/db_dependent/Koha/AdditionalField.pm
> 
> Actually I guess it should be renamed to .t

Small rebase (conflicts in the USE statements, not hard), fixed the test, unit and QA tests pass now. Also did some tests in the GUI.

Hi Julian et al, there's a failing test. Please fix against this branch:

https://gitlab.com/thekesolutions/Koha/-/commits/qa_11844

#   Failed test 'No tests run for subtest "set_additional_fields with marcfield_mode = "get""'
#   at /kohadevbox/koha/t/db_dependent/Koha/Object/Mixin/AdditionalFields.t line 54.
Can't call method "value" on an undefined value at /kohadevbox/koha/t/db_dependent/Koha/Object/Mixin/AdditionalFields.t line 51.
 # Looks like your test exited with 255 just after 1.
 /kohadevbox/koha/t/db_dependent/Koha/Object/Mixin/AdditionalFields.t ..
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 2/2 subtests

Test Summary Report
-------------------
 /kohadevbox/koha/t/db_dependent/Koha/Object/Mixin/AdditionalFields.t (Wstat: 65280 Tests: 1 Failed: 1)
  Failed test:   1
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 2 tests but ran 1.
Files=1, Tests=1,  3 wallclock secs ( 0.03 usr  0.01 sys +  1.66 cusr  0.38 csys =  2.08 CPU)
Result: FAIL

I can fix the test. I think what happens is:

If the field is a repeatable one, we are only looking at the first occurence. The test was using 999, but this field is already present in the created biblio, before another 999 is added with $Z.

So when trying to set the field, no $Z was found and the additional_field was not set.

This can be fixed by: 

1) Using another field than 999 that doesn't already exist in the record.
2) appending Z to the already existing 999
3) Making sure that set_additional_fields loops through repeatable subfields, using the first subfield found.

1) is done easily, 3) is harder. I tried to determine how it worked before this patch set, as the 'set' functionality was already there:

https://git.koha-community.org/Koha-community/Koha/src/commit/f891156f424a466aadb5ab85c445fcdbae4f3536/serials/subscription-add.pl#L370

        my $value = $query->param('additional_field_' . $field->id);
        if ($field->marcfield) {
            my ($field, $subfield) = split /\$/, $field->marcfield;
            if ( $record and $field and $subfield ) {
                $value = $record->subfield( $field, $subfield );
            }
        }

It appears it never dealt 'cleverly' with repeated subfields. So we might move this to a separate bug and go with 1) for now.

Created attachment 151221 [details] [review]
Bug 11844: Use additional fields for order lines

This patch allows to create additional fields for order lines.
Once created, these fields can be filled during order line creation or
modification.

If additional field is linked to a MARC field, there are two possible
scenario:
- MARC field mode = get: The field cannot be modified and its value is
  retrieved from the bibliographic record (current behaviour)
- MARC field mode = set: The field can be modified and its value is
  saved to the bibliographic record (new behaviour)

If additional field is linked to an authorised value category, then
authorised values are used. If not directly linked to an authorised
value category, but linked to a MARC field, a search for an AV category
is made on MARC default framework.

This patch doesn't display additional fields value anywhere (except in
order line creation/modification). Future patches will do that.

Test plan:
1/ Go to Acquisitions home
2/ In the left menu, click on "Add order line fields"
3/ Click on "New field" button
4/ Give the field a name (unique), no AV category and no MARC field.
5/ Save.
6/ Create 5 other fields:
   a/ no AV category, a MARC field not linked to AV category, MARC field
      mode = get
   b/ no AV category, a MARC field not linked to AV category, MARC field
      mode = set
   c/ no AV category, a MARC field linked to AV category, MARC field
      mode = get
   d/ no AV category, a MARC field linked to AV category, MARC field
      mode = set
   e/ an AV category, no MARC field
7/ Create everything you need to be able to create order lines
   (supplier, basket, ...)
8/ Create an order line. At bottom of the page, you should see your
   additional fields, with authorised values dropdrown list for fields
   (c), (d) and (e). Fields (a) and (c) should be disabled.
9/ Fill these fields with some data and save order line
10/ check that data was correctly saved into biblio for fields (b) and
    (d), but not for (a) and (c)
11/ modify the same order line, check that values you've filled are
    correctly retrieved and that values for (a) and (c) were correctly
    retrieved from the bibliographic record
12/ modify all values, save, and check biblio once again

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 151222 [details] [review]
Bug 11844: Take into account itemtypes, branches, and cn_source

MARC subfields can be linked to "authorised values" that are not real
authorised values: itemtypes, branches, and cn_source.
Those were not taken into account. This patch fixes that

Test plan:
1. Create additional fields for order lines with a MARC subfield that is
   linked to one of those "fake" authorised values list
2. Follow the same steps as in the main test plan

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 151223 [details] [review]
Bug 11844: Fix QA issues (POD, TT filters, file exec flag)

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 151224 [details] [review]
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record

This is about the note displaying after an additional field
that is linked to a MARC field.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 151225 [details] [review]
Bug 11844: (follow-up) Add standard note to database update

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 151226 [details] [review]
Bug 11844: (follow-up) Add new option to acq navigation

The navigation on the left in the acq module has links to
the additional fields for baskets and invoices. This also
adds a link for the new order lines feature.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 151227 [details] [review]
Bug 11844: (follow-up) Add format hint to the additional fields admin page

The MARC field needs to be added with a $ between field and subfield
for things to work right. So this adds a hint about the expected format.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 151228 [details] [review]
Bug 11844: Add tests

Tests added for:

- Koha::AdditionalField
- TransferOrder
- marcfield_mode

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 151229 [details] [review]
Bug 11844: DBIC schema

Created attachment 151230 [details] [review]
Bug 11844: (follow-up) Fix tests

Moves the test from 999 which already exists in record to 998
as the feature only matches on the first field.

Patches pushed for 23.05

Thanks for the research and follow-up, Katrina.

Created attachment 151366 [details] [review]
Bug 11844: Only display 'Additional fields' section if they are defined

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

New feature - not backporting to 22.11.x