Bug 17181 - Patron card creator replaces existing image when uploading image with same name
Summary: Patron card creator replaces existing image when uploading image with same name
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 normal (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 15000 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-08-24 00:35 UTC by Aleisha Amohia
Modified: 2017-07-31 11:31 UTC (History)
5 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 17181: Check for duplicate image names when uploading image to patron card creator (7.10 KB, patch)
2017-01-11 00:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17181: Check for duplicate image names when uploading image to patron card creator (7.16 KB, patch)
2017-01-11 12:52 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 17181: Check for duplicate image names when uploading image to patron card creator (6.97 KB, patch)
2017-01-12 23:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 17181: Check for duplicate image names when uploading image to patron card creator (6.97 KB, patch)
2017-01-13 08:39 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 17181: Check for duplicate image names when uploading image to patron card creator (7.04 KB, patch)
2017-01-13 09:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17181: Simplify code (1.95 KB, patch)
2017-01-13 09:24 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-08-24 00:35:12 UTC
If you try to upload an image into Patron Card Creator with the same file name as an image that has already been upload, the original image gets replaced, without any warning. There should be some kind of warning before replacing, i.e. "The name "filename" already exists. Upload and replace image or cancel upload?"
Comment 1 Aleisha Amohia 2017-01-11 00:43:23 UTC
Created attachment 58789 [details] [review]
Bug 17181: Check for duplicate image names when uploading image to patron card creator

This patch adds a check for duplicates before uploading the image.

To test:
1) Go to Tools -> Patron card creator -> Manage images
2) If you haven't already, upload an image
3) Try to upload another image with the same image name
4) Notice the first image is replaced with the second image, with no
warning.
5) Apply patch and refresh page
6) Try to upload an image with the same image name again
7) Notice you are now warned about a duplicate image name.
8) Check that uploading an image with a unique name still works.

Sponsored-by: Catalyst IT
Comment 2 Claire Gravely 2017-01-11 12:52:49 UTC
Created attachment 58808 [details] [review]
Bug 17181: Check for duplicate image names when uploading image to patron card creator

This patch adds a check for duplicates before uploading the image.

To test:
1) Go to Tools -> Patron card creator -> Manage images
2) If you haven't already, upload an image
3) Try to upload another image with the same image name
4) Notice the first image is replaced with the second image, with no
warning.
5) Apply patch and refresh page
6) Try to upload an image with the same image name again
7) Notice you are now warned about a duplicate image name.
8) Check that uploading an image with a unique name still works.

Sponsored-by: Catalyst IT
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Comment 3 Jonathan Druart 2017-01-12 12:19:45 UTC
Comment on attachment 58808 [details] [review]
Bug 17181: Check for duplicate image names when uploading image to patron card creator

Review of attachment 58808 [details] [review]:
-----------------------------------------------------------------

::: patroncards/image-manage.pl
@@ +49,5 @@
>  if ($op eq 'upload') {
> +    # Checking for duplicate image name
> +    my $duplicate;
> +    my $dbh = C4::Context->dbh;
> +    my $query = "SELECT image_name FROM creator_images";

What you want here is to select the number of images with the $image_name:
  SELECT COUNT(*) FROM creator_images WHERE image_name=?

That will simplify this part of code.
Comment 4 Aleisha Amohia 2017-01-12 23:30:37 UTC
Created attachment 58888 [details] [review]
Bug 17181: Check for duplicate image names when uploading image to patron card creator

This patch adds a check for duplicates before uploading the image.

To test:
1) Go to Tools -> Patron card creator -> Manage images
2) If you haven't already, upload an image
3) Try to upload another image with the same image name
4) Notice the first image is replaced with the second image, with no
warning.
5) Apply patch and refresh page
6) Try to upload an image with the same image name again
7) Notice you are now warned about a duplicate image name.
8) Check that uploading an image with a unique name still works.

Sponsored-by: Catalyst IT
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Comment 5 Claire Gravely 2017-01-13 08:39:49 UTC
Created attachment 58917 [details] [review]
Bug 17181: Check for duplicate image names when uploading image to patron card creator

This patch adds a check for duplicates before uploading the image.

To test:
1) Go to Tools -> Patron card creator -> Manage images
2) If you haven't already, upload an image
3) Try to upload another image with the same image name
4) Notice the first image is replaced with the second image, with no
warning.
5) Apply patch and refresh page
6) Try to upload an image with the same image name again
7) Notice you are now warned about a duplicate image name.
8) Check that uploading an image with a unique name still works.

Sponsored-by: Catalyst IT
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Comment 6 Jonathan Druart 2017-01-13 09:24:41 UTC
Created attachment 58921 [details] [review]
Bug 17181: Check for duplicate image names when uploading image to patron card creator

This patch adds a check for duplicates before uploading the image.

To test:
1) Go to Tools -> Patron card creator -> Manage images
2) If you haven't already, upload an image
3) Try to upload another image with the same image name
4) Notice the first image is replaced with the second image, with no
warning.
5) Apply patch and refresh page
6) Try to upload an image with the same image name again
7) Notice you are now warned about a duplicate image name.
8) Check that uploading an image with a unique name still works.

Sponsored-by: Catalyst IT
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2017-01-13 09:24:45 UTC
Created attachment 58922 [details] [review]
Bug 17181: Simplify code

- do not prepare statement when used only once
- remove use of Data::Dumper
- remove $duplicate, not needed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Kyle M Hall 2017-01-13 12:28:45 UTC
Pushed to master for 17.05, thanks Aleisha, Jonathan!
Comment 9 Katrin Fischer 2017-01-15 18:38:11 UTC
This won't get ported back to 16.11.x as it is an enhancement.
Comment 10 Marc Véron 2017-06-14 08:42:25 UTC
*** Bug 15000 has been marked as a duplicate of this bug. ***
Comment 11 Marc Véron 2017-06-14 08:44:41 UTC
(In reply to Katrin Fischer from comment #9)
> This won't get ported back to 16.11.x as it is an enhancement.

Hi Katrin, can you reconsider, since users can run into a data loss by overwriting images?
Marc
Comment 12 Katrin Fischer 2017-06-28 05:49:27 UTC
These patches have been pushed to 16.11.x and will be in 16.11.10.
Comment 13 Mason James 2017-07-31 11:31:38 UTC
Pushed to 16.05.x, for 16.05.15 release