Bug 15711

Summary: Deleting patroncard images has unexpected behaviour and is broken
Product: Koha Reporter: Aleisha Amohia <aleisha>
Component: Label/patron card printingAssignee: Aleisha Amohia <aleisha>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: jonathan.druart, julian.maurice, katrin.fischer, kyle, mtj, mtompset, veron
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16747
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 15711: Fixing the 'Delete selected' button on patroncard images
[SIGNED-OFF] Bug 15711: Fixing the 'Delete selected' button on patroncard images
Bug 15711: Fixing the 'Delete selected' button on patroncard images
[SIGNED-OFF] Bug 15711: Fixing the 'Delete selected' button on patroncard images
[SIGNED-OFF] Bug 15711: Follow up batch_id not used
[SIGNED-OFF] Bug 15711: Fixing the 'Delete selected' button on patroncard images
[SIGNED-OFF] Bug 15711: Follow up batch_id not used
Bug 15711: Fixing the 'Delete selected' button on patroncard images
Bug 15711: Follow up batch_id not used

Description Aleisha Amohia 2016-02-01 01:08:46 UTC
To reproduce:

1) Go to Tools -> Patron Card Creator -> Manage Images. Upload an image if you do not already have something uploaded
2) Without checking any boxes, click the 'Delete selected' box
3) A box will pop up saying 'Please select image(s) to delete', as expected. Click 'OK'
4) Another pop-up saying 'Are you sure you want to delete image(s): undefined?' This is unexpected and shouldn't be showing up if nothing has been selected.
5) Click 'OK' - This also doesn't work (image isn't deleted) and gives an error 'WARNING: An error has occurred. Please ask your system administrator to check the error log for more details.'
6) Check the box next to an image and click 'Delete selected', then 'OK'. You get the WARNING error again and the image isn't deleted. This is broken.
Comment 1 Marc VĂ©ron 2016-06-15 20:56:02 UTC
Confirmed. I could delete an image using the button "Delete" in the same row. 
Selecting the image and using the button "Delete selected" results the warning as mentioned by Aleisha.
Comment 2 Aleisha Amohia 2016-08-24 01:12:06 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2016-08-25 15:07:13 UTC
Created attachment 54879 [details] [review]
[SIGNED-OFF] Bug 15711: Fixing the 'Delete selected' button on patroncard images

To test:
1) Go to Tools -> Patron Card Creator -> Manage images
2) Upload an image if you haven't already
3) Click Delete selected without selecting any images
4) Notice broken behaviour as described in Description
5) Apply patch, refresh page
6) Click Delete selected without selecting any images
7) Notice alert. Click OK
8) Select one image or more, click Delete selected
9) Notice confirm delete message.

Sponsored-by: Catalyst IT

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Jonathan Druart 2016-08-26 10:02:15 UTC
Aleisha, I get the exact same behavior with and without the patch:
I get the first "Please select images to delete.", then "Are you sure you want to delete image(s): undefined?"
Comment 5 Aleisha Amohia 2016-09-16 03:37:12 UTC
(In reply to Jonathan Druart from comment #4)
> Aleisha, I get the exact same behavior with and without the patch:
> I get the first "Please select images to delete.", then "Are you sure you
> want to delete image(s): undefined?"

Hi Jonathan, are you sure the patch applied correctly? I just applied and tested it again and it worked fine for me.
Comment 6 Jonathan Druart 2016-09-19 11:49:44 UTC
I still get the same behavior.
Comment 7 Jonathan Druart 2016-09-19 11:54:17 UTC
(In reply to Jonathan Druart from comment #6)
> I still get the same behavior.

  6             function DeleteConfirm() {
  7                 var results = selected_images("delete");
  8                 if (results.images != 0) {

It seems that selected_images returns -1, and so the if is evaluated true (results.images is undefined which is != 0)
Comment 8 Aleisha Amohia 2016-12-02 03:31:53 UTC
Created attachment 57877 [details] [review]
Bug 15711: Fixing the 'Delete selected' button on patroncard images

To test:
1) Go to Tools -> Patron Card Creator -> Manage images
2) Upload an image if you haven't already
3) Click Delete selected without selecting any images
4) Notice broken behaviour as described in Description
5) Apply patch, refresh page
6) Click Delete selected without selecting any images
7) Notice alert. Click OK
8) Select one image or more, click Delete selected
9) Notice confirm delete message.

Sponsored-by: Catalyst IT

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Mark Tompsett 2016-12-02 19:43:50 UTC
Created attachment 57944 [details] [review]
[SIGNED-OFF] Bug 15711: Fixing the 'Delete selected' button on patroncard images

To test:
1) Go to Tools -> Patron Card Creator -> Manage images
2) Upload an image if you haven't already
3) Click Delete selected without selecting any images
4) Notice broken behaviour as described in Description
5) Apply patch, refresh page
6) Click Delete selected without selecting any images
7) Notice alert. Click OK
8) Select one image or more, click Delete selected
9) Notice confirm delete message.

Sponsored-by: Catalyst IT

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 10 Mark Tompsett 2016-12-02 19:43:59 UTC
Created attachment 57945 [details] [review]
[SIGNED-OFF] Bug 15711: Follow up batch_id not used

Changed batch_id to image_id in the template, as the multiparam
would catch the array case.

TEST PLAN
---------
1) attempt to delete a selected item, but get a warning.
2) apply this follow up
3) attempt to delete a selected item, it deletes.
4) run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 11 Mark Tompsett 2016-12-02 19:45:18 UTC
Given that I wrote the follow up, I think another sign off is in order.
Additionally, encountered valid_template error unrelated to any of the changes made.
Comment 12 Owen Leonard 2016-12-14 18:38:57 UTC
Created attachment 58189 [details] [review]
[SIGNED-OFF] Bug 15711: Fixing the 'Delete selected' button on patroncard images

To test:
1) Go to Tools -> Patron Card Creator -> Manage images
2) Upload an image if you haven't already
3) Click Delete selected without selecting any images
4) Notice broken behaviour as described in Description
5) Apply patch, refresh page
6) Click Delete selected without selecting any images
7) Notice alert. Click OK
8) Select one image or more, click Delete selected
9) Notice confirm delete message.

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 13 Owen Leonard 2016-12-14 18:39:42 UTC
Created attachment 58190 [details] [review]
[SIGNED-OFF] Bug 15711: Follow up batch_id not used

Changed batch_id to image_id in the template, as the multiparam
would catch the array case.

TEST PLAN
---------
1) attempt to delete a selected item, but get a warning.
2) apply this follow up
3) attempt to delete a selected item, it deletes.
4) run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 14 Jonathan Druart 2016-12-27 16:08:05 UTC
Created attachment 58444 [details] [review]
Bug 15711: Fixing the 'Delete selected' button on patroncard images

To test:
1) Go to Tools -> Patron Card Creator -> Manage images
2) Upload an image if you haven't already
3) Click Delete selected without selecting any images
4) Notice broken behaviour as described in Description
5) Apply patch, refresh page
6) Click Delete selected without selecting any images
7) Notice alert. Click OK
8) Select one image or more, click Delete selected
9) Notice confirm delete message.

Sponsored-by: Catalyst IT

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2016-12-27 16:08:10 UTC
Created attachment 58445 [details] [review]
Bug 15711: Follow up batch_id not used

Changed batch_id to image_id in the template, as the multiparam
would catch the array case.

TEST PLAN
---------
1) attempt to delete a selected item, but get a warning.
2) apply this follow up
3) attempt to delete a selected item, it deletes.
4) run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Kyle M Hall 2016-12-28 13:37:35 UTC
Pushed to master for 17.05, thanks Aleisha, Mark!
Comment 17 Katrin Fischer 2017-01-04 21:43:12 UTC
These patches have been pushed to 16.11.x and will be in 16.11.02.
.
Comment 18 Julian Maurice 2017-01-19 15:31:25 UTC
Pushed to 3.22.x for 3.22.15
Comment 19 Mason James 2017-01-27 02:41:53 UTC
Pushed to 16.05.x, for 16.05.08 release