Bug 14648 - Batch remove reserve items
Summary: Batch remove reserve items
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Course reserves (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Assignee: Joe Sikowitz
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-05 13:05 UTC by Tom Misilo
Modified: 2020-11-16 21:32 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
This allows to batch remove items from any course reserve they have been added to using a barcode list. Access the new feature using the "Batch remove reserves" button on the course reserves module start page.
Version(s) released in:
20.11.00


Attachments
Bug 14648: Batch Remove Reserve Items (4.13 KB, patch)
2020-05-21 13:12 UTC, Joe Sikowitz
Details | Diff | Splinter Review
Bug-14648-Batch-remove-reserve-items (9.00 KB, patch)
2020-06-06 13:27 UTC, Joe Sikowitz
Details | Diff | Splinter Review
Bug 14648: Batch remove reserve items (9.07 KB, patch)
2020-06-07 11:32 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14648: (follow-up) Remove formatting from note for better translatability (1.45 KB, patch)
2020-06-07 11:32 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14648: Batch remove reserve items (10.11 KB, patch)
2020-07-17 15:44 UTC, Joe Sikowitz
Details | Diff | Splinter Review
Bug 14648: Batch remove reserve items (10.15 KB, patch)
2020-09-16 18:06 UTC, David Nind
Details | Diff | Splinter Review
Bug 14648: Batch remove reserve items (10.21 KB, patch)
2020-09-17 20:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14648: (QA follow-up) Add missing filters (1.63 KB, patch)
2020-09-17 20:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14648: (QA follow-up) Terminology items-reserves delete-remove (2.71 KB, patch)
2020-09-17 20:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14648: Take advantage of I18N to deal with plural (1.75 KB, patch)
2020-09-30 13:17 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Misilo 2015-08-05 13:05:07 UTC
Hi,

It would be great to see the ability to mass remove course reserves. For example, if given a list of Barcodes have the option to remove the items from any course they are tied to.

This would be useful at the end of an academic semester, when books are returned to their original shelving location.
Comment 1 Hannah Olsen 2019-08-02 14:59:22 UTC
We use Course Reserves for our displays and would find this feature very useful as well. One can do a workaround through deactivating the course, but there are times when that is not feasible.
Comment 2 Joe Sikowitz 2020-05-21 13:12:53 UTC
Created attachment 105190 [details] [review]
Bug 14648: Batch Remove Reserve Items

Patch adds a button to course-details page based on the delete_reserves permission. Test plan in patch description.
Comment 3 Katrin Fischer 2020-05-22 22:57:53 UTC
Hi Joe, once you attach a patch for testing, don't forget to set the status to 'Needs signoff' so testers can see it in the queue :)
Comment 4 Katrin Fischer 2020-05-22 23:03:47 UTC
Hi Joe,

having a look at the patch:

1) QA test tools

We have a script the QA team uses to find common mistakes and coding guideline variations in patches. It's a good habit for devs to run the script on their patches too. In this case it alerts of tabs:

 FAIL	course_reserves/course-details.pl
   OK	  critic
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 55)
		forbidden pattern: tab char (line 56)
		forbidden pattern: tab char (line 57)
		forbidden pattern: tab char (line 58)
   OK	  git manipulation
   OK	  pod
   OK	  spelling
   OK	  valid

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt
   OK	  filters
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 39)
		forbidden pattern: tab char (line 40)
		forbidden pattern: tab char (line 41)
		forbidden pattern: tab char (line 290)
		forbidden pattern: tab char (line 291)
		forbidden pattern: tab char (line 292)
   OK	  git manipulation

In Koha we have agreed to use 4 spaces and no tabs. Please check your editor settings, that should make things easier.

