Bug 11844 - Additional fields for order lines
Summary: Additional fields for order lines
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature (vote)
Assignee: Julian Maurice
QA Contact: Martin Renvoize
URL:
Keywords:
: 28219 (view as bug list)
Depends on: 10855 11944 13596 15774
Blocks: 11870 35451
  Show dependency treegraph
 
Reported: 2014-02-26 09:42 UTC by Julian Maurice
Modified: 2023-11-30 12:20 UTC (History)
18 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/692
Text to go in the release notes:
This adds the option to add additional user defined fields to the order lines in the acquisition module. The fields can be set up as free text fields or pull down lists driven by authorised values. They can also pull information from the MARC record or allow you to create and edit a field in the MARC record.
Version(s) released in:
23.05.00


Attachments
Bug 11844: Use additional fields for order lines (19.01 KB, patch)
2014-02-26 09:43 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (19.07 KB, patch)
2014-02-26 11:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Copy additional fields values when transfering order (1.25 KB, patch)
2015-08-24 13:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Fix additional field values when creating a new order line (908 bytes, patch)
2015-08-24 13:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (20.07 KB, patch)
2015-12-14 17:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11844: Copy additional fields values when transfering order (1.25 KB, patch)
2015-12-14 17:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11844: Fix additional field values when creating a new order line (914 bytes, patch)
2015-12-14 17:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
picture of warning for AV and Marcfield (138.80 KB, image/png)
2015-12-17 10:57 UTC, jmbroust
Details
Bug 11844: Use additional fields for order lines (20.13 KB, patch)
2015-12-22 17:44 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: Copy additional fields values when transfering order (1.31 KB, patch)
2015-12-22 17:44 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: Fix additional field values when creating a new order line (969 bytes, patch)
2015-12-22 17:45 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (19.62 KB, patch)
2016-07-21 20:49 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 11844: Copy additional fields values when transfering order (1.24 KB, patch)
2016-07-21 20:50 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 11844: Fix additional field values when creating a new order line (989 bytes, patch)
2016-07-21 20:50 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 11844: (followup) use new unified additional field screen (12.94 KB, patch)
2016-07-21 20:50 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 11844: (followup) use new unified additional field screen (15.20 KB, patch)
2016-07-21 21:55 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 11844: Fix syntax errors (1.03 KB, patch)
2016-09-13 13:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (27.69 KB, patch)
2018-05-15 13:47 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Update kohastructure.sql (1.10 KB, patch)
2018-05-16 11:55 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (24.33 KB, patch)
2022-09-30 10:02 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (24.87 KB, patch)
2023-01-20 14:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (24.38 KB, patch)
2023-01-20 16:18 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Take into account itemtypes, branches, and cn_source (9.62 KB, patch)
2023-01-20 16:18 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Fix QA issues (POD, TT filters, file exec flag) (2.89 KB, patch)
2023-01-20 16:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (24.44 KB, patch)
2023-02-10 16:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Take into account itemtypes, branches, and cn_source (9.67 KB, patch)
2023-02-10 16:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Fix QA issues (POD, TT filters, file exec flag) (2.95 KB, patch)
2023-02-10 16:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: [DO NOT PUSH] Schema changes (1.81 KB, patch)
2023-02-10 16:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record (1.60 KB, patch)
2023-02-10 16:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add standard note to database update (930 bytes, patch)
2023-02-10 16:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add new option to acq navigation (1.29 KB, patch)
2023-02-10 16:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add format hint to the additional fields admin page (1.32 KB, patch)
2023-02-10 16:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Add tests for Koha::AdditionalField (3.14 KB, patch)
2023-03-29 08:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Add tests for TransferOrder (2.97 KB, patch)
2023-03-29 09:45 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Add tests for marcfield_mode (3.13 KB, patch)
2023-03-29 10:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (24.50 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: Take into account itemtypes, branches, and cn_source (9.73 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: Fix QA issues (POD, TT filters, file exec flag) (3.01 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: [DO NOT PUSH] Schema changes (1.86 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record (1.66 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add standard note to database update (987 bytes, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add new option to acq navigation (1.35 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add format hint to the additional fields admin page (1.38 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: Add tests for Koha::AdditionalField (3.19 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: Add tests for TransferOrder (3.03 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: Add tests for marcfield_mode (3.18 KB, patch)
2023-04-03 09:53 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (24.56 KB, patch)
2023-04-14 15:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: Take into account itemtypes, branches, and cn_source (9.79 KB, patch)
2023-04-14 15:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: Fix QA issues (POD, TT filters, file exec flag) (3.07 KB, patch)
2023-04-14 15:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: [DO NOT PUSH] Schema changes (1.93 KB, patch)
2023-04-14 15:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record (1.72 KB, patch)
2023-04-14 15:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add standard note to database update (1.03 KB, patch)
2023-04-14 15:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add new option to acq navigation (1.41 KB, patch)
2023-04-14 15:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add format hint to the additional fields admin page (1.44 KB, patch)
2023-04-14 15:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: Add tests for Koha::AdditionalField (3.26 KB, patch)
2023-04-14 15:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: Add tests for TransferOrder (3.09 KB, patch)
2023-04-14 15:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: Add tests for marcfield_mode (3.25 KB, patch)
2023-04-14 15:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (24.59 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Take into account itemtypes, branches, and cn_source (9.79 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Fix QA issues (POD, TT filters, file exec flag) (3.07 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: [DO NOT PUSH] Schema changes (1.93 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record (1.72 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add standard note to database update (1.03 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add new option to acq navigation (1.41 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add format hint to the additional fields admin page (1.44 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Add tests for Koha::AdditionalField (3.26 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Add tests for TransferOrder (3.09 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Add tests for marcfield_mode (3.25 KB, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (QA follow-up) Fix unit test file ending (727 bytes, patch)
2023-05-02 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
k (1.48 KB, patch)
2023-05-15 20:56 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Use additional fields for order lines (24.59 KB, patch)
2023-05-15 21:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Take into account itemtypes, branches, and cn_source (9.79 KB, patch)
2023-05-15 21:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Fix QA issues (POD, TT filters, file exec flag) (3.07 KB, patch)
2023-05-15 21:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Fix terminology: biblio should be bibliographic record (1.72 KB, patch)
2023-05-15 21:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add standard note to database update (1.03 KB, patch)
2023-05-15 21:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add new option to acq navigation (1.41 KB, patch)
2023-05-15 21:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Add format hint to the additional fields admin page (1.44 KB, patch)
2023-05-15 21:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Add tests (8.87 KB, patch)
2023-05-15 21:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: DBIC schema (1.73 KB, patch)
2023-05-15 21:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: (follow-up) Fix tests (1.48 KB, patch)
2023-05-15 21:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11844: Only display 'Additional fields' section if they are defined (1.05 KB, patch)
2023-05-17 17:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2014-02-26 09:42:26 UTC
Make use of new module Koha::AdditionalField to provide additional fields to order lines.
Comment 1 Julian Maurice 2014-02-26 09:43:10 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2014-02-26 11:10:07 UTC Comment hidden (obsolete)
Comment 3 Jacek Ablewicz 2014-03-04 09:04:46 UTC
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 (?).
Comment 4 Yves Tomic 2014-03-13 09:12:42 UTC
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.
Comment 5 Julian Maurice 2015-02-12 12:39:26 UTC
Changing status to Failed QA, as it requires some work
Comment 6 Julian Maurice 2015-08-24 13:28:49 UTC
(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?
Comment 7 Julian Maurice 2015-08-24 13:32:48 UTC Comment hidden (obsolete)
Comment 8 Julian Maurice 2015-08-24 13:32:52 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2015-12-14 17:11:11 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2015-12-14 17:11:18 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2015-12-14 17:11:21 UTC Comment hidden (obsolete)
Comment 12 jmbroust 2015-12-17 10:57:25 UTC
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 ?
Comment 13 Biblibre Sandboxes 2015-12-22 17:44:15 UTC
Patch tested with a sandbox, by Harold Dramer <harold.dramer@nyls.edu>
Comment 14 Biblibre Sandboxes 2015-12-22 17:44:54 UTC Comment hidden (obsolete)
Comment 15 Biblibre Sandboxes 2015-12-22 17:44:57 UTC Comment hidden (obsolete)
Comment 16 Biblibre Sandboxes 2015-12-22 17:45:00 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2016-01-07 11:09:02 UTC
Since I am the author of bug 10855, it would be better to see someone else QA this new enhancement.
Comment 18 Jonathan Druart 2016-03-22 10:29:45 UTC
Jesse, any chances to see you QA this one?
Comment 19 Katrin Fischer 2016-03-28 21:50:39 UTC
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).
Comment 20 Jonathan Druart 2016-04-04 12:21:29 UTC
(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.
Comment 21 Katrin Fischer 2016-04-04 12:26:30 UTC
>> 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.
Comment 22 Jesse Weaver 2016-07-21 20:49:43 UTC
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>
Comment 23 Jesse Weaver 2016-07-21 20:50:05 UTC
Created attachment 53615 [details] [review]
Bug 11844: Copy additional fields values when transfering order

Signed-off-by: Harold Dramer <harold.dramer@nyls.edu>
Comment 24 Jesse Weaver 2016-07-21 20:50:11 UTC
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>
Comment 25 Jesse Weaver 2016-07-21 20:50:51 UTC Comment hidden (obsolete)
Comment 26 Jesse Weaver 2016-07-21 21:55:01 UTC
Created attachment 53619 [details] [review]
Bug 11844: (followup) use new unified additional field screen
Comment 27 Jonathan Druart 2016-07-22 09:18:31 UTC
Global symbol "$additional_fields" requires explicit package name (did you forget to declare "my $additional_fields"?) at acqui/neworderempty.pl line 313.
Comment 28 Julian Maurice 2016-09-13 13:33:42 UTC
Created attachment 55529 [details] [review]
Bug 11844: Fix syntax errors
Comment 29 Jonathan Druart 2016-09-14 14:09:25 UTC
Blocked by bug 15774.
Comment 30 Julian Maurice 2018-05-15 13:47:52 UTC
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>
Comment 31 Julian Maurice 2018-05-15 13:51:25 UTC
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.
Comment 32 Julian Maurice 2018-05-16 11:55:18 UTC
Created attachment 75362 [details] [review]
Bug 11844: Update kohastructure.sql
Comment 33 Katrin Fischer 2022-07-07 13:12:58 UTC
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?
Comment 34 Katrin Fischer 2022-07-07 13:13:14 UTC
*** Bug 28219 has been marked as a duplicate of this bug. ***
Comment 35 Julian Maurice 2022-09-30 10:02:52 UTC
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>
Comment 36 Julian Maurice 2022-09-30 10:04:52 UTC
Patch rebased on master
Comment 37 Michaela Sieber 2022-09-30 11:45:23 UTC
(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.
Comment 38 Julian Maurice 2022-09-30 11:56:04 UTC
I had the same issue. After applying the patch, you need to run updatedatabase.pl, update_dbix_class_files.pl, and then restart Koha.
Comment 39 Michaela Sieber 2022-09-30 12:44:34 UTC
(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)
Comment 40 Julian Maurice 2022-09-30 13:06:21 UTC
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 ?
Comment 41 Katrin Fischer 2022-09-30 13:33:13 UTC
(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.
Comment 42 Julian Maurice 2022-09-30 13:51:35 UTC
Oh... ok then I'll check what's wrong :)
Thanks
Comment 43 Katrin Fischer 2022-10-01 08:14:19 UTC
(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.
Comment 44 Katrin Fischer 2023-01-20 14:36:19 UTC
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>
Comment 45 Katrin Fischer 2023-01-20 15:07:17 UTC
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)
Comment 46 Julian Maurice 2023-01-20 16:18:52 UTC
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>
Comment 47 Julian Maurice 2023-01-20 16:18:59 UTC
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
Comment 48 Julian Maurice 2023-01-20 16:35:28 UTC
Created attachment 145540 [details] [review]
Bug 11844: Fix QA issues (POD, TT filters, file exec flag)
Comment 49 Julian Maurice 2023-01-20 16:36:55 UTC
(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.
Comment 50 Michaela Sieber 2023-01-21 09:43:24 UTC
(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?"
Comment 51 Julian Maurice 2023-01-23 08:00:17 UTC
(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
Comment 52 Katrin Fischer 2023-01-23 08:05:11 UTC
(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/
Comment 53 Julian Maurice 2023-01-23 08:18:33 UTC
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.
Comment 54 Katrin Fischer 2023-01-23 08:31:27 UTC
(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?
Comment 55 Michaela Sieber 2023-01-24 20:28:10 UTC
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"
Comment 56 Katrin Fischer 2023-02-10 15:44:49 UTC
Trying to see if I can help out here...
Comment 57 Katrin Fischer 2023-02-10 16:46:38 UTC
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>
Comment 58 Katrin Fischer 2023-02-10 16:46:42 UTC
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>
Comment 59 Katrin Fischer 2023-02-10 16:46:46 UTC
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>
Comment 60 Katrin Fischer 2023-02-10 16:46:50 UTC
Created attachment 146502 [details] [review]
Bug 11844: [DO NOT PUSH] Schema changes

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 61 Katrin Fischer 2023-02-10 16:46:54 UTC
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>
Comment 62 Katrin Fischer 2023-02-10 16:46:58 UTC
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>
Comment 63 Katrin Fischer 2023-02-10 16:47:01 UTC
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>
Comment 64 Katrin Fischer 2023-02-10 16:47:05 UTC
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>
Comment 65 Katrin Fischer 2023-02-10 16:50:37 UTC
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.
Comment 66 Martin Renvoize 2023-02-12 08:53:15 UTC
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.
Comment 67 Katrin Fischer 2023-02-17 09:32:08 UTC
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!
Comment 68 Julian Maurice 2023-03-29 08:48:34 UTC
Created attachment 148889 [details] [review]
Bug 11844: Add tests for Koha::AdditionalField
Comment 69 Julian Maurice 2023-03-29 09:45:38 UTC
Created attachment 148895 [details] [review]
Bug 11844: Add tests for TransferOrder
Comment 70 Julian Maurice 2023-03-29 10:26:48 UTC
Created attachment 148897 [details] [review]
Bug 11844: Add tests for marcfield_mode
Comment 71 Julian Maurice 2023-03-29 10:34:06 UTC
I think I covered everything... Back to signed off!
Comment 72 Biblibre Sandboxes 2023-04-03 09:53:18 UTC
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>
Comment 73 Biblibre Sandboxes 2023-04-03 09:53:22 UTC
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>
Comment 74 Biblibre Sandboxes 2023-04-03 09:53:25 UTC
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>
Comment 75 Biblibre Sandboxes 2023-04-03 09:53:28 UTC
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>
Comment 76 Biblibre Sandboxes 2023-04-03 09:53:32 UTC
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>
Comment 77 Biblibre Sandboxes 2023-04-03 09:53:35 UTC
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>
Comment 78 Biblibre Sandboxes 2023-04-03 09:53:38 UTC
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>
Comment 79 Biblibre Sandboxes 2023-04-03 09:53:42 UTC
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>
Comment 80 Biblibre Sandboxes 2023-04-03 09:53:45 UTC
Created attachment 149089 [details] [review]
Bug 11844: Add tests for Koha::AdditionalField

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 81 Biblibre Sandboxes 2023-04-03 09:53:48 UTC
Created attachment 149090 [details] [review]
Bug 11844: Add tests for TransferOrder

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 82 Biblibre Sandboxes 2023-04-03 09:53:52 UTC
Created attachment 149091 [details] [review]
Bug 11844: Add tests for marcfield_mode

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 83 Martin Renvoize 2023-04-14 15:06:51 UTC
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>
Comment 84 Martin Renvoize 2023-04-14 15:06:55 UTC
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>
Comment 85 Martin Renvoize 2023-04-14 15:06:59 UTC
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>
Comment 86 Martin Renvoize 2023-04-14 15:07:03 UTC
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>
Comment 87 Martin Renvoize 2023-04-14 15:07:07 UTC
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>
Comment 88 Martin Renvoize 2023-04-14 15:07:11 UTC
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>
Comment 89 Martin Renvoize 2023-04-14 15:07:15 UTC
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>
Comment 90 Martin Renvoize 2023-04-14 15:07:19 UTC
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>
Comment 91 Martin Renvoize 2023-04-14 15:07:23 UTC
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>
Comment 92 Martin Renvoize 2023-04-14 15:07:27 UTC
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>
Comment 93 Martin Renvoize 2023-04-14 15:07:31 UTC
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>
Comment 94 Martin Renvoize 2023-04-14 15:08:51 UTC
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.
Comment 95 Jonathan Druart 2023-05-02 09:53:10 UTC
(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
Comment 96 Katrin Fischer 2023-05-02 22:03:06 UTC
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>
Comment 97 Katrin Fischer 2023-05-02 22:03:10 UTC
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>
Comment 98 Katrin Fischer 2023-05-02 22:03:14 UTC
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>
Comment 99 Katrin Fischer 2023-05-02 22:03:18 UTC
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>
Comment 100 Katrin Fischer 2023-05-02 22:03:22 UTC
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>
Comment 101 Katrin Fischer 2023-05-02 22:03:26 UTC
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>
Comment 102 Katrin Fischer 2023-05-02 22:03:30 UTC
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>
Comment 103 Katrin Fischer 2023-05-02 22:03:34 UTC
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>
Comment 104 Katrin Fischer 2023-05-02 22:03:39 UTC
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>
Comment 105 Katrin Fischer 2023-05-02 22:03:43 UTC
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>
Comment 106 Katrin Fischer 2023-05-02 22:03:47 UTC
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>
Comment 107 Katrin Fischer 2023-05-02 22:03:51 UTC
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
Comment 108 Katrin Fischer 2023-05-02 22:05:42 UTC
(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.
Comment 109 Tomás Cohen Arazi 2023-05-15 18:09:30 UTC
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
Comment 110 Katrin Fischer 2023-05-15 20:53:34 UTC
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.
Comment 112 Katrin Fischer 2023-05-15 21:01:47 UTC
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>
Comment 113 Katrin Fischer 2023-05-15 21:01:52 UTC
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>
Comment 114 Katrin Fischer 2023-05-15 21:01:57 UTC
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>
Comment 115 Katrin Fischer 2023-05-15 21:02:01 UTC
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>
Comment 116 Katrin Fischer 2023-05-15 21:02:06 UTC
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>
Comment 117 Katrin Fischer 2023-05-15 21:02:10 UTC
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>
Comment 118 Katrin Fischer 2023-05-15 21:02:14 UTC
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>
Comment 119 Katrin Fischer 2023-05-15 21:02:19 UTC
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>
Comment 120 Katrin Fischer 2023-05-15 21:02:23 UTC
Created attachment 151229 [details] [review]
Bug 11844: DBIC schema
Comment 121 Katrin Fischer 2023-05-15 21:02:27 UTC
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.
Comment 122 Jonathan Druart 2023-05-16 10:59:26 UTC
Patches pushed for 23.05
Comment 123 Tomás Cohen Arazi 2023-05-16 11:17:49 UTC
Thanks for the research and follow-up, Katrina.
Comment 124 Tomás Cohen Arazi 2023-05-17 17:35:40 UTC
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>
Comment 125 Matt Blenkinsop 2023-06-02 16:18:38 UTC
New feature - not backporting to 22.11.x