Bug 15711 - Deleting patroncard images has unexpected behaviour and is broken
Summary: Deleting patroncard images has unexpected behaviour and is broken
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-01 01:08 UTC by Aleisha Amohia
Modified: 2017-01-27 02:41 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 15711: Fixing the 'Delete selected' button on patroncard images (1.81 KB, patch)
2016-08-24 01:12 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15711: Fixing the 'Delete selected' button on patroncard images (1.92 KB, patch)
2016-08-25 15:07 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 15711: Fixing the 'Delete selected' button on patroncard images (3.00 KB, patch)
2016-12-02 03:31 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15711: Fixing the 'Delete selected' button on patroncard images (3.07 KB, patch)
2016-12-02 19:43 UTC, M. Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15711: Follow up batch_id not used (1.48 KB, patch)
2016-12-02 19:43 UTC, M. Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15711: Fixing the 'Delete selected' button on patroncard images (3.11 KB, patch)
2016-12-14 18:38 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15711: Follow up batch_id not used (1.57 KB, patch)
2016-12-14 18:39 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 15711: Fixing the 'Delete selected' button on patroncard images (3.12 KB, patch)
2016-12-27 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15711: Follow up batch_id not used (1.59 KB, patch)
2016-12-27 16:08 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 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 M. 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 M. 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 M. 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