Some librarians have requested the ability to batch add items to a course using the same settings for all items.
Created attachment 73209 [details] [review] Bug 20467: Add ability to batch add items to a course Some librarians have requested the ability to batch add items to a course using the same settings for all items. Test Plan: 1) Apply this patch 2) Create a new course 3) Click the new 'Batch add reserves' button 4) Enter some valid and invalid barcodes in the text box, add some settings 5) Submit the batch, note the results list added items and invalid barcodes 6) Note the valid items were added to the course
Created attachment 73213 [details] [review] Bug 20467: Add ability to batch add items to a course Some librarians have requested the ability to batch add items to a course using the same settings for all items. Test Plan: 1) Apply this patch 2) Create a new course 3) Click the new 'Batch add reserves' button 4) Enter some valid and invalid barcodes in the text box, add some settings 5) Submit the batch, note the results list added items and invalid barcodes 6) Note the valid items were added to the course
Created attachment 73214 [details] [review] Bug 20467: Add ability to batch add items to a course Some librarians have requested the ability to batch add items to a course using the same settings for all items. Test Plan: 1) Apply this patch 2) Create a new course 3) Click the new 'Batch add reserves' button 4) Enter some valid and invalid barcodes in the text box, add some settings 5) Submit the batch, note the results list added items and invalid barcodes 6) Note the valid items were added to the course
Created attachment 73219 [details] [review] Bug 20467: Add ability to batch add items to a course Some librarians have requested the ability to batch add items to a course using the same settings for all items. Test Plan: 1) Apply this patch 2) Create a new course 3) Click the new 'Batch add reserves' button 4) Enter some valid and invalid barcodes in the text box, add some settings 5) Submit the batch, note the results list added items and invalid barcodes 6) Note the valid items were added to the course Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 73220 [details] [review] Bug 20467: Add ability to batch add items to a course Some librarians have requested the ability to batch add items to a course using the same settings for all items. Test Plan: 1) Apply this patch 2) Create a new course 3) Click the new 'Batch add reserves' button 4) Enter some valid and invalid barcodes in the text box, add some settings 5) Submit the batch, note the results list added items and invalid barcodes 6) Note the valid items were added to the course Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Created attachment 73239 [details] [review] Bug 20467: Add ability to batch add items to a course Some librarians have requested the ability to batch add items to a course using the same settings for all items. Test Plan: 1) Apply this patch 2) Create a new course 3) Click the new 'Batch add reserves' button 4) Enter some valid and invalid barcodes in the text box, add some settings 5) Submit the batch, note the results list added items and invalid barcodes 6) Note the valid items were added to the course Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Maksim Sen <maksim.sen@inlibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
QA: Have to check something, please don't push yet!
The holding library pull down is empty in the batch add form, please fix! Thx Owen for noticing!
Created attachment 73419 [details] [review] Bug 20467 [QA Followup]: Add Branches plugin to template
Created attachment 73453 [details] [review] Bug 20467: (QA follow-up) Add Branches plugin to template Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
1. Need to be patched with changes on bug 20282 2. I think you should use the TT plugin AuthorisedValues (and include file options_for_authorised_value_categories) for CCODE and LOC 3. You should use uniq on @barcodes (and use the same split we already do in tools/*: \s\n)
Created attachment 73550 [details] [review] Bug 20467 [QA Followup]: Add Branches plugin to template Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 73551 [details] [review] Bug 20467 [QA Followup] - Filter barcodes, Remove selector code as batch changes cannot have a previous selection, use template plugins
A small thing: - Hit /course_reserves/batch_add_items.pl (without course_id parameter or invalid) - add items - submit => no error - "view course" => "Invalid course!" I think we should warn before
Does it really make sense to have this new batch_add_items couple? I would have adjusted add_items to accept a batch. Why do we need 2 different screens?
Created attachment 73705 [details] [review] Bug 20467: Remove useless use statements
(In reply to Jonathan Druart from comment #15) > Does it really make sense to have this new batch_add_items couple? > I would have adjusted add_items to accept a batch. Why do we need 2 > different screens? I was going to do that at first, but once I started writing the code I realized that there wasn't much shared code. So it made sense to keep the new feature in a separate script.
I am sorry Kyle but they really look the same to me (compare them with only your first patch applied). Also, we you add a single item, you are redirected to the same 'add item' view, which means people were expecting to scan multiple items. I really think we should have only 1 page to add item to a course, scan 1+ then redirect to the course detail. I would like another QA POV.
(In reply to Jonathan Druart from comment #18) > I am sorry Kyle but they really look the same to me (compare them with only > your first patch applied). > > Also, we you add a single item, you are redirected to the same 'add item' > view, which means people were expecting to scan multiple items. > > I really think we should have only 1 page to add item to a course, scan 1+ > then redirect to the course detail. > > I would like another QA POV. I think your issue is a matter of philosophy. I think it's better to have multiple simpler scripts, whereas you definitely prefer a monolithic approach. That is fine, as there is more than one way to do it ; ) That being said, I'm not interested in rewriting this patch for the sake of a difference in style. If you'd like to submit a followup to merge the two I'd be happy to sign it off!
Created attachment 73797 [details] [review] Bug 20467 [QA Followup]: Display error if no or invalid course id is passed
I feel like the dev is good enough to be considered for inclusion. In the merge-patrons case, I did the rewrite because I didn't want to have code in Koha::* that diverted from what we've been doing for a couple years, but the logic itself was good too. In this case, those controller scripts... I don't think this matter of taste should care that much. If I was asked before this dev was written, I would've agreed with a same-page approach. And a REST endpoint to handle CRUD. But I don't think this should be rejected because of it.
When I edit an item, I get: "This course already has this item on reserve." and "Number of courses reserving this item: 1" Are they expected? (with and without the patch) Also the legend of the fieldset always says "Add" (instead of edit)
(In reply to Jonathan Druart from comment #22) > When I edit an item, I get: > "This course already has this item on reserve." > and > "Number of courses reserving this item: 1" > > Are they expected? (with and without the patch) > > Also the legend of the fieldset always says "Add" (instead of edit) Yes, that is expected though not perfect. The "Number of courses" message is exactly how it should be. We could pass an 'edit' flag to change the message and legend when purposefully editing a course item ( as opposed adding it only to find it's already on the course ). If you would file a separate bug report I'd be happy to submit a patch for that!
Back on this, will fix patch subjects.
I've filed bug 21003 for the superfluous message about the existing item. It makes no sense when editing a reserve item.
Created attachment 76468 [details] [review] Bug 20467: Add ability to batch add items to a course Some librarians have requested the ability to batch add items to a course using the same settings for all items. Test Plan: 1) Apply this patch 2) Create a new course 3) Click the new 'Batch add reserves' button 4) Enter some valid and invalid barcodes in the text box, add some settings 5) Submit the batch, note the results list added items and invalid barcodes 6) Note the valid items were added to the course Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Maksim Sen <maksim.sen@inlibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76469 [details] [review] Bug 20467: (QA follow-up) Add Branches plugin to template Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76470 [details] [review] Bug 20467: (QA follow-up) Filter barcodes, remove selector code, use template plugins Removes selector code as batch changes cannot have a previous selection. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76471 [details] [review] Bug 20467: Remove useless use statements Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76472 [details] [review] Bug 20467: (QA follow-up) Display error if no or invalid course id is passed Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Awesome work all! Pushed to master for 18.11
Enhancement, not backporting to 18.05.x series.