Bug 15774 - Additional fields for baskets
Summary: Additional fields for baskets
Status: Failed QA
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
Depends on:
Blocks: 11844
  Show dependency treegraph
Reported: 2016-02-09 19:46 UTC by Jesse Weaver
Modified: 2017-10-23 17:33 UTC (History)
7 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:

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

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?