Bug 34146 - Add confirmation question when more than 99 items are to be added
Summary: Add confirmation question when more than 99 items are to be added
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Marcel de Rooy
QA Contact: Martin Renvoize
URL:
Keywords:
: 20932 (view as bug list)
Depends on:
Blocks:
 
Reported: 2023-06-28 14:03 UTC by victor
Modified: 2023-12-28 20:47 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08


Attachments
Bug 34146: Do not allow multiple copies to crash server (2.78 KB, patch)
2023-06-28 14:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34146: Do not allow multiple copies to crash server (2.78 KB, patch)
2023-06-28 14:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34146: Add a client-side check with maxlength too (1.61 KB, patch)
2023-06-29 05:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34146: Counterpart for serials-edit (3.97 KB, patch)
2023-06-29 06:23 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34146: Do not allow multiple copies to crash server (2.79 KB, patch)
2023-06-29 06:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34146: Add a client-side check with maxlength too (1.61 KB, patch)
2023-06-29 06:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34146: Counterpart for serials-edit (3.89 KB, patch)
2023-06-29 06:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34146: Do not allow multiple copies to crash server (2.81 KB, patch)
2023-06-29 07:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34146: Add a client-side check with maxlength too (1.63 KB, patch)
2023-06-29 07:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34146: Counterpart for serials-edit (3.92 KB, patch)
2023-06-29 07:00 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description victor 2023-06-28 14:03:02 UTC
In Koha it is possible to Add an instance or choose to "Add Multiple Copies" simultaneously to a record.
So far so good. More..... I had a situation where an employee mistakenly typed 800,000 copies for a record.
As a result, when Koha started its automatic Indexing routine, it consumed all of the server's memory and crashed system access. When I tried to delete the 800,000 copies in batch, the system crashed. It took slow deletion from 5,000 to 5,000 to reach 800,000.
Considering the aforementioned case, would any colleague know if we can limit the quantity in the option "Add Multiple Copies"?
If there is no option, I believe that a configuration option to limit the entry of copies is important.
Thanks,
Victor
Comment 1 Marcel de Rooy 2023-06-28 14:05:26 UTC
Thanks Victor for reporting this one.
Moving this from enh to bug.
Starting with normal, but we could raise severity still.
Comment 2 Marcel de Rooy 2023-06-28 14:17:24 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2023-06-28 14:17:51 UTC
Trivial fix, self signoff.
Comment 4 Marcel de Rooy 2023-06-28 14:18:13 UTC
Raising to major
Comment 5 Marcel de Rooy 2023-06-28 14:19:21 UTC
Created attachment 152808 [details] [review]
Bug 34146: Do not allow multiple copies to crash server

Currently hardcoded to 1000.
Can be refined later. Let's first prevent this kind of accidents.

Test plan:
Add additem.pl. Edit the 1000 to 2. Restart all.
Add 3 multiple copies. Notice that you got 2.
Revert your code change.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2023-06-28 14:40:38 UTC
Someone removed this btw in js:

+    <!-- Add a soft-limit of 99 with a reminder about potential data entry error -->
+    if (f>99) {
+        return confirm(_("More than 99 copies will be added. Should they be added?"));
Comment 7 Marcel de Rooy 2023-06-28 14:57:11 UTC
It seems that Bootstrap does not respect the size attribute that is present on the number_of_copies input. There is a size="2" but it does not work.
Comment 8 Owen Leonard 2023-06-28 19:42:06 UTC
(In reply to Marcel de Rooy from comment #7)
> There is a size="2" but it does not work.

It sounds like you may be thinking of the "maxlength" attribute? Size only controls the width, not the values it accepts.
Comment 9 Marcel de Rooy 2023-06-29 05:56:21 UTC
(In reply to Owen Leonard from comment #8)
> (In reply to Marcel de Rooy from comment #7)
> > There is a size="2" but it does not work.
> 
> It sounds like you may be thinking of the "maxlength" attribute? Size only
> controls the width, not the values it accepts.

Good point. Will add maxlength.
Was trying to deduct why the check was removed. Note that size for a input text has a vague connection to the number of chars according to html specs.

This attribute tells the user agent the initial width of the control. The width is given in pixels except when type attribute has the value "text" or "password". In that case, its value refers to the (integer) number of characters.
END QUOTE

Note that it also depends on font etc. So not a good choice. Bootstrap offers other options for width too.
Comment 10 Marcel de Rooy 2023-06-29 05:59:27 UTC
Created attachment 152843 [details] [review]
Bug 34146: Add a client-side check with maxlength too

Test plan:
Try to add more than 3 characters now in the number of copies.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2023-06-29 06:23:02 UTC
Created attachment 152844 [details] [review]
Bug 34146: Counterpart for serials-edit

Not only additem suffers from it. We can do the same with serials-edit.
This patch adds a server-side and client-side check as we did for additem.

Test plan:
Receive serial with adding items.
Try to add more than 999 items in number of copies.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2023-06-29 06:26:12 UTC
Created attachment 152845 [details] [review]
Bug 34146: Do not allow multiple copies to crash server

Currently hardcoded to 1000.
Can be refined later. Let's first prevent this kind of accidents.

Test plan:
Add additem.pl. Edit the 1000 to 2. Restart all.
Add 3 multiple copies. Notice that you got 2.
Revert your code change.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2023-06-29 06:26:15 UTC
Created attachment 152846 [details] [review]
Bug 34146: Add a client-side check with maxlength too

Test plan:
Try to add more than 3 characters now in the number of copies.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2023-06-29 06:26:17 UTC
Created attachment 152847 [details] [review]
Bug 34146: Counterpart for serials-edit

Not only additem suffers from it. We can do the same with serials-edit.
This patch adds a server-side and client-side check as we did for additem.

Test plan:
Receive serial with adding items.
Try to add more than 999 items in number of copies.
Comment 15 Marcel de Rooy 2023-06-29 06:26:50 UTC
Fixed warning on first patch:

[2023/06/28 14:14:26] [WARN] CGI::param called in list context from /usr/share/koha/cataloguing/additem.pl line 241, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 414.
Comment 16 Martin Renvoize 2023-06-29 07:00:19 UTC
Created attachment 152849 [details] [review]
Bug 34146: Do not allow multiple copies to crash server

Currently hardcoded to 1000.
Can be refined later. Let's first prevent this kind of accidents.

Test plan:
Add additem.pl. Edit the 1000 to 2. Restart all.
Add 3 multiple copies. Notice that you got 2.
Revert your code change.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2023-06-29 07:00:22 UTC
Created attachment 152850 [details] [review]
Bug 34146: Add a client-side check with maxlength too

Test plan:
Try to add more than 3 characters now in the number of copies.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2023-06-29 07:00:25 UTC
Created attachment 152851 [details] [review]
Bug 34146: Counterpart for serials-edit

Not only additem suffers from it. We can do the same with serials-edit.
This patch adds a server-side and client-side check as we did for additem.

Test plan:
Receive serial with adding items.
Try to add more than 999 items in number of copies.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Tomás Cohen Arazi 2023-07-03 17:51:17 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 20 Martin Renvoize 2023-07-17 15:00:23 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 21 Matt Blenkinsop 2023-07-18 13:12:13 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 22 Katrin Fischer 2023-09-16 13:03:50 UTC
*** Bug 20932 has been marked as a duplicate of this bug. ***