Bug 21539 - addorderiso2709.pl forces librarian to select a ccode and notforloan code when using MarcItemFieldsToOrder
Summary: addorderiso2709.pl forces librarian to select a ccode and notforloan code whe...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-10-10 15:13 UTC by Kyle M Hall
Modified: 2020-09-20 21:44 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.04, 19.11.10


Attachments
Bug 21539 - addorderiso2709.pl forces librarian to select a ccode and notforloan code when using MarcItemFieldsToOrder (2.46 KB, patch)
2018-10-10 15:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21539 - addorderiso2709.pl forces librarian to select a ccode and notforloan code when using MarcItemFieldsToOrder (2.51 KB, patch)
2020-08-18 20:41 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 21539: addorderiso2709.pl forces librarian to select a ccode and notforloan code when using MarcItemFieldsToOrder (2.56 KB, patch)
2020-08-22 16:22 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2018-10-10 15:13:26 UTC
There is no reason that MarcItemFieldsToOrder should force librarians to select a ccode and notforloan value for each item. They are both optional fields. As with location, the first value should be an empty 'none' option.
Comment 1 Kyle M Hall 2018-10-10 15:18:01 UTC
Created attachment 80361 [details] [review]
Bug 21539 - addorderiso2709.pl forces librarian to select a ccode and notforloan code when using MarcItemFieldsToOrder

There is no reason that MarcItemFieldsToOrder should force librarians to select a ccode and notforloan value for each item. They are both optional fields. As with location, the first value should be an empty 'none' option.

Test Plan:
1) Set up MarcItemFieldsToOrder, do *not* set mappings for ccode and notforloan
2) Add an order record to a basket that uses MarcItemFieldsToOrder
3) Note you must choose a ccode and nfl value
4) Apply this patch
5) Reload the page
6) Note you no longer have to select values for ccode and notforloan
Comment 2 Hayley Mapley 2019-07-25 23:00:48 UTC
(In reply to Kyle M Hall from comment #1)
> Created attachment 80361 [details] [review] [review]
> Bug 21539 - addorderiso2709.pl forces librarian to select a ccode and
> notforloan code when using MarcItemFieldsToOrder
> 
> There is no reason that MarcItemFieldsToOrder should force librarians to
> select a ccode and notforloan value for each item. They are both optional
> fields. As with location, the first value should be an empty 'none' option.
> 
> Test Plan:
> 1) Set up MarcItemFieldsToOrder, do *not* set mappings for ccode and
> notforloan
> 2) Add an order record to a basket that uses MarcItemFieldsToOrder
> 3) Note you must choose a ccode and nfl value
> 4) Apply this patch
> 5) Reload the page
> 6) Note you no longer have to select values for ccode and notforloan

Hi Kyle,

I have tried to replicate this on master but have so far been unable to. 

What do you mean when you say 'Add an order record to a basket that uses MarcItemFieldsToOrder'? I have set up the syspref before staging the records, is there something else I need to do?
Comment 3 Myka Kennedy Stephens 2019-12-25 21:12:58 UTC
I'm also finding that the test plan isn't clear.

Here's what I tried:
1. Set up MarcItemsFieldsToOrder; used the example provided in the system preferences and set mappings for holdingbranch and itype.
2. According to the manual, MarcItemsFieldsToOrder applies to item records created from a MARC record in a staged file:
    a. Staged a MARC record
    b. Opened a basket for "My Vendor"
    c. Added an order to the basket from staged MARC record
    d. Closed the basket
    e. Received a shipment
    f. Went to record and saw new item
3. At no point in the process was I prompted to enter a ccode or not for loan value. These remained blank throughout the process.

Tested on master (19.12.00.002)
Comment 4 Katrin Fischer 2019-12-29 14:16:08 UTC
Setting to FQA because of the last 2 comments - Kyle, can you please check?
Comment 5 Andrew Fuerste-Henry 2020-08-04 12:06:32 UTC
This works as intended, testing on master. Clarifying test process.

1) Have at least one field mapped in MARCItemFieldsToOrder, but do not map CCODE or location - I used "homebranch: 947$a"
2) Set AcqCreateItem to Placing an Order
3) Have a MARC record with at least one valid value in one of the fields mapped in MARCItemFieldsToOrder - I had a record with "947$aCPL"
4) Create a basket in acq, use default item creation of Placing an Order
5) Add to basket from a new file, selecting your MARC file
6) After the file has uploaded and you've clicked "Add staged file to basket," you're taken to addorderiso2709.pl
7) On addorderiso2709.pl, click to select your bib for import
8) If your values mapped via MARCItemFieldsToOrder was set successfully, your order should include an inset "Item Record 1" box with an abbreviated item record
9) Note that collection code and location have defaulted to the first authorized value and cannot be set to empty

10) Apply patch, restart
11) Repeat steps 1-8
12) Note that collection code and location are blank by default and can be set to other values

13) Add ccode and location to you MARCItemFieldsToOrder values and your MARC record
14) Add to your basket via your updated file
15) Note that CCODE and loc populate values as desired
Comment 6 Andrew Fuerste-Henry 2020-08-18 20:41:05 UTC
Created attachment 108553 [details] [review]
Bug 21539 - addorderiso2709.pl forces librarian to select a ccode and notforloan code when using MarcItemFieldsToOrder

There is no reason that MarcItemFieldsToOrder should force librarians to select a ccode and notforloan value for each item. They are both optional fields. As with location, the first value should be an empty 'none' option.

Test Plan:
1) Set up MarcItemFieldsToOrder, do *not* set mappings for ccode and notforloan
2) Add an order record to a basket that uses MarcItemFieldsToOrder
3) Note you must choose a ccode and nfl value
4) Apply this patch
5) Reload the page
6) Note you no longer have to select values for ccode and notforloan

Signed-off-by: Daniel Gaghan <daniel.gaghan@pueblolibrary.org>
Comment 7 Katrin Fischer 2020-08-22 16:22:54 UTC
Created attachment 108935 [details] [review]
Bug 21539: addorderiso2709.pl forces librarian to select a ccode and notforloan code when using MarcItemFieldsToOrder

There is no reason that MarcItemFieldsToOrder should force librarians
to select a ccode and notforloan value for each item.
They are both optional fields.
As with location, the first value should be an empty 'none' option.

Test Plan:
1) Set up MarcItemFieldsToOrder, do *not* set mappings for ccode and notforloan
2) Add an order record to a basket that uses MarcItemFieldsToOrder
3) Note you must choose a ccode and nfl value
4) Apply this patch
5) Reload the page
6) Note you no longer have to select values for ccode and notforloan

Signed-off-by: Daniel Gaghan <daniel.gaghan@pueblolibrary.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Jonathan Druart 2020-08-24 10:32:36 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 9 Lucas Gass 2020-09-04 19:39:52 UTC
backported to 20.05.x for 20.05.04
Comment 10 Aleisha Amohia 2020-09-10 04:16:52 UTC
backported to 19.11.x for 19.11.10
Comment 11 Victor Grousset/tuxayo 2020-09-20 18:21:09 UTC
Missing dependencies for 19.05.x, can't backport.