Bug 10473 - Prompt for confirmation if more than 99 items are being added
Summary: Prompt for confirmation if more than 99 items are being added
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P3 normal (vote)
Assignee: Mark Tompsett
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-15 11:34 UTC by Amit Gupta
Modified: 2015-06-04 23:33 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:


Attachments
Max length should be 2 digit for adding multiple copies in add items page. (1.77 KB, patch)
2013-06-15 11:44 UTC, Amit Gupta
Details | Diff | Splinter Review
Bug 10473 - Max length should be 2 digit for adding multiple copies in add items page (2.66 KB, patch)
2014-09-18 16:21 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 10473 - Max length should be 2 digit for adding multiple copies in add items page (2.72 KB, patch)
2014-09-19 22:42 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 10473 - Max length should be 2 digit for adding multiple copies in add items page (2.78 KB, patch)
2014-09-26 08:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10473: Get rif of the placeholder (1.58 KB, patch)
2014-09-26 08:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10473: (RM followup) small wording change (1.30 KB, patch)
2014-11-14 16:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Amit Gupta 2013-06-15 11:34:45 UTC
I have recently encountered this case in Mysore University. Someone tried to add multiple copies around 50,000. 

So in this case max digit allow should 2 digit length.
Comment 1 Amit Gupta 2013-06-15 11:44:23 UTC Comment hidden (obsolete)
Comment 2 Magnus Enger 2013-06-17 21:58:03 UTC
I disagree with this patch, since I know libraries that need to add more than 99 items at a time. In fact, the library that sponsored the "multiple items" feature in the first place sometimes need to add more than 99 items at a time. 

If the functionality to set a max number of items is desired, I think it should be made a syspref, so it can be more flexible.
Comment 3 Robin Sheat 2013-06-17 22:04:00 UTC
Another option might be adding a confirmation step if more than 99 are to be added. I imagine that this is an unusual but not unknown case, so adding a single click to "You're adding 31,416 items, please confirm this is correct" might be useful. It wouldn't significantly slow down normal work, but does add a bit of validation.
Comment 4 Amit Gupta 2013-06-18 02:14:48 UTC
Hi Magnus Enger,

I think system preference is a right option. User can give the max digit value according to there requirement. Can you please tell me what is the default max digit value? so i can recreate the patch.
Comment 5 Robin Sheat 2013-06-18 02:36:50 UTC
IMO, a syspref is a bad idea for a few reasons. 

1) Now you're removing flexibility and making things more awkward for that time when you do need to exceed a pre-defined limit,
2) You're attempting to have people make decisions on the sorts of errors that other people are likely to make. They have no reference on this, so won't really be able to do anything meaningful with it,
3) We already have lots of system preferences and adding a new one for something that doesn't really do anything (except interfere, as in #1) seems like the wrong approach.

Validate by all means, but make it a soft validation. There's no point having a hard limit. We're just trying to stop typos really.
Comment 6 Liz Rea 2013-06-18 06:26:30 UTC
I agree with Robin on this - validate the field, don't force people to set hard limit.