2) I'd like to suggest to change the label on the button. The 'batch' features usually work by giving it a list, what you do is delete all - so I think we should we should name it that and leave the 'Batch remove...' to be added later.
"Remove all reserves"
Comment 5 Joe Sikowitz 2020-05-23 13:28:51 UTC
Hi Katrin. Thanks for the feedback on the patch. I'll update it without tabs and make the change in the language of the button. I'll then readd the patch and post it for testing. Thanks!
Comment 6 Katrin Fischer 2020-05-23 13:39:55 UTC
Sounds perfect, thank you!
Comment 7 Joe Sikowitz 2020-05-26 14:45:28 UTC
Moved patch to Bug 25606 as "Remove all reserves" button.
Comment 8 Hannah Olsen 2020-05-26 15:08:38 UTC
Wait, so is this set up to only remove *everything* from the reserve? Not to let you choose which ones to remove?
Comment 9 Joe Sikowitz 2020-05-26 15:11:52 UTC
Hi Hannah, you can choose to remove items one-by-one. I moved my original patch to Bug 25606 to add an option to remove all items on a course record at once. As Katrin describes, this bug will be used to create a batch remove reserves function which will involve entering barcodes and then removing those items from all course reserves to which they are attached.
Comment 10 Joe Sikowitz 2020-06-06 13:27:51 UTC
Created attachment 105607 [details] [review]
Bug-14648-Batch-remove-reserve-items
Comment 11 Joe Sikowitz 2020-06-06 13:28:52 UTC
Test Plan
1. Log in as a user with permissions to add and delete course reserves
2. Add the same items to multiple course reserves
3. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
4. Click the "- Batch remove items" button
5. Enter the barcodes that you added to multiple reserves in step 1
6. Click submit
7. Click Ok if you are ready to delete all of these items
8. Check each course reserve that you added the items to in step 1 to make sure the items you entered were deleted but nothing else
9. Log out
10. Log in as a user without permission to delete course reserves
11. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
12. You should not longer see the "- Batch remove items" button
Comment 12 Katrin Fischer 2020-06-07 11:32:02 UTC
Created attachment 105609 [details] [review]
Bug 14648: Batch remove reserve items

Adds "Batch remove items" button to course-reserves.tt. Creates batch_rm_items.pl and batch_rm_items.tt. Allows user to enter a list of
barcode numbers so that they will be removed from all course reserves to which they are attached. A dialog box will provide a warning
before the final deletion is made.

Test Plan
1. Log in as a user with permissions to add and delete course reserves
2. Add the same items to multiple course reserves
3. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
4. Click the "- Batch remove items" button
5. Enter the barcodes that you added to multiple reserves in step 1
6. Click submit
7. Click Ok if you are ready to delete all of these items
8. Check each course reserve that you added the items to in step 1 to make sure the items you entered were deleted but nothing else
9. Log out
10. Log in as a user without permission to delete course reserves
11. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
12. You should not longer see the "- Batch remove items" button

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2020-06-07 11:32:06 UTC
Created attachment 105610 [details] [review]
Bug 14648: (follow-up) Remove formatting from note for better translatability

Removes <i></i> to avoid that the sentence is split into separate
parts in translation files.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2020-06-07 11:34:05 UTC
Hi Joe,

thx for the patch! 

I noticed the attached patch file had a bit of a strange description - are you using git bz for attaching your patches? It makes interaction with Bugzilla very easy.

I've done a follow-up, that I'd like to explain a bit:

<p>All course reserve items will be deleted from <i>all</i> courses to which they are attached.</p>

Will result in 3 separate strings to translate.

- All course reserve items will be deleted from
- all
- courses to which they are attached

I've seen there is a pre-existing occurence on the batch add feature, but we should fix that another time.
Comment 15 Joe Sikowitz 2020-06-08 14:00:42 UTC
Hi Katrin, thanks for the info. No, I haven't used git bz before, but I'll look at it and start to use it. I'm a bit new to the git/Bugzilla world so I'm still learning the ins and outs of getting my patches to the community. Thanks for all of your help!
Comment 16 Katrin Fischer 2020-07-04 21:15:33 UTC
QA team ... I signed off, can one of you take over here please?
Comment 17 Nick Clemens 2020-07-07 12:30:28 UTC
(In reply to Katrin Fischer from comment #16)
> QA team ... I signed off, can one of you take over here please?

These patches keep things simple and work as expected.

Blocker:
1 - If I enter barcodes that are not in any courses I report successful deletion from all courses - I don't think that's right

Couple questions:
1 - Why not a bulk option to remove from a single course?
2 - Could the results report which courses the items were deleted from, at the very least maybe the number of courses it was removed from?
3 - I would rather see this use Objects, but CourseReserves could use a refactor as a whole
Comment 18 Joe Sikowitz 2020-07-07 15:18:29 UTC
Hi Nick,

These patches keep things simple and work as expected.

Blocker:
1 - If I enter barcodes that are not in any courses I report successful deletion from all courses - I don't think that's right

I'm going to fix this one and readd the patch.

Couple questions:
1 - Why not a bulk option to remove from a single course?

I've already submitted this patch and it has been signed off as bug 25606.

2 - Could the results report which courses the items were deleted from, at the very least maybe the number of courses it was removed from?

This sounds reasonable. I'll look into adding it.

3 - I would rather see this use Objects, but CourseReserves could use a refactor as a whole

I'm not sure what Objects you are referring to. Any information you can provide would be great.

Thanks,
Joe
Comment 19 Nick Clemens 2020-07-09 10:48:35 UTC
(In reply to Joe Sikowitz from comment #18)
> Hi Nick,
> 
> These patches keep things simple and work as expected.
> 
> Blocker:
> 1 - If I enter barcodes that are not in any courses I report successful
> deletion from all courses - I don't think that's right
> 
> I'm going to fix this one and readd the patch.
> 
> Couple questions:
> 1 - Why not a bulk option to remove from a single course?
> 
> I've already submitted this patch and it has been signed off as bug 25606.
> 
> 2 - Could the results report which courses the items were deleted from, at
> the very least maybe the number of courses it was removed from?
> 
> This sounds reasonable. I'll look into adding it.
> 
> 3 - I would rather see this use Objects, but CourseReserves could use a
> refactor as a whole
> 
> I'm not sure what Objects you are referring to. Any information you can
> provide would be great.
> 
> Thanks,
> Joe

For #3 I meant using:
Koha::Course::Reserve->find({itemnumber=> $item->id});
instead of:
GetItemCourseReservesInfo(itemnumber => $item->id);

Probably we should have a 'course_reserves' function in Item.pm or something, but I think that is beyond scope here
Comment 20 Joe Sikowitz 2020-07-17 15:44:45 UTC
Created attachment 107043 [details] [review]
Bug 14648: Batch remove reserve items

Adds "Batch remove items" button to course-reserves.tt. Creates batch_rm_items.pl and batch_rm_items.tt. Allows user to enter a list of
barcode numbers so that they will be removed from all course reserves to which they are attached. A dialog box will provide a warning
before the final deletion is made.

This patch also now moves barcodes that are invalid or not attached to a course reserve to the invalid barcodes output. It also provides
a count of the courses that the deleted items were attached to.

Test Plan
1. Log in as a user with permissions to add and delete course reserves
2. Add the same items to multiple course reserves
3. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
4. Click the "- Batch remove items" button
5. Enter the barcodes that you added to multiple reserves in step 1
6. Click submit
7. Click Ok if you are ready to delete all of these items
8. Check each course reserve that you added the items to in step 1 to make sure the items you entered were deleted but nothing else
9. Repeat 2-5
10. Add an invalid barcode and a valid barcode that is not attached to a course reserve
11. Repeat 6-7
12. You should see that the items attached to course reserves were removed while the invalid and unattached barcodes were rejected
13. Log out
14. Log in as a user without permission to delete course reserves
15. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
16. You should not longer see the "- Batch remove items" button
Comment 21 Joe Sikowitz 2020-09-15 18:49:58 UTC
I set this back to the "Needs Signoff" status even though the first iteration already went to QA and failed. I'm not sure if this is the correct process, but it's been pending for a while.
Comment 22 David Nind 2020-09-16 18:06:11 UTC
Created attachment 110228 [details] [review]
Bug 14648: Batch remove reserve items

Adds "Batch remove items" button to course-reserves.tt. Creates batch_rm_items.pl and batch_rm_items.tt. Allows user to enter a list of
barcode numbers so that they will be removed from all course reserves to which they are attached. A dialog box will provide a warning
before the final deletion is made.

This patch also now moves barcodes that are invalid or not attached to a course reserve to the invalid barcodes output. It also provides
a count of the courses that the deleted items were attached to.

Test Plan
1. Log in as a user with permissions to add and delete course reserves
2. Add the same items to multiple course reserves
3. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
4. Click the "- Batch remove items" button
5. Enter the barcodes that you added to multiple reserves in step 1
6. Click submit
7. Click Ok if you are ready to delete all of these items
8. Check each course reserve that you added the items to in step 1 to make sure the items you entered were deleted but nothing else
9. Repeat 2-5
10. Add an invalid barcode and a valid barcode that is not attached to a course reserve
11. Repeat 6-7
12. You should see that the items attached to course reserves were removed while the invalid and unattached barcodes were rejected
13. Log out
14. Log in as a user without permission to delete course reserves
15. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
16. You should not longer see the "- Batch remove items" button

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 Katrin Fischer 2020-09-17 20:25:29 UTC
Hi Joe, once things have been fixed, you can usually go back to Signed off if the changes were not too major (in this case, do a follow-up patch instead of replacing the original one, so QA can easier see the new changes). Set back to "Needs Signoff" for more drastic changes.
Comment 24 Katrin Fischer 2020-09-17 20:36:51 UTC
Created attachment 110293 [details] [review]
Bug 14648: Batch remove reserve items

Adds "Batch remove items" button to course-reserves.tt. Creates batch_rm_items.pl and batch_rm_items.tt. Allows user to enter a list of
barcode numbers so that they will be removed from all course reserves to which they are attached. A dialog box will provide a warning
before the final deletion is made.

This patch also now moves barcodes that are invalid or not attached to a course reserve to the invalid barcodes output. It also provides
a count of the courses that the deleted items were attached to.

Test Plan
1. Log in as a user with permissions to add and delete course reserves
2. Add the same items to multiple course reserves
3. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
4. Click the "- Batch remove items" button
5. Enter the barcodes that you added to multiple reserves in step 1
6. Click submit
7. Click Ok if you are ready to delete all of these items
8. Check each course reserve that you added the items to in step 1 to make sure the items you entered were deleted but nothing else
9. Repeat 2-5
10. Add an invalid barcode and a valid barcode that is not attached to a course reserve
11. Repeat 6-7
12. You should see that the items attached to course reserves were removed while the invalid and unattached barcodes were rejected
13. Log out
14. Log in as a user without permission to delete course reserves
15. Go to the main course reserves page (i.e. /cgi-bin/koha/course_reserves/course-reserves.pl)
16. You should not longer see the "- Batch remove items" button

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2020-09-17 20:36:57 UTC
Created attachment 110294 [details] [review]
Bug 14648: (QA follow-up) Add missing filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2020-09-17 20:37:03 UTC
Created attachment 110295 [details] [review]
Bug 14648: (QA follow-up) Terminology items-reserves delete-remove

- As we have batch add reserves, we should also have batch remove reserves
- Delete items sounds a bit scary, removed items I think is a little better here

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Jonathan Druart 2020-09-30 13:02:25 UTC
Would it make sense to dissociate invalid (non-existent) barcodes from items that are not attached to a course (on the result screen)?
Comment 28 Jonathan Druart 2020-09-30 13:17:07 UTC
Created attachment 110988 [details] [review]
Bug 14648: Take advantage of I18N to deal with plural
Comment 29 Jonathan Druart 2020-10-01 09:44:01 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 30 Lucas Gass 2020-10-20 18:10:08 UTC
enhancement will not be backported to 20.05.x