Bug 38275 - Unable to delete patron card creator images
Summary: Unable to delete patron card creator images
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: David Cook
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 36192
Blocks:
  Show dependency tree / graph
 
Reported: 2024-10-25 14:34 UTC by Laura Escamilla
Modified: 2024-11-22 21:36 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes deleting images when using the patron card creator's image manager. You could not delete images, and received an error message "WARNING: An unsupported operation was attempted. Please have your system administrator check the error log for details." (This is related to the CSRF changes added in Koha 24.05 to improve form security.)
Version(s) released in:
24.11.00,24.05.06
Circulation function:

Attachments
Bug 38275: Remove "cud" from delete (687 bytes, patch)
2024-10-25 15:43 UTC, Laura Escamilla 		Details | Diff | Splinter Review
Bug 38275: Ensure that the script block is loaded after the HTML elements are available. (2.15 KB, patch)
2024-10-25 15:43 UTC, Laura Escamilla 		Details | Diff | Splinter Review
Bug 38275: Remove "cud" from delete (735 bytes, patch)
2024-10-25 18:20 UTC, David Nind 		Details | Diff | Splinter Review
Bug 38275: Ensure that the script block is loaded after the HTML elements are available. (2.19 KB, patch)
2024-10-25 18:20 UTC, David Nind 		Details | Diff | Splinter Review
Bug 38275: Fix patron card creator image delete (6.04 KB, patch)
2024-10-27 23:23 UTC, David Cook 		Details | Diff | Splinter Review
Bug 38275: Fix patron card creator image delete (6.09 KB, patch)
2024-10-27 23:47 UTC, David Nind 		Details | Diff | Splinter Review
Bug 38275: Fix patron card creator image delete (6.15 KB, patch)
2024-10-28 08:28 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Show Obsolete (6) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Laura Escamilla 2024-10-25 14:34:50 UTC 
To recreate:
1. Go to Tools > Patron Card Creator > New Image (or Manage Images)
2. Upload a photo to the image manager if one does not already exist
3. On the table labeled "Delete Images" attempt to delete the image using the "Delete" button, or try using the checkbox and "Delete selected" button. Neither method deletes the image. Instead you get the following error:
"WARNING: An unsupported operation was attempted. Please have your system administrator check the error log for details."

The logs show: "Image upload interface called an unsupported operation: delete at /usr/share/koha/intranet/cgi-bin/patroncards/image-manage.pl line 158."
Comment 1 Laura Escamilla 2024-10-25 15:43:48 UTC 
Created attachment 173397 [details] [review]
Bug 38275: Remove "cud" from delete

To test:
1. Go to Tools > Patron Card Creator > New Image (or Manage Images)
2. Upload a photo to the image manager if one does not already exist
3. On the table labeled "Delete Images" attempt to delete the image using the "Delete" button, or try using the checkbox and "Delete selected" button. Neither method deletes the image. Instead you get the following error:
"WARNING: An unsupported operation was attempted. Please have your system administrator check the error log for details."
4. Apply patch and restart_all
5. Upload more images to the image manager
6. Delete one image using the “Delete button”. The deletion is successful.
7. Use the checkboxes to select the remaining images that you want to delete. Click on “Delete selected” at the bottom of the table. All selected images are now deleted.
8. Sign off and have an amazing day! :D
Comment 2 Laura Escamilla 2024-10-25 15:43:49 UTC 
Created attachment 173398 [details] [review]
Bug 38275: Ensure that the script block is loaded after the HTML elements are available.
Comment 3 David Nind 2024-10-25 18:20:52 UTC 
Created attachment 173407 [details] [review]
Bug 38275: Remove "cud" from delete

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-10-25 18:20:54 UTC 
Created attachment 173408 [details] [review]
Bug 38275: Ensure that the script block is loaded after the HTML elements are available.

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Phil Ringnalda 2024-10-25 19:30:29 UTC 
That's one messed-up CSRF-protection avoiding page: it has a form, with method="post" and op="cud-delete" just like anything doing a delete should, but then the submit button doesn't submit the form, it just concatenates the selected_images together and sticks them on a GET of ?op=delete.
Comment 6 David Cook 2024-10-27 22:41:51 UTC 
(In reply to Phil Ringnalda from comment #5)
> That's one messed-up CSRF-protection avoiding page: it has a form, with
> method="post" and op="cud-delete" just like anything doing a delete should,
> but then the submit button doesn't submit the form, it just concatenates the
> selected_images together and sticks them on a GET of ?op=delete.

Good spotting, Phil. In this case, you should mark this bug as Failed QA. (Note: anyone can mark a bug as Failed QA.)
Comment 7 David Cook 2024-10-27 23:23:25 UTC 
Created attachment 173419 [details] [review]
Bug 38275: Fix patron card creator image delete

This change changes the patron card creator image delete from a
GET to a POST, and supports single delete and multi-delete.

Test plan:
1. Apply the patch (and refresh browser JS cache)
2. Go to http://localhost:8081/cgi-bin/koha/patroncards/image-manage.pl
3. Upload multiple images
4. Select all the images using the checkboxes
5. Delete one image using the "Delete" button in the row
6. Note that only the image selected using the "Delete" button is
deleted and the other images are de-selected
7. Delete multiple images using the "Delete selected" button
8. Note that the correct images are noted in the confirm message,
and the correct images are deleted
Comment 8 David Cook 2024-10-27 23:25:55 UTC 
(In reply to Phil Ringnalda from comment #5)
> That's one messed-up CSRF-protection avoiding page: it has a form, with
> method="post" and op="cud-delete" just like anything doing a delete should,
> but then the submit button doesn't submit the form, it just concatenates the
> selected_images together and sticks them on a GET of ?op=delete.

Apologies for taking this one over.

As Phil notes, this stateful (CUD) operation needs to be done with a POST. 

I used Phil's idea of leveraging the existing POST form.
Comment 9 David Nind 2024-10-27 23:47:38 UTC 
Created attachment 173420 [details] [review]
Bug 38275: Fix patron card creator image delete

This change changes the patron card creator image delete from a
GET to a POST, and supports single delete and multi-delete.

Test plan:
1. Apply the patch (and refresh browser JS cache)
2. Go to http://localhost:8081/cgi-bin/koha/patroncards/image-manage.pl
3. Upload multiple images
4. Select all the images using the checkboxes
5. Delete one image using the "Delete" button in the row
6. Note that only the image selected using the "Delete" button is
deleted and the other images are de-selected
7. Delete multiple images using the "Delete selected" button
8. Note that the correct images are noted in the confirm message,
and the correct images are deleted

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Martin Renvoize (ashimema) 2024-10-28 08:28:10 UTC 
Created attachment 173434 [details] [review]
Bug 38275: Fix patron card creator image delete

This change changes the patron card creator image delete from a
GET to a POST, and supports single delete and multi-delete.

Test plan:
1. Apply the patch (and refresh browser JS cache)
2. Go to http://localhost:8081/cgi-bin/koha/patroncards/image-manage.pl
3. Upload multiple images
4. Select all the images using the checkboxes
5. Delete one image using the "Delete" button in the row
6. Note that only the image selected using the "Delete" button is
deleted and the other images are de-selected
7. Delete multiple images using the "Delete selected" button
8. Note that the correct images are noted in the confirm message,
and the correct images are deleted

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Katrin Fischer 2024-10-29 07:54:52 UTC 
Pushed for 24.11!

Well done everyone, thank you!
Comment 12 Lucas Gass (lukeg) 2024-11-22 21:36:40 UTC 
Backported to 24.05.x for upcoming 24.05.06