Bug 15774 - Additional fields for baskets
Summary: Additional fields for baskets
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Nick Clemens
QA Contact: Josef Moravec
URL:
Keywords: Manual
Depends on:
Blocks: 11844 22934
  Show dependency treegraph
 
Reported: 2016-02-09 19:46 UTC by Jesse Weaver
Modified: 2019-11-05 19:47 UTC (History)
13 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Large patch
Who signed the patch off:
Text to go in the release notes:
This new feature adds the ability to define additional fields to store information about acquisition baskets. It refactors and centralises the code used to add fields to subscriptions as well. There is a new 'Additional fields' page in the adminsitration module to configure the fields. Users can name additional fields, tie them to authorised values, and specify whether the fields can be searched in the acquisitions module.
Version(s) released in:


Attachments
Bug 15774: Add additional fields to order baskets (45.19 KB, patch)
2016-02-18 23:07 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (45.77 KB, patch)
2016-07-21 19:32 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15774: (QA followup) Make sure that tablename is correctly saved (779 bytes, patch)
2016-07-21 19:39 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (45.96 KB, patch)
2018-02-22 10:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: (QA followup) Make sure that tablename is correctly saved (786 bytes, patch)
2018-02-22 10:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: Fix AV categories dropdown list in admin (1.97 KB, patch)
2018-02-22 12:54 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.29 KB, patch)
2018-02-22 12:54 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (45.95 KB, patch)
2018-05-04 15:43 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: (QA followup) Make sure that tablename is correctly saved (786 bytes, patch)
2018-05-04 15:43 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.29 KB, patch)
2018-05-04 15:43 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: Use Koha::Object(s) for additional fields (69.36 KB, patch)
2018-05-04 15:43 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: Add POD and license notices (7.91 KB, patch)
2018-05-15 15:17 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (2.90 KB, patch)
2018-05-16 07:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (3.47 KB, patch)
2018-05-16 10:46 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (46.08 KB, patch)
2018-10-04 20:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved (780 bytes, patch)
2018-10-04 20:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.38 KB, patch)
2018-10-04 20:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Use Koha::Object(s) for additional fields (69.42 KB, patch)
2018-10-04 20:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add POD and license notices (7.87 KB, patch)
2018-10-04 20:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (3.45 KB, patch)
2018-10-04 20:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) Add html filters (11.24 KB, patch)
2018-10-04 20:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (46.23 KB, patch)
2018-10-05 12:35 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved (929 bytes, patch)
2018-10-05 12:35 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.53 KB, patch)
2018-10-05 12:35 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 15774: Use Koha::Object(s) for additional fields (69.57 KB, patch)
2018-10-05 12:35 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 15774: Add POD and license notices (8.02 KB, patch)
2018-10-05 12:35 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (3.59 KB, patch)
2018-10-05 12:35 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 15774: (follow-up) Add html filters (11.39 KB, patch)
2018-10-05 12:35 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 15774: Fix JS check when AV and MARC field are both selected (1.69 KB, patch)
2018-10-16 20:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (46.23 KB, patch)
2018-10-29 13:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved (929 bytes, patch)
2018-10-29 13:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.53 KB, patch)
2018-10-29 13:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Use Koha::Object(s) for additional fields (69.53 KB, patch)
2018-10-29 13:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add POD and license notices (8.02 KB, patch)
2018-10-29 13:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (3.52 KB, patch)
2018-10-29 13:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) Add html filters (11.39 KB, patch)
2018-10-29 13:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix JS check when AV and MARC field are both selected (1.68 KB, patch)
2018-10-29 13:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) Address QA issues (14.69 KB, patch)
2018-11-01 18:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (46.30 KB, patch)
2018-11-01 21:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved (1003 bytes, patch)
2018-11-01 21:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.60 KB, patch)
2018-11-01 21:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Use Koha::Object(s) for additional fields (69.60 KB, patch)
2018-11-01 21:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Add POD and license notices (8.09 KB, patch)
2018-11-01 21:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (3.60 KB, patch)
2018-11-01 21:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: (follow-up) Add html filters (11.46 KB, patch)
2018-11-01 21:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Fix JS check when AV and MARC field are both selected (1.75 KB, patch)
2018-11-01 21:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: (follow-up) Address QA issues (14.69 KB, patch)
2018-11-01 21:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Use the include file to display the subscription's additional fields (3.07 KB, patch)
2018-11-01 21:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Fix insert and update for subscription (1.82 KB, patch)
2018-11-01 21:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Add missing tests for GetSubscription (2.08 KB, patch)
2018-11-01 21:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields (2.73 KB, patch)
2018-11-01 21:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Fix and unify additional_fields in C4::Serials (2.62 KB, patch)
2018-11-01 21:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Do not fetch additional fields from GetSubscription (1.02 KB, patch)
2018-11-01 21:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Remove unecessary returned value for ->delete (994 bytes, patch)
2018-11-01 21:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (48.11 KB, patch)
2018-12-18 12:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved (1010 bytes, patch)
2018-12-18 12:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.62 KB, patch)
2018-12-18 12:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Use Koha::Object(s) for additional fields (69.37 KB, patch)
2018-12-18 12:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add POD and license notices (8.13 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (3.61 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) Add html filters (11.48 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix JS check when AV and MARC field are both selected (1.76 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) Address QA issues (14.89 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Use the include file to display the subscription's additional fields (3.18 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix insert and update for subscription (1.82 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields (2.74 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix and unify additional_fields in C4::Serials (2.62 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Do not fetch additional fields from GetSubscription (1.02 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Remove unecessary returned value for ->delete (1001 bytes, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add permission for managing additional fields (10.01 KB, patch)
2018-12-18 12:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (48.11 KB, patch)
2018-12-21 11:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved (1010 bytes, patch)
2018-12-21 11:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.62 KB, patch)
2018-12-21 11:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Use Koha::Object(s) for additional fields (69.39 KB, patch)
2018-12-21 11:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add POD and license notices (8.13 KB, patch)
2018-12-21 11:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (3.61 KB, patch)
2018-12-21 11:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) Add html filters (11.48 KB, patch)
2018-12-21 11:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix JS check when AV and MARC field are both selected (1.76 KB, patch)
2018-12-21 11:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) Address QA issues (14.89 KB, patch)
2018-12-21 11:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Use the include file to display the subscription's additional fields (3.18 KB, patch)
2018-12-21 11:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix insert and update for subscription (1.82 KB, patch)
2018-12-21 11:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields (2.74 KB, patch)
2018-12-21 11:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix and unify additional_fields in C4::Serials (2.62 KB, patch)
2018-12-21 11:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Do not fetch additional fields from GetSubscription (1.02 KB, patch)
2018-12-21 11:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Remove unecessary returned value for ->delete (1001 bytes, patch)
2018-12-21 11:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add permission for managing additional fields (11.24 KB, patch)
2018-12-21 11:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (48.52 KB, patch)
2019-02-20 13:09 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved (1010 bytes, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.62 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Use Koha::Object(s) for additional fields (69.55 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add POD and license notices (8.13 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (3.69 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) Add html filters (11.48 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix JS check when AV and MARC field are both selected (1.76 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) Address QA issues (14.89 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Use the include file to display the subscription's additional fields (3.18 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix insert and update for subscription (1.82 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields (2.74 KB, patch)
2019-02-20 13:10 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Fix and unify additional_fields in C4::Serials (2.71 KB, patch)
2019-02-20 13:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Do not fetch additional fields from GetSubscription (1.17 KB, patch)
2019-02-20 13:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Remove unecessary returned value for ->delete (1001 bytes, patch)
2019-02-20 13:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add permission for managing additional fields (11.67 KB, patch)
2019-02-20 13:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: (follow-up) fix rebase issues (3.18 KB, patch)
2019-02-27 18:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15774: Add additional fields to order baskets (48.57 KB, patch)
2019-02-28 08:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved (1.04 KB, patch)
2019-02-28 08:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Fix additional fields filters (4.67 KB, patch)
2019-02-28 08:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Use Koha::Object(s) for additional fields (69.60 KB, patch)
2019-02-28 08:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Add POD and license notices (8.18 KB, patch)
2019-02-28 08:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Show additional field values in edit form (3.74 KB, patch)
2019-02-28 08:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: (follow-up) Add html filters (11.53 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Fix JS check when AV and MARC field are both selected (1.81 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: (follow-up) Address QA issues (14.95 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Use the include file to display the subscription's additional fields (3.24 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Fix insert and update for subscription (1.88 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields (2.79 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Fix and unify additional_fields in C4::Serials (2.77 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Do not fetch additional fields from GetSubscription (1.23 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Remove unecessary returned value for ->delete (1.03 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: Add permission for managing additional fields (11.72 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: (follow-up) fix rebase issues (3.23 KB, patch)
2019-02-28 08:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Fix use statemenets (1.33 KB, patch)
2019-02-28 08:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: (QA follow-up) Remove asset of datables.css, and corresponding plugins (1.09 KB, patch)
2019-02-28 08:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 15774: (follow-up) Fix TestBuilder.t so it doesn't catch wrong classes (1.05 KB, patch)
2019-03-09 08:42 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 Jesse Weaver 2016-02-09 19:46:47 UTC

    
Comment 1 Jonathan Druart 2016-02-10 08:50:44 UTC
Jesse, since we will have 3 different places to configure the additional fields, it would be great to have a configuration page in the admin area to configure all of them on the same page.
Comment 2 Jesse Weaver 2016-02-11 16:21:23 UTC
That's exactly the plan, Jonathan.
Comment 3 Jesse Weaver 2016-02-18 23:07:43 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-02-19 15:34:48 UTC
The tablename is not saved when I create a new field for aqbasket.
Comment 5 Jonathan Druart 2016-02-19 15:35:32 UTC
Note that I would prefer to this patch based on top of bug 11844.
Comment 6 Jesse Weaver 2016-04-21 22:29:04 UTC
I'll fix these issues, but could I ask you to prepare a patch to rebase bug 11844? There's a bit of mucking around that has to be done that I'd prefer was done by somebody familiar with the code. I'll happily QA it.
Comment 7 Jonathan Druart 2016-04-22 20:38:15 UTC
I have changed the status of bug 11844 to "does not apply". Hopefully Julian will rebase it quickly.
Comment 8 Jesse Weaver 2016-07-21 19:32:59 UTC Comment hidden (obsolete)
Comment 9 Jesse Weaver 2016-07-21 19:39:45 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2016-09-14 14:07:24 UTC
QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the users searched for and the additional fields linked to an AV are not displayed as a dropdown list.

2/ What does mean an additional field linked to a MARC field for aqbasket?
I think that this option should be hidden for this table (additional fields admin page, /acqui/histsearch.pl, add basket page, etc.)

3/ No test coverage for update_fields_from_query, get_filters_from_query and get_filters_as_values.

4/ When searching an order, there is no way not to set a "all" value for additional fields linked to AV.

5/ When searching an order and the search does not return any results, the field should contain what the user filled in.

6/ update_fields_from_query and get_filters_from_query should not deal with the CGI object. It would be better to pass the relevant variables instead.

Marked as Failed QA.
Comment 11 Katrin Fischer 2017-08-16 00:09:17 UTC
Can this be rescued?
Comment 12 Julian Maurice 2018-02-22 10:33:00 UTC Comment hidden (obsolete)
Comment 13 Julian Maurice 2018-02-22 10:33:05 UTC Comment hidden (obsolete)
Comment 14 Julian Maurice 2018-02-22 10:34:15 UTC
Patches rebased on master

QA comments in comment 10 still need to be addressed.
Comment 15 Julian Maurice 2018-02-22 12:54:41 UTC Comment hidden (obsolete)
Comment 16 Julian Maurice 2018-02-22 12:54:48 UTC Comment hidden (obsolete)
Comment 17 Julian Maurice 2018-02-22 13:02:55 UTC
(In reply to Jonathan Druart from comment #10)
> 2/ What does mean an additional field linked to a MARC field for aqbasket?
> I think that this option should be hidden for this table (additional fields
> admin page, /acqui/histsearch.pl, add basket page, etc.)

I think we should answer to that before fixing the other issues.
Changing status to In Discussion
Comment 18 Nick Clemens 2018-02-23 11:59:05 UTC
I agree with Jonathan, I don't think that option makes sense and should be hidden/disabled for basket fields.
Comment 19 Katrin Fischer 2018-02-23 12:35:01 UTC
Great - follow up patch? :)
Comment 20 Nick Clemens 2018-02-23 15:06:46 UTC
(In reply to Katrin Fischer from comment #19)
> Great - follow up patch? :)

Will attempt, having some issues in the code right now, will dig later
Comment 21 Julian Maurice 2018-05-04 15:43:38 UTC
Created attachment 75083 [details] [review]
Bug 15774: Add additional fields to order baskets

This also moves the admin page for additional fields for all tables to a
single common screen, and factors out display/input parsing logic.

Test plan:
  1. Create an additional field for a subscription (under Serials -> Add
     subscription fields).
  2. Apply patch.
  3. Visit Additional fields under administration, and verify that
     the field created above still shows under the list for the
     subscription table.
  4. Create at least four fields for aqbasket for each combination of
     searchable/not-searchable and with/without an authorized value.
  5. Create an order basket, and verify that all fields are visible and
     correctly save.
  6. Edit the basket, verifying that changes to these additional fields
     are saved.
  7. Add an order to the basket (contents are irrelevant).
  8. Go to advanced search within acquisitions.
  9. Verify that only the searchable fields show in the form, and that
     their contents may be searched.
Comment 22 Julian Maurice 2018-05-04 15:43:43 UTC
Created attachment 75084 [details] [review]
Bug 15774: (QA followup) Make sure that tablename is correctly saved
Comment 23 Julian Maurice 2018-05-04 15:43:47 UTC
Created attachment 75085 [details] [review]
Bug 15774: Fix additional fields filters

Addresses the following QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the
users searched for and the additional fields linked to an AV are not
displayed as a dropdown list.

4/ When searching an order, there is no way not to set a "all" value for
additional fields linked to AV.

5/ When searching an order and the search does not return any results,
the field should contain what the user filled in.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15774#c10
Comment 24 Julian Maurice 2018-05-04 15:43:54 UTC
Created attachment 75086 [details] [review]
Bug 15774: Use Koha::Object(s) for additional fields

A lot of code can be removed just by using Koha::Object

It also makes fetching and updating additional field values easier.
Comment 25 Julian Maurice 2018-05-04 15:47:17 UTC
Patches rebased on master + new patch (attachment 75086 [details] [review])

Reset bug status to Needs signoff since the last patch brings a lot of changes.
To test, use test plan from comment 21
Comment 26 Julian Maurice 2018-05-15 15:17:43 UTC
Created attachment 75336 [details] [review]
Bug 15774: Add POD and license notices
Comment 27 Julian Maurice 2018-05-16 07:32:28 UTC Comment hidden (obsolete)
Comment 28 Julian Maurice 2018-05-16 10:46:56 UTC
Created attachment 75361 [details] [review]
Bug 15774: Show additional field values in edit form
Comment 29 Katrin Fischer 2018-09-27 23:07:13 UTC
i tried to rebase this... but failed :(

The method Koha::Subscription->additional_field_values is not covered by tests!

Nick, could you give this a go?
Comment 30 Nick Clemens 2018-10-04 20:10:39 UTC
Created attachment 79976 [details] [review]
Bug 15774: Add additional fields to order baskets

This also moves the admin page for additional fields for all tables to a
single common screen, and factors out display/input parsing logic.

Test plan:
  1. Create an additional field for a subscription (under Serials -> Add
     subscription fields).
  2. Apply patch.
  3. Visit Additional fields under administration, and verify that
     the field created above still shows under the list for the
     subscription table.
  4. Create at least four fields for aqbasket for each combination of
     searchable/not-searchable and with/without an authorized value.
  5. Create an order basket, and verify that all fields are visible and
     correctly save.
  6. Edit the basket, verifying that changes to these additional fields
     are saved.
  7. Add an order to the basket (contents are irrelevant).
  8. Go to advanced search within acquisitions.
  9. Verify that only the searchable fields show in the form, and that
     their contents may be searched.
Comment 31 Nick Clemens 2018-10-04 20:10:45 UTC
Created attachment 79977 [details] [review]
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved
Comment 32 Nick Clemens 2018-10-04 20:10:50 UTC
Created attachment 79978 [details] [review]
Bug 15774: Fix additional fields filters

Addresses the following QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the
users searched for and the additional fields linked to an AV are not
displayed as a dropdown list.

4/ When searching an order, there is no way not to set a "all" value for
additional fields linked to AV.

5/ When searching an order and the search does not return any results,
the field should contain what the user filled in.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15774#c10
Comment 33 Nick Clemens 2018-10-04 20:10:55 UTC
Created attachment 79979 [details] [review]
Bug 15774: Use Koha::Object(s) for additional fields

A lot of code can be removed just by using Koha::Object

It also makes fetching and updating additional field values easier.
Comment 34 Nick Clemens 2018-10-04 20:10:59 UTC
Created attachment 79980 [details] [review]
Bug 15774: Add POD and license notices
Comment 35 Nick Clemens 2018-10-04 20:11:04 UTC
Created attachment 79981 [details] [review]
Bug 15774: Show additional field values in edit form
Comment 36 Nick Clemens 2018-10-04 20:11:09 UTC
Created attachment 79982 [details] [review]
Bug 15774: (follow-up) Add html filters
Comment 37 Séverine Queune 2018-10-05 12:35:06 UTC
Created attachment 80060 [details] [review]
Bug 15774: Add additional fields to order baskets

This also moves the admin page for additional fields for all tables to a
single common screen, and factors out display/input parsing logic.

Test plan:
  1. Create an additional field for a subscription (under Serials -> Add
     subscription fields).
  2. Apply patch.
  3. Visit Additional fields under administration, and verify that
     the field created above still shows under the list for the
     subscription table.
  4. Create at least four fields for aqbasket for each combination of
     searchable/not-searchable and with/without an authorized value.
  5. Create an order basket, and verify that all fields are visible and
     correctly save.
  6. Edit the basket, verifying that changes to these additional fields
     are saved.
  7. Add an order to the basket (contents are irrelevant).
  8. Go to advanced search within acquisitions.
  9. Verify that only the searchable fields show in the form, and that
     their contents may be searched.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 38 Séverine Queune 2018-10-05 12:35:11 UTC
Created attachment 80061 [details] [review]
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 39 Séverine Queune 2018-10-05 12:35:16 UTC
Created attachment 80062 [details] [review]
Bug 15774: Fix additional fields filters

Addresses the following QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the
users searched for and the additional fields linked to an AV are not
displayed as a dropdown list.

4/ When searching an order, there is no way not to set a "all" value for
additional fields linked to AV.

5/ When searching an order and the search does not return any results,
the field should contain what the user filled in.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15774#c10
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 40 Séverine Queune 2018-10-05 12:35:21 UTC
Created attachment 80063 [details] [review]
Bug 15774: Use Koha::Object(s) for additional fields

A lot of code can be removed just by using Koha::Object

It also makes fetching and updating additional field values easier.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 41 Séverine Queune 2018-10-05 12:35:25 UTC
Created attachment 80064 [details] [review]
Bug 15774: Add POD and license notices

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 42 Séverine Queune 2018-10-05 12:35:30 UTC
Created attachment 80065 [details] [review]
Bug 15774: Show additional field values in edit form

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 43 Séverine Queune 2018-10-05 12:35:34 UTC
Created attachment 80066 [details] [review]
Bug 15774: (follow-up) Add html filters

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 44 Jonathan Druart 2018-10-16 20:11:41 UTC
Created attachment 80680 [details] [review]
Bug 15774: Fix JS check when AV and MARC field are both selected
Comment 45 Jonathan Druart 2018-10-16 20:24:49 UTC
* Tests missing for GetHistory
* +    my $rs = Koha::Database->new->schema->resultset('AdditionalFieldValue');
must be Koha::AdditionalFieldValue instead.
I would suggest to have a look at Koha::Patron::Category and how the 'branch_limitation' add/update/remove is done, in order to follow an existing pattern
* additional_field_values must return a Koha::Objects
* Should not search_additional_fields be more search_*by*_additional_fields?
I think we agreed on "filter_by_*" somewhere else (Martin?)
* +        # TODO Add column additional_field_values.tablename to avoid subquery ?
I do not think it is relevant, we want the tablename at additional_fields level
* +    for my $field (Koha::AdditionalFields->search({ tablename => 'aqbasket' })) {
Better to use while ->next instead

These patches are great, please provide a quick follow-up and we will make it part of 18.11!
Comment 46 Katrin Fischer 2018-10-16 20:27:40 UTC
I think search_by doesn't sound quite right... search_on? search_in? Native speaker? :)
Comment 47 Martin Renvoize 2018-10-16 20:59:37 UTC
How's about 'filter_by' as per the suggestions/discussion over on bug 11983 ?
Comment 48 M. Tompsett 2018-10-17 17:22:37 UTC
(In reply to Katrin Fischer from comment #46)
> I think search_by doesn't sound quite right... search_on? search_in? Native
> speaker? :)

search_for? I don't like filter_by, because filter_by suggests we are taking a given set and shrinking it. No such thing is passed. We are searching, but it is limited for a particular set of conditions. The rest would describe the condition. And why not just get_by_marc_field (would do tagfield, but tagsubfield is included, I believe), get_by_kohafield?
Comment 49 Nick Clemens 2018-10-29 13:33:34 UTC
Created attachment 81490 [details] [review]
Bug 15774: Add additional fields to order baskets

This also moves the admin page for additional fields for all tables to a
single common screen, and factors out display/input parsing logic.

Test plan:
  1. Create an additional field for a subscription (under Serials -> Add
     subscription fields).
  2. Apply patch.
  3. Visit Additional fields under administration, and verify that
     the field created above still shows under the list for the
     subscription table.
  4. Create at least four fields for aqbasket for each combination of
     searchable/not-searchable and with/without an authorized value.
  5. Create an order basket, and verify that all fields are visible and
     correctly save.
  6. Edit the basket, verifying that changes to these additional fields
     are saved.
  7. Add an order to the basket (contents are irrelevant).
  8. Go to advanced search within acquisitions.
  9. Verify that only the searchable fields show in the form, and that
     their contents may be searched.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 50 Nick Clemens 2018-10-29 13:33:39 UTC
Created attachment 81491 [details] [review]
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 51 Nick Clemens 2018-10-29 13:33:43 UTC
Created attachment 81492 [details] [review]
Bug 15774: Fix additional fields filters

Addresses the following QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the
users searched for and the additional fields linked to an AV are not
displayed as a dropdown list.

4/ When searching an order, there is no way not to set a "all" value for
additional fields linked to AV.

5/ When searching an order and the search does not return any results,
the field should contain what the user filled in.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15774#c10
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 52 Nick Clemens 2018-10-29 13:33:48 UTC
Created attachment 81493 [details] [review]
Bug 15774: Use Koha::Object(s) for additional fields

A lot of code can be removed just by using Koha::Object

It also makes fetching and updating additional field values easier.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 53 Nick Clemens 2018-10-29 13:33:53 UTC
Created attachment 81494 [details] [review]
Bug 15774: Add POD and license notices

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 54 Nick Clemens 2018-10-29 13:33:58 UTC
Created attachment 81495 [details] [review]
Bug 15774: Show additional field values in edit form

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 55 Nick Clemens 2018-10-29 13:34:03 UTC
Created attachment 81496 [details] [review]
Bug 15774: (follow-up) Add html filters

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 56 Nick Clemens 2018-10-29 13:34:08 UTC
Created attachment 81497 [details] [review]
Bug 15774: Fix JS check when AV and MARC field are both selected
Comment 57 Martin Renvoize 2018-11-01 15:11:45 UTC
(In reply to M. Tompsett from comment #48)
> (In reply to Katrin Fischer from comment #46)
> > I think search_by doesn't sound quite right... search_on? search_in? Native
> > speaker? :)
> 
> search_for? I don't like filter_by, because filter_by suggests we are taking
> a given set and shrinking it. No such thing is passed. We are searching, but
> it is limited for a particular set of conditions. The rest would describe
> the condition. And why not just get_by_marc_field (would do tagfield, but
> tagsubfield is included, I believe), get_by_kohafield?

But you are filtering... you pass in a resultset (whether that be a new one or an existing already partially filtered one) and then refine it by filtering down...

We're trying to bring consistency to the objects, so to me filter_by which is already being discussed elsewhere seems to make sense.
Comment 58 Nick Clemens 2018-11-01 18:32:33 UTC
Created attachment 81833 [details] [review]
Bug 15774: (follow-up) Address QA issues
Comment 59 Jonathan Druart 2018-11-01 21:08:25 UTC
Created attachment 81837 [details] [review]
Bug 15774: Add additional fields to order baskets

This also moves the admin page for additional fields for all tables to a
single common screen, and factors out display/input parsing logic.

Test plan:
  1. Create an additional field for a subscription (under Serials -> Add
     subscription fields).
  2. Apply patch.
  3. Visit Additional fields under administration, and verify that
     the field created above still shows under the list for the
     subscription table.
  4. Create at least four fields for aqbasket for each combination of
     searchable/not-searchable and with/without an authorized value.
  5. Create an order basket, and verify that all fields are visible and
     correctly save.
  6. Edit the basket, verifying that changes to these additional fields
     are saved.
  7. Add an order to the basket (contents are irrelevant).
  8. Go to advanced search within acquisitions.
  9. Verify that only the searchable fields show in the form, and that
     their contents may be searched.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 60 Jonathan Druart 2018-11-01 21:08:34 UTC
Created attachment 81838 [details] [review]
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 61 Jonathan Druart 2018-11-01 21:08:44 UTC
Created attachment 81839 [details] [review]
Bug 15774: Fix additional fields filters

Addresses the following QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the
users searched for and the additional fields linked to an AV are not
displayed as a dropdown list.

4/ When searching an order, there is no way not to set a "all" value for
additional fields linked to AV.

5/ When searching an order and the search does not return any results,
the field should contain what the user filled in.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15774#c10
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 62 Jonathan Druart 2018-11-01 21:08:53 UTC
Created attachment 81840 [details] [review]
Bug 15774: Use Koha::Object(s) for additional fields

A lot of code can be removed just by using Koha::Object

It also makes fetching and updating additional field values easier.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 63 Jonathan Druart 2018-11-01 21:09:01 UTC
Created attachment 81841 [details] [review]
Bug 15774: Add POD and license notices

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 64 Jonathan Druart 2018-11-01 21:09:09 UTC
Created attachment 81842 [details] [review]
Bug 15774: Show additional field values in edit form

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 65 Jonathan Druart 2018-11-01 21:09:18 UTC
Created attachment 81843 [details] [review]
Bug 15774: (follow-up) Add html filters

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 66 Jonathan Druart 2018-11-01 21:09:27 UTC
Created attachment 81844 [details] [review]
Bug 15774: Fix JS check when AV and MARC field are both selected

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 67 Jonathan Druart 2018-11-01 21:09:34 UTC
Created attachment 81845 [details] [review]
Bug 15774: (follow-up) Address QA issues

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 68 Jonathan Druart 2018-11-01 21:09:44 UTC
Created attachment 81846 [details] [review]
Bug 15774: Use the include file to display the subscription's additional fields

We have it, why not to use it?

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 69 Jonathan Druart 2018-11-01 21:09:53 UTC
Created attachment 81847 [details] [review]
Bug 15774: Fix insert and update for subscription

Call it a typo

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 70 Jonathan Druart 2018-11-01 21:10:02 UTC
Created attachment 81848 [details] [review]
Bug 15774: Add missing tests for GetSubscription

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 71 Jonathan Druart 2018-11-01 21:10:11 UTC
Created attachment 81849 [details] [review]
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 72 Jonathan Druart 2018-11-01 21:10:19 UTC
Created attachment 81850 [details] [review]
Bug 15774: Fix and unify additional_fields in C4::Serials

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 73 Jonathan Druart 2018-11-01 21:10:28 UTC
Created attachment 81851 [details] [review]
Bug 15774: Do not fetch additional fields from GetSubscription

Finally we do not want that, they should be fetch when needed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 74 Jonathan Druart 2018-11-01 21:10:37 UTC
Created attachment 81852 [details] [review]
Bug 15774: Remove unecessary returned value for ->delete

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 75 Jonathan Druart 2018-11-01 21:11:53 UTC
Comment on attachment 81848 [details] [review]
Bug 15774: Add missing tests for GetSubscription

We finally do not need that I'd say.
Comment 76 Nick Clemens 2018-11-08 13:17:45 UTC
This needs a new permission now to reflect changes made by 14391
Comment 77 Katrin Fischer 2018-11-08 13:42:17 UTC
(In reply to Nick Clemens from comment #76)
> This needs a new permission now to reflect changes made by 14391

Hm, are these under administration now?
Comment 78 Katrin Fischer 2018-11-12 19:56:06 UTC
Some conflicts:

CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc
Auto-merging admin/additional-fields.pl
Auto-merging acqui/histsearch.pl
CONFLICT (content): Merge conflict in acqui/histsearch.pl
Auto-merging acqui/basket.pl
Auto-merging C4/Acquisition.pm
CONFLICT (content): Merge conflict in C4/Acquisition.pm
error: Failed to merge in the changes.

For permissions: 

Until this patch everyone with any serials permission could add/update additional fields: { serials => '*' },

It's a little hard to find a good pref for a feature that spans multiple-modules now. I think a new admin permission might make the most sense. Then update patrons with serials permissions to have that.
Comment 79 Jonathan Druart 2018-11-13 17:12:36 UTC
Patches rebased: https://gitlab.com/joubu/Koha/commits/bug_15774
Comment 80 Nick Clemens 2018-12-18 12:12:42 UTC
Created attachment 83349 [details] [review]
Bug 15774: Add additional fields to order baskets

This also moves the admin page for additional fields for all tables to a
single common screen, and factors out display/input parsing logic.

Test plan:
  1. Create an additional field for a subscription (under Serials -> Add
     subscription fields).
  2. Apply patch.
  3. Visit Additional fields under administration, and verify that
     the field created above still shows under the list for the
     subscription table.
  4. Create at least four fields for aqbasket for each combination of
     searchable/not-searchable and with/without an authorized value.
  5. Create an order basket, and verify that all fields are visible and
     correctly save.
  6. Edit the basket, verifying that changes to these additional fields
     are saved.
  7. Add an order to the basket (contents are irrelevant).
  8. Go to advanced search within acquisitions.
  9. Verify that only the searchable fields show in the form, and that
     their contents may be searched.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 81 Nick Clemens 2018-12-18 12:12:48 UTC
Created attachment 83350 [details] [review]
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 82 Nick Clemens 2018-12-18 12:12:53 UTC
Created attachment 83351 [details] [review]
Bug 15774: Fix additional fields filters

Addresses the following QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the
users searched for and the additional fields linked to an AV are not
displayed as a dropdown list.

4/ When searching an order, there is no way not to set a "all" value for
additional fields linked to AV.

5/ When searching an order and the search does not return any results,
the field should contain what the user filled in.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15774#c10
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 83 Nick Clemens 2018-12-18 12:12:57 UTC
Created attachment 83352 [details] [review]
Bug 15774: Use Koha::Object(s) for additional fields

A lot of code can be removed just by using Koha::Object

It also makes fetching and updating additional field values easier.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 84 Nick Clemens 2018-12-18 12:13:02 UTC
Created attachment 83353 [details] [review]
Bug 15774: Add POD and license notices

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 85 Nick Clemens 2018-12-18 12:13:08 UTC
Created attachment 83354 [details] [review]
Bug 15774: Show additional field values in edit form

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 86 Nick Clemens 2018-12-18 12:13:14 UTC
Created attachment 83355 [details] [review]
Bug 15774: (follow-up) Add html filters

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 87 Nick Clemens 2018-12-18 12:13:19 UTC
Created attachment 83356 [details] [review]
Bug 15774: Fix JS check when AV and MARC field are both selected

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 88 Nick Clemens 2018-12-18 12:13:24 UTC
Created attachment 83357 [details] [review]
Bug 15774: (follow-up) Address QA issues

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 89 Nick Clemens 2018-12-18 12:13:29 UTC
Created attachment 83358 [details] [review]
Bug 15774: Use the include file to display the subscription's additional fields

We have it, why not to use it?

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 90 Nick Clemens 2018-12-18 12:13:34 UTC
Created attachment 83359 [details] [review]
Bug 15774: Fix insert and update for subscription

Call it a typo

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 91 Nick Clemens 2018-12-18 12:13:39 UTC
Created attachment 83360 [details] [review]
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 92 Nick Clemens 2018-12-18 12:13:44 UTC
Created attachment 83361 [details] [review]
Bug 15774: Fix and unify additional_fields in C4::Serials

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 93 Nick Clemens 2018-12-18 12:13:48 UTC
Created attachment 83362 [details] [review]
Bug 15774: Do not fetch additional fields from GetSubscription

Finally we do not want that, they should be fetch when needed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 94 Nick Clemens 2018-12-18 12:13:53 UTC
Created attachment 83363 [details] [review]
Bug 15774: Remove unecessary returned value for ->delete

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 95 Nick Clemens 2018-12-18 12:13:58 UTC
Created attachment 83364 [details] [review]
Bug 15774: Add permission for managing additional fields

To test:
 1 - Have three borrowers, one with order_manage permission, one with
 edit_subscription permisson, andone with both
 2 - Apply patch, updatedatabase
 3 - Verify all three now have the manage_additional_fields permission
 4 - Visit the admin page with these users, they should all see the
 'Manage additional fields' link
 5 - Click the link
 6 - User with order_manage should see 'Order baskest'
 7 - User with edit_subscription should see 'Subscriptions'
 8 - User with both should see both
 9 - Remove the additional permissions from a user - they should see a
 note about needing additional permissions
Comment 96 Katrin Fischer 2018-12-20 10:47:57 UTC
Comment on attachment 83364 [details] [review]
Bug 15774: Add permission for managing additional fields

Review of attachment 83364 [details] [review]:
-----------------------------------------------------------------

Hi Nick,

I like he approach of the new permission depending on order_manage and edit_subscription, but I think it's not quite ready yet. Trying to comment with Splinter.

1) You need to update installer/data/mysql/userpermissions.sql so the permission is installed for new installations.

::: admin/additional-fields.pl
@@ +33,5 @@
> +my $op = $input->param('op') // ( $tablename ? 'list' : 'list_tables' );
> +
> +if( $op ne 'list_tables' ){
> +    $flagsrequired{acquisition} = 'order_manage' if $tablename eq 'aqbasket';
> +    $flagsrequired{serials} = 'edit_subscription' if $tablename eq 'subscription';

2) I think you'd want to change this to use the new permission now.

::: installer/data/mysql/atomicupdate/Bug_15774_add_additional_fields_permissions.perl
@@ +6,5 @@
> +        (3,'manage_additional_fields','Add, edit, or delete additional custom fields for baskets or subscriptions (also requires order_manage or edit_subscription permissions)')
> +    });
> +    $dbh->do( q{
> +        INSERT INTO user_permissions (borrowernumber, module_bit, code)
> +        SELECT borrowernumber, 3, 'manage_additional_fields' FROM borrowers WHERE borrowernumber IN (SELECT DISTINCT borrowernumber FROM user_permissions WHERE code = 'order_manage' OR code = 'edit_subscription');

3) This will work for everyone who has not full Serials permission. If the top level "Serials" permission is checked, the granular ones are not in user_permissions. You also need to check for those using the bit operation.
Comment 97 Nick Clemens 2018-12-21 11:41:18 UTC
Created attachment 83444 [details] [review]
Bug 15774: Add additional fields to order baskets

This also moves the admin page for additional fields for all tables to a
single common screen, and factors out display/input parsing logic.

Test plan:
  1. Create an additional field for a subscription (under Serials -> Add
     subscription fields).
  2. Apply patch.
  3. Visit Additional fields under administration, and verify that
     the field created above still shows under the list for the
     subscription table.
  4. Create at least four fields for aqbasket for each combination of
     searchable/not-searchable and with/without an authorized value.
  5. Create an order basket, and verify that all fields are visible and
     correctly save.
  6. Edit the basket, verifying that changes to these additional fields
     are saved.
  7. Add an order to the basket (contents are irrelevant).
  8. Go to advanced search within acquisitions.
  9. Verify that only the searchable fields show in the form, and that
     their contents may be searched.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 98 Nick Clemens 2018-12-21 11:41:23 UTC
Created attachment 83445 [details] [review]
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 99 Nick Clemens 2018-12-21 11:41:28 UTC
Created attachment 83446 [details] [review]
Bug 15774: Fix additional fields filters

Addresses the following QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the
users searched for and the additional fields linked to an AV are not
displayed as a dropdown list.

4/ When searching an order, there is no way not to set a "all" value for
additional fields linked to AV.

5/ When searching an order and the search does not return any results,
the field should contain what the user filled in.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15774#c10
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 100 Nick Clemens 2018-12-21 11:41:33 UTC
Created attachment 83447 [details] [review]
Bug 15774: Use Koha::Object(s) for additional fields

A lot of code can be removed just by using Koha::Object

It also makes fetching and updating additional field values easier.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 101 Nick Clemens 2018-12-21 11:41:39 UTC
Created attachment 83448 [details] [review]
Bug 15774: Add POD and license notices

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 102 Nick Clemens 2018-12-21 11:41:44 UTC
Created attachment 83449 [details] [review]
Bug 15774: Show additional field values in edit form

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 103 Nick Clemens 2018-12-21 11:41:49 UTC
Created attachment 83450 [details] [review]
Bug 15774: (follow-up) Add html filters

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 104 Nick Clemens 2018-12-21 11:41:54 UTC
Created attachment 83451 [details] [review]
Bug 15774: Fix JS check when AV and MARC field are both selected

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 105 Nick Clemens 2018-12-21 11:41:59 UTC
Created attachment 83452 [details] [review]
Bug 15774: (follow-up) Address QA issues

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 106 Nick Clemens 2018-12-21 11:42:04 UTC
Created attachment 83453 [details] [review]
Bug 15774: Use the include file to display the subscription's additional fields

We have it, why not to use it?

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 107 Nick Clemens 2018-12-21 11:42:09 UTC
Created attachment 83454 [details] [review]
Bug 15774: Fix insert and update for subscription

Call it a typo

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 108 Nick Clemens 2018-12-21 11:42:14 UTC
Created attachment 83455 [details] [review]
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 109 Nick Clemens 2018-12-21 11:42:19 UTC
Created attachment 83456 [details] [review]
Bug 15774: Fix and unify additional_fields in C4::Serials

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 110 Nick Clemens 2018-12-21 11:42:24 UTC
Created attachment 83457 [details] [review]
Bug 15774: Do not fetch additional fields from GetSubscription

Finally we do not want that, they should be fetch when needed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 111 Nick Clemens 2018-12-21 11:42:28 UTC
Created attachment 83458 [details] [review]
Bug 15774: Remove unecessary returned value for ->delete

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 112 Nick Clemens 2018-12-21 11:42:33 UTC
Created attachment 83459 [details] [review]
Bug 15774: Add permission for managing additional fields

To test:
 1 - Have three borrowers, one with order_manage permission, one with
 edit_subscription permisson, andone with both
 2 - Apply patch, updatedatabase
 3 - Verify all three now have the manage_additional_fields permission
 4 - Visit the admin page with these users, they should all see the
 'Manage additional fields' link
 5 - Click the link
 6 - User with order_manage should see 'Order baskest'
 7 - User with edit_subscription should see 'Subscriptions'
 8 - User with both should see both
 9 - Remove the additional permissions from a user - they should see a
 note about needing additional permissions
Comment 113 Katrin Fischer 2018-12-21 11:47:39 UTC
ah, I think I understand 2) now.
Comment 114 Nick Clemens 2018-12-21 11:52:05 UTC
(In reply to Katrin Fischer from comment #96)
> I like the approach of the new permission depending on order_manage and
> edit_subscription
Excellent!

> 1) You need to update installer/data/mysql/userpermissions.sql so the
> permission is installed for new installations.
Done!

> ::: admin/additional-fields.pl
> @@ +33,5 @@
> > +my $op = $input->param('op') // ( $tablename ? 'list' : 'list_tables' );
> > +
> > +if( $op ne 'list_tables' ){
> > +    $flagsrequired{acquisition} = 'order_manage' if $tablename eq 'aqbasket';
> > +    $flagsrequired{serials} = 'edit_subscription' if $tablename eq 'subscription';
> 
> 2) I think you'd want to change this to use the new permission now.
I do set it in the lines above - the main permission gets you to the page, you need the others to access the individual modules

> 
> :::
> installer/data/mysql/atomicupdate/
> Bug_15774_add_additional_fields_permissions.perl
> @@ +6,5 @@
> > +        (3,'manage_additional_fields','Add, edit, or delete additional custom fields for baskets or subscriptions (also requires order_manage or edit_subscription permissions)')
> > +    });
> > +    $dbh->do( q{
> > +        INSERT INTO user_permissions (borrowernumber, module_bit, code)
> > +        SELECT borrowernumber, 3, 'manage_additional_fields' FROM borrowers WHERE borrowernumber IN (SELECT DISTINCT borrowernumber FROM user_permissions WHERE code = 'order_manage' OR code = 'edit_subscription');
> 
> 3) This will work for everyone who has not full Serials permission. If the
> top level "Serials" permission is checked, the granular ones are not in
> user_permissions. You also need to check for those using the bit operation.
Updated
Comment 115 Katrin Fischer 2019-02-03 11:33:02 UTC
I am sorry, but there are some conflicts. Please rebase, I'll try to come back to this soon:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15774: Add additional fields to order baskets
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/includes/serials-menu.inc
M	koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt
M	serials/serials-search.pl
M	serials/subscription-add.pl
Falling back to patching base and 3-way merge...
Auto-merging serials/subscription-add.pl
CONFLICT (content): Merge conflict in serials/subscription-add.pl
Auto-merging serials/serials-search.pl
CONFLICT (content): Merge conflict in serials/serials-search.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt
Removing koha-tmpl/intranet-tmpl/prog/en/modules/serials/add_fields.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/serials-menu.inc
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/serials-menu.inc
Auto-merging admin/additional-fields.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 15774: Add additional fields to order baskets
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-15774-Add-additional-fields-to-order-baskets-LGaSot.patch
Comment 116 Nick Clemens 2019-02-20 13:09:59 UTC
Created attachment 85333 [details] [review]
Bug 15774: Add additional fields to order baskets

This also moves the admin page for additional fields for all tables to a
single common screen, and factors out display/input parsing logic.

Test plan:
  1. Create an additional field for a subscription (under Serials -> Add
     subscription fields).
  2. Apply patch.
  3. Visit Additional fields under administration, and verify that
     the field created above still shows under the list for the
     subscription table.
  4. Create at least four fields for aqbasket for each combination of
     searchable/not-searchable and with/without an authorized value.
  5. Create an order basket, and verify that all fields are visible and
     correctly save.
  6. Edit the basket, verifying that changes to these additional fields
     are saved.
  7. Add an order to the basket (contents are irrelevant).
  8. Go to advanced search within acquisitions.
  9. Verify that only the searchable fields show in the form, and that
     their contents may be searched.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 117 Nick Clemens 2019-02-20 13:10:05 UTC
Created attachment 85334 [details] [review]
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 118 Nick Clemens 2019-02-20 13:10:11 UTC
Created attachment 85335 [details] [review]
Bug 15774: Fix additional fields filters

Addresses the following QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the
users searched for and the additional fields linked to an AV are not
displayed as a dropdown list.

4/ When searching an order, there is no way not to set a "all" value for
additional fields linked to AV.

5/ When searching an order and the search does not return any results,
the field should contain what the user filled in.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15774#c10
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 119 Nick Clemens 2019-02-20 13:10:18 UTC
Created attachment 85336 [details] [review]
Bug 15774: Use Koha::Object(s) for additional fields

A lot of code can be removed just by using Koha::Object

It also makes fetching and updating additional field values easier.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 120 Nick Clemens 2019-02-20 13:10:23 UTC
Created attachment 85337 [details] [review]
Bug 15774: Add POD and license notices

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 121 Nick Clemens 2019-02-20 13:10:28 UTC
Created attachment 85338 [details] [review]
Bug 15774: Show additional field values in edit form

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 122 Nick Clemens 2019-02-20 13:10:33 UTC
Created attachment 85339 [details] [review]
Bug 15774: (follow-up) Add html filters

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 123 Nick Clemens 2019-02-20 13:10:38 UTC
Created attachment 85340 [details] [review]
Bug 15774: Fix JS check when AV and MARC field are both selected

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 124 Nick Clemens 2019-02-20 13:10:43 UTC
Created attachment 85341 [details] [review]
Bug 15774: (follow-up) Address QA issues

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 125 Nick Clemens 2019-02-20 13:10:48 UTC
Created attachment 85342 [details] [review]
Bug 15774: Use the include file to display the subscription's additional fields

We have it, why not to use it?

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 126 Nick Clemens 2019-02-20 13:10:53 UTC
Created attachment 85343 [details] [review]
Bug 15774: Fix insert and update for subscription

Call it a typo

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 127 Nick Clemens 2019-02-20 13:10:57 UTC
Created attachment 85344 [details] [review]
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 128 Nick Clemens 2019-02-20 13:11:02 UTC
Created attachment 85345 [details] [review]
Bug 15774: Fix and unify additional_fields in C4::Serials

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 129 Nick Clemens 2019-02-20 13:11:07 UTC
Created attachment 85346 [details] [review]
Bug 15774: Do not fetch additional fields from GetSubscription

Finally we do not want that, they should be fetch when needed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 130 Nick Clemens 2019-02-20 13:11:12 UTC
Created attachment 85347 [details] [review]
Bug 15774: Remove unecessary returned value for ->delete

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 131 Nick Clemens 2019-02-20 13:11:18 UTC
Created attachment 85348 [details] [review]
Bug 15774: Add permission for managing additional fields

To test:
 1 - Have three borrowers, one with order_manage permission, one with
 edit_subscription permisson, andone with both
 2 - Apply patch, updatedatabase
 3 - Verify all three now have the manage_additional_fields permission
 4 - Visit the admin page with these users, they should all see the
 'Manage additional fields' link
 5 - Click the link
 6 - User with order_manage should see 'Order baskest'
 7 - User with edit_subscription should see 'Subscriptions'
 8 - User with both should see both
 9 - Remove the additional permissions from a user - they should see a
 note about needing additional permissions
Comment 132 Josef Moravec 2019-02-25 16:51:22 UTC
Sorry, but QA tools are not happy:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/additional-fields.tt
   FAIL	  forbidden patterns
		forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 180)


 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
   FAIL	  forbidden patterns
		forbidden pattern: merge marker (=======) (line 212)
		forbidden pattern: merge marker (>>>>>>>) (line 214)
		forbidden pattern: merge marker (<<<<<<<) (line 206)
   FAIL	  valid_template
		parse error - /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt line 225: unexpected end of input



 FAIL	serials/serials-search.pl
   FAIL	  valid
		serials/serials-search.pl had compilation errors.
		Global symbol "$additional_fields" requires explicit package name (did you forget to declare "my $additional_fields"?) 
		Global symbol "$additional_field_filters" requires explicit package name (did you forget to declare "my $additional_field_filters"?) 


Also, I have one concern: this patchset does much more than this report describes.
I would like to see, at least the refactoring part of this code (change to Koha::Object and adapt current codebase on it) as own report.
Comment 133 Jonathan Druart 2019-02-27 13:41:13 UTC
(In reply to Josef Moravec from comment #132)
> Also, I have one concern: this patchset does much more than this report
> describes.
> I would like to see, at least the refactoring part of this code (change to
> Koha::Object and adapt current codebase on it) as own report.

The patch set is huge, and the dependency with bug 11844 tricky. I would not suggest such changes as we may broke everything (again).

What advantage(s) could it bring here?
Comment 134 Jonathan Druart 2019-02-27 13:42:29 UTC
/broke/break/
Comment 135 Nick Clemens 2019-02-27 18:44:33 UTC
Created attachment 85792 [details] [review]
Bug 15774: (follow-up) fix rebase issues
Comment 136 Nick Clemens 2019-02-27 18:47:06 UTC
(In reply to Josef Moravec from comment #132)
> Sorry, but QA tools are not happy:

Fixed QA tool errors
Comment 137 Josef Moravec 2019-02-28 07:47:48 UTC
(In reply to Jonathan Druart from comment #133)
> (In reply to Josef Moravec from comment #132)
> > Also, I have one concern: this patchset does much more than this report
> > describes.
> > I would like to see, at least the refactoring part of this code (change to
> > Koha::Object and adapt current codebase on it) as own report.
> 
> The patch set is huge, and the dependency with bug 11844 tricky. I would not
> suggest such changes as we may broke everything (again).
> 
> What advantage(s) could it bring here?

Thanks for discussion, I will not block it then.
Comment 138 Josef Moravec 2019-02-28 08:06:31 UTC
Created attachment 85806 [details] [review]
Bug 15774: Add additional fields to order baskets

This also moves the admin page for additional fields for all tables to a
single common screen, and factors out display/input parsing logic.

Test plan:
  1. Create an additional field for a subscription (under Serials -> Add
     subscription fields).
  2. Apply patch.
  3. Visit Additional fields under administration, and verify that
     the field created above still shows under the list for the
     subscription table.
  4. Create at least four fields for aqbasket for each combination of
     searchable/not-searchable and with/without an authorized value.
  5. Create an order basket, and verify that all fields are visible and
     correctly save.
  6. Edit the basket, verifying that changes to these additional fields
     are saved.
  7. Add an order to the basket (contents are irrelevant).
  8. Go to advanced search within acquisitions.
  9. Verify that only the searchable fields show in the form, and that
     their contents may be searched.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 139 Josef Moravec 2019-02-28 08:06:37 UTC
Created attachment 85807 [details] [review]
Bug 15774: (QA follow-up) Make sure that tablename is correctly saved

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 140 Josef Moravec 2019-02-28 08:06:42 UTC
Created attachment 85808 [details] [review]
Bug 15774: Fix additional fields filters

Addresses the following QA comments:

1/ Regression:
On serials/serials-search.pl, the filters are not filled with what the
users searched for and the additional fields linked to an AV are not
displayed as a dropdown list.

4/ When searching an order, there is no way not to set a "all" value for
additional fields linked to AV.

5/ When searching an order and the search does not return any results,
the field should contain what the user filled in.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15774#c10
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 141 Josef Moravec 2019-02-28 08:06:48 UTC
Created attachment 85809 [details] [review]
Bug 15774: Use Koha::Object(s) for additional fields

A lot of code can be removed just by using Koha::Object

It also makes fetching and updating additional field values easier.

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 142 Josef Moravec 2019-02-28 08:06:53 UTC
Created attachment 85810 [details] [review]
Bug 15774: Add POD and license notices

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 143 Josef Moravec 2019-02-28 08:06:58 UTC
Created attachment 85811 [details] [review]
Bug 15774: Show additional field values in edit form

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 144 Josef Moravec 2019-02-28 08:07:04 UTC
Created attachment 85812 [details] [review]
Bug 15774: (follow-up) Add html filters

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 145 Josef Moravec 2019-02-28 08:07:09 UTC
Created attachment 85813 [details] [review]
Bug 15774: Fix JS check when AV and MARC field are both selected

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 146 Josef Moravec 2019-02-28 08:07:15 UTC
Created attachment 85814 [details] [review]
Bug 15774: (follow-up) Address QA issues

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 147 Josef Moravec 2019-02-28 08:07:21 UTC
Created attachment 85815 [details] [review]
Bug 15774: Use the include file to display the subscription's additional fields

We have it, why not to use it?

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 148 Josef Moravec 2019-02-28 08:07:26 UTC
Created attachment 85816 [details] [review]
Bug 15774: Fix insert and update for subscription

Call it a typo

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 149 Josef Moravec 2019-02-28 08:07:31 UTC
Created attachment 85817 [details] [review]
Bug 15774: Add missing tests for Koha::Object::Mixin::AdditionalFields

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 150 Josef Moravec 2019-02-28 08:07:36 UTC
Created attachment 85818 [details] [review]
Bug 15774: Fix and unify additional_fields in C4::Serials

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 151 Josef Moravec 2019-02-28 08:07:42 UTC
Created attachment 85819 [details] [review]
Bug 15774: Do not fetch additional fields from GetSubscription

Finally we do not want that, they should be fetch when needed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 152 Josef Moravec 2019-02-28 08:07:47 UTC
Created attachment 85820 [details] [review]
Bug 15774: Remove unecessary returned value for ->delete

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 153 Josef Moravec 2019-02-28 08:07:52 UTC
Created attachment 85821 [details] [review]
Bug 15774: Add permission for managing additional fields

To test:
 1 - Have three borrowers, one with order_manage permission, one with
 edit_subscription permisson, andone with both
 2 - Apply patch, updatedatabase
 3 - Verify all three now have the manage_additional_fields permission
 4 - Visit the admin page with these users, they should all see the
 'Manage additional fields' link
 5 - Click the link
 6 - User with order_manage should see 'Order baskest'
 7 - User with edit_subscription should see 'Subscriptions'
 8 - User with both should see both
 9 - Remove the additional permissions from a user - they should see a
 note about needing additional permissions

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 154 Josef Moravec 2019-02-28 08:07:58 UTC
Created attachment 85822 [details] [review]
Bug 15774: (follow-up) fix rebase issues

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 155 Josef Moravec 2019-02-28 08:08:03 UTC
Created attachment 85823 [details] [review]
Bug 15774: (QA follow-up) Fix use statemenets

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 156 Josef Moravec 2019-02-28 08:08:09 UTC
Created attachment 85824 [details] [review]
Bug 15774: (QA follow-up) Remove asset of datables.css, and corresponding plugins

It is now part of doc-head-close.inc

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 157 Nick Clemens 2019-03-07 21:19:42 UTC
Awesome work all!

Pushed to master for 19.05
Comment 158 Tomás Cohen Arazi 2019-03-09 08:42:21 UTC
Created attachment 86411 [details] [review]
Bug 15774: (follow-up) Fix TestBuilder.t so it doesn't catch wrong classes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 159 Martin Renvoize 2019-03-12 08:36:56 UTC
New feature will not be backported to 18.11.x series