Liz
Comment 7 Magnus Enger 2013-06-18 06:59:00 UTC
(In reply to comment #5)
> Validate by all means, but make it a soft validation. There's no point
> having a hard limit. We're just trying to stop typos really.

I agree with Robin too!
Comment 8 Mark Tompsett 2014-03-16 17:15:32 UTC
I was thinking.

Use the biblio with the most items multipled by 10 as an upper limit for items to be added? If a library has 1 item per biblio for everything in their library, what are the odds they will suddenly stock 10 items of the same biblio?

You could use a quick check of the number of records in biblioitems as an upper limit. No one is going to double the size of their library in one step, except at the beginning of data entry.
Comment 9 Galen Charlton 2014-04-14 19:40:41 UTC
(In reply to M. Tompsett from comment #8)
> You could use a quick check of the number of records in biblioitems as an
> upper limit. No one is going to double the size of their library in one
> step, except at the beginning of data entry.

... until we run into a user that wants to create a few thousand items for their photography collection or something.

It could happen!

I agree with Robin.  A hard-coded soft limit to catch typos is fine; IMO we'd need to see a lot more evidence that this is a problem that folks run into often before we get any fancier.

I also don't think it is a major bug, so I'm resetting the severity accordingly.
Comment 10 Mark Tompsett 2014-09-18 16:21:14 UTC Comment hidden (obsolete)
Comment 11 PTFS Europe Sandboxes 2014-09-19 22:41:47 UTC
Patch tested with a sandbox, by Nick Clemens <nick@quecheelibrary.org>
Comment 12 PTFS Europe Sandboxes 2014-09-19 22:42:05 UTC Comment hidden (obsolete)
Comment 13 Nick Clemens 2014-09-19 22:44:51 UTC
Just adding the comment that I, personally, don't like the placeholder '3' and find it confusing for the general user.  I don't think it fails for that though.

I also didn't run QA tools as was on sandbox
Comment 14 Mark Tompsett 2014-09-20 01:14:52 UTC
I'm willing to remove the placeholder, if you prefer, Nick. I just thought by putting a single digit number, it might help unconsciously lead people to type smaller, more valid numbers. :)
Comment 15 Nick Clemens 2014-09-20 13:50:26 UTC
Fair enough, I was just thinking of the questions I anticipated from librarians in the system about why they were seeing a number there.  It seemed unusual to me for Koha to suggest a value in a field, I almost wondered if it was a new default of 3 at first glance.

I think it could go either way, my opinion is definitely not always the common one, or even the correct one :)  Leave it for QA to weigh in?
Comment 16 Jonathan Druart 2014-09-26 07:56:49 UTC
I am not fond of the placeholder neither.

Magnus, Robin, Liz and Galen, can we have your point of view on this new patch?
IMO it is a good compromise: it only alerts the user that he entered a huge number.
Comment 17 Magnus Enger 2014-09-26 08:01:45 UTC
(In reply to Jonathan Druart from comment #16)
> I am not fond of the placeholder neither.
> 
> Magnus, Robin, Liz and Galen, can we have your point of view on this new
> patch?
> IMO it is a good compromise: it only alerts the user that he entered a huge
> number.

THe warning/alert sounds good to me. I'd vote to get rid of the placeholder.
Comment 18 Jonathan Druart 2014-09-26 08:12:08 UTC
Created attachment 31889 [details] [review]
Bug 10473 - Max length should be 2 digit for adding multiple copies in add items page

As per the discussion, a prompt on a hard coded soft-limit is
far more acceptable as a solution.

TEST PLAN
---------
 0) Back up your DB. -- because a backup is always good!
 1) Log in to staff client
 2) Navigate to any biblio details
    (e.g. cgi-bin/koha/catalogue/detail.pl?biblionumber=#####)
 3) Click the 'Edit' dropdown button.
 4) Click 'Edit items'.
 5) Click 'Add multiple items'
 6) Enter a crazy high number (e.g. 999)
 7) Click 'Add'
    -- Koha just adds it! YIKES!
 8) Apply patch
 9) Repeat steps 5-7
10) Click 'Cancel'
    -- Koha does not add the items.
11) Repeat steps 5-7
12) Click 'Ok'
    -- Koha does add the items.
13) run koha QA test tools
14) Restore your DB.

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 19 Jonathan Druart 2014-09-26 08:12:15 UTC
Created attachment 31890 [details] [review]
Bug 10473: Get rif of the placeholder

This placeholder is not really relevant here.
Comment 20 Jonathan Druart 2014-09-26 08:13:48 UTC
I let the RM to decide about the placeholder.
I think the commit message should be rewrite too.

Passed QA.
Comment 21 Tomás Cohen Arazi 2014-11-14 16:15:43 UTC
Created attachment 33577 [details] [review]
Bug 10473: (RM followup) small wording change

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 22 Tomás Cohen Arazi 2014-11-14 16:17:11 UTC
Patches pushed to master.

Thanks Mark!