Bug 15774 - Additional fields for baskets
Summary: Additional fields for baskets
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 11844
  Show dependency treegraph
 
Reported: 2016-02-09 19:46 UTC by Jesse Weaver
Modified: 2018-05-16 10:46 UTC (History)
8 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


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

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
Created attachment 53608 [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 9 Jesse Weaver 2016-07-21 19:39:45 UTC
Created attachment 53609 [details] [review]
Bug 15774: (QA followup) Make sure that tablename is correctly saved

This should fix QA issues, rebasing 11844 shortly.
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
Created attachment 72078 [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 13 Julian Maurice 2018-02-22 10:33:05 UTC
Created attachment 72079 [details] [review]
Bug 15774: (QA followup) Make sure that tablename is correctly saved
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
Created attachment 72085 [details] [review]
Bug 15774: Fix AV categories dropdown list in admin
Comment 16 Julian Maurice 2018-02-22 12:54:48 UTC
Created attachment 72086 [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 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
Created attachment 75354 [details] [review]
Bug 15774: Show additional field values in edit form
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