Bug 18139 - 'Too many checked out' can confuse librarians
Summary: 'Too many checked out' can confuse librarians
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 35776
  Show dependency treegraph
 
Reported: 2017-02-17 15:06 UTC by Kyle M Hall
Modified: 2024-01-16 15:18 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.02


Attachments
Bug 18139 - Prep unit tests (17.31 KB, patch)
2017-02-17 15:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18139 - 'Too many checked out' can confuse librarians (6.64 KB, patch)
2017-02-17 15:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18139 - Add additional unit tests (11.94 KB, patch)
2017-02-17 15:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18139 - Prep unit tests (17.36 KB, patch)
2017-03-22 18:09 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 18139 - 'Too many checked out' can confuse librarians (6.70 KB, patch)
2017-03-22 18:09 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 18139 - Add additional unit tests (11.99 KB, patch)
2017-03-22 18:09 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 18139 - Prep unit tests (17.50 KB, patch)
2017-03-23 14:40 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18139 - 'Too many checked out' can confuse librarians (6.83 KB, patch)
2017-03-23 14:41 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18139 - Add additional unit tests (12.30 KB, patch)
2017-03-23 14:44 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18139: Adapt tests (8.55 KB, patch)
2017-03-28 15:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18139: Display the rule critera used to determin the checkout limits to the template (9.15 KB, patch)
2023-11-28 17:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18139: Add/update unit tests (23.36 KB, patch)
2023-11-28 17:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18139: Display the rule critera used to determin the checkout limits to the template (9.21 KB, patch)
2023-12-04 14:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 18139: Add/update unit tests (23.42 KB, patch)
2023-12-04 14:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 18139: Change 'All libraries' to 'All' for consistency (932 bytes, patch)
2023-12-12 12:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18139: Tidy code (5.57 KB, patch)
2023-12-12 12:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18139: Specify 'all item types' and 'all patron categories' in template (1.21 KB, patch)
2023-12-12 13:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18139: Display the rule critera used to determin the checkout limits to the template (9.26 KB, patch)
2023-12-15 20:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18139: Add/update unit tests (23.47 KB, patch)
2023-12-15 20:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18139: Tidy code (5.62 KB, patch)
2023-12-15 20:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18139: Specify 'all item types' and 'all patron categories' in template (1.27 KB, patch)
2023-12-15 20:09 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 Kyle M Hall 2017-02-17 15:06:29 UTC
The too many checked out message can be very confusing for librarians when the limit specified is more or less than the number of checkouts the patron already has. We should make it clear to the librarian that this is only counting certain types of checkouts by giving them the issuing rule criteria used.
Comment 1 Kyle M Hall 2017-02-17 15:23:56 UTC
Created attachment 60427 [details] [review]
Bug 18139 - Prep unit tests
Comment 2 Kyle M Hall 2017-02-17 15:24:03 UTC
Created attachment 60428 [details] [review]
Bug 18139 - 'Too many checked out' can confuse librarians

The too many checked out message can be very confusing for librarians
when the limit specified is more or less than the number of checkouts
the patron already has. We should make it clear to the librarian that
his is only counting certain types of checkouts by giving them the
issuing rule criteria used.

Test Plan:
1) Apply this patch
2) Try to check out more items than the circ rules allow for
   regular and on-site checkouts
Comment 3 Kyle M Hall 2017-02-17 15:24:06 UTC
Created attachment 60429 [details] [review]
Bug 18139 - Add additional unit tests
Comment 4 Marc Véron 2017-02-18 15:00:06 UTC
Hi Kyle,
I like this feature very much.

However, I get an Internal Server Error while systematically testing. It occurs with following settings:

- Delete all rules in 'Circulation rules and fines'.
- Set a Default limit by patron category, e.g. Student 2 current checkouts , 2 on-site-chekouts

Checkout to a student until the limit is reached. 

Result: 
Internal server error
Plack error log says:
Template process failed: undef error - Can't call method "description" on an undefined value at (...)/Koha/Template/Plugin/Categories.pm line 32.

Without patch, it works fine, the message is triggered.

BTW: I can always check out one item more than defined by the rules. E.g. I allow 2 checkouts (by any rule), the message triggers with the 4th checkout. I would expect it to trigger with the 3rd checkout. This happens with and without patch.
Comment 5 Biblibre Sandboxes 2017-03-22 18:09:13 UTC
Patch tested with a sandbox, by Eric Gosselin <eric.gosselin@inlibro.com>
Comment 6 Biblibre Sandboxes 2017-03-22 18:09:50 UTC
Created attachment 61489 [details] [review]
Bug 18139 - Prep unit tests

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>
Comment 7 Biblibre Sandboxes 2017-03-22 18:09:53 UTC
Created attachment 61490 [details] [review]
Bug 18139 - 'Too many checked out' can confuse librarians

The too many checked out message can be very confusing for librarians
when the limit specified is more or less than the number of checkouts
the patron already has. We should make it clear to the librarian that
his is only counting certain types of checkouts by giving them the
issuing rule criteria used.

Test Plan:
1) Apply this patch
2) Try to check out more items than the circ rules allow for
   regular and on-site checkouts

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>
Comment 8 Biblibre Sandboxes 2017-03-22 18:09:56 UTC
Created attachment 61491 [details] [review]
Bug 18139 - Add additional unit tests

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>
Comment 9 Jonathan Druart 2017-03-22 19:09:04 UTC
(In reply to Marc Véron from comment #4)

Mark, I do not recreate. Could you double check please?
Comment 10 Marc Véron 2017-03-23 14:35:56 UTC
(In reply to Jonathan Druart from comment #9)
> (In reply to Marc Véron from comment #4)
> 
> Mark, I do not recreate. Could you double check please?

Hi Jonathan

I can no longer recreate the first part of comment #4 (internal server error)

However I can recreate the "BTW":
I can always check out one item more than defined by the rules. E.g. I allow 2 checkouts (by any rule), the message triggers with the 4th checkout. I would expect it to trigger with the 3rd checkout. This happens with and without patch.

With the 4th checkout, message says:
"Too many checked out. 3 checked out, only 2 are allowed."

Should that go to a separate Bug?

Marc
Comment 11 Marc Véron 2017-03-23 14:40:18 UTC
Created attachment 61541 [details] [review]
Bug 18139 - Prep unit tests

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 12 Marc Véron 2017-03-23 14:41:27 UTC
Created attachment 61542 [details] [review]
Bug 18139 - 'Too many checked out' can confuse librarians

The too many checked out message can be very confusing for librarians
when the limit specified is more or less than the number of checkouts
the patron already has. We should make it clear to the librarian that
his is only counting certain types of checkouts by giving them the
issuing rule criteria used.

Test Plan:
1) Apply this patch
2) Try to check out more items than the circ rules allow for
   regular and on-site checkouts

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 13 Marc Véron 2017-03-23 14:44:14 UTC
Created attachment 61543 [details] [review]
Bug 18139 - Add additional unit tests

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>

Tested al patches together, works OK, can no longer reprocuce
internal server error from comment 4.
Regarding "Too meny count" (see BTW in comment #4) I will file a separate bug.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 14 Jonathan Druart 2017-03-23 14:52:23 UTC
(In reply to Marc Véron from comment #10)
> However I can recreate the "BTW":
> I can always check out one item more than defined by the rules. E.g. I allow
> 2 checkouts (by any rule), the message triggers with the 4th checkout. I
> would expect it to trigger with the 3rd checkout. This happens with and
> without patch.
> 
> With the 4th checkout, message says:
> "Too many checked out. 3 checked out, only 2 are allowed."
> 
> Should that go to a separate Bug?

Yes please, with more information because I did not recreate either.
Comment 15 Marc Véron 2017-03-23 15:11:08 UTC
(In reply to Jonathan Druart from comment #14)
> (In reply to Marc Véron from comment #10)
> > However I can recreate the "BTW":
> > I can always check out one item more than defined by the rules. E.g. I allow
> > 2 checkouts (by any rule), the message triggers with the 4th checkout. I
> > would expect it to trigger with the 3rd checkout. This happens with and
> > without patch.
> > 
> > With the 4th checkout, message says:
> > "Too many checked out. 3 checked out, only 2 are allowed."
> > 
> > Should that go to a separate Bug?
> 
> Yes please, with more information because I did not recreate either.

See Bug 18321  :-)
Comment 16 Jonathan Druart 2017-03-28 15:46:18 UTC
Created attachment 61663 [details] [review]
Bug 18139: Adapt tests
Comment 17 Jonathan Druart 2017-03-28 15:48:31 UTC
Kyle, I think the way you changed the tests is wrong.
We need to deeply check if the issuing rule is correct instead of only check if the key exists.

Take a look at these tests, they almost all passed, unless if an issuing rule does not exist but one exists at patron category level (BranchBorrowerCircRule).
I think it highlights an issue with your changes.
Comment 18 Kyle M Hall 2023-11-28 17:29:07 UTC
Created attachment 159307 [details] [review]
Bug 18139: Display the rule critera used to determin the checkout limits to the template

The too many checked out message can be very confusing for librarians
when the limit specified is more or less than the number of checkouts
the patron already has. We should make it clear to the librarian that
his is only counting certain types of checkouts by giving them the
issuing rule criteria used.

Test Plan:
1) Apply this patch
2) Try to check out more items than the circ rules allow for
   regular and on-site checkouts

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 19 Kyle M Hall 2023-11-28 17:29:14 UTC
Created attachment 159308 [details] [review]
Bug 18139: Add/update unit tests

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 20 Martin Renvoize 2023-12-04 14:10:33 UTC
Created attachment 159522 [details] [review]
Bug 18139: Display the rule critera used to determin the checkout limits to the template

The too many checked out message can be very confusing for librarians
when the limit specified is more or less than the number of checkouts
the patron already has. We should make it clear to the librarian that
his is only counting certain types of checkouts by giving them the
issuing rule criteria used.

Test Plan:
1) Apply this patch
2) Try to check out more items than the circ rules allow for
   regular and on-site checkouts

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 21 Martin Renvoize 2023-12-04 14:10:36 UTC
Created attachment 159523 [details] [review]
Bug 18139: Add/update unit tests

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize 2023-12-04 14:11:22 UTC
All working here.. minor perltidy issues highlighted, but on inspection, I can't work out how to resolve them.

Passing QA
Comment 23 Katrin Fischer 2023-12-12 11:28:51 UTC
1) Please tidy!

 WARN	C4/Circulation.pm
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 1093, now: 1098)

 OK	koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt

 WARN	t/db_dependent/Circulation/TooMany.t
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 172, now: 178)

2) We have "All libraries" in the include, but "All" for categories and itemtypes. Can we make this consistent?

Sorry, running out of time today for adjusting myself!
Comment 24 Kyle M Hall 2023-12-12 12:04:44 UTC
Created attachment 159728 [details] [review]
Bug 18139: Change 'All libraries' to 'All' for consistency
Comment 25 Kyle M Hall 2023-12-12 12:04:54 UTC
Created attachment 159729 [details] [review]
Bug 18139: Tidy code
Comment 26 Katrin Fischer 2023-12-12 13:45:47 UTC
(In reply to Kyle M Hall from comment #24)
> Created attachment 159728 [details] [review] [review]
> Bug 18139: Change 'All libraries' to 'All' for consistency

Argh, I am sorry, I was not clear. The "All" is causing a bit of a translation headache, I meant to make them all specific. 

See bug 31735 for context.

"In French, those different things have different genders and the "All" must be in that gender ("tous" for "male-gendered" things and "toutes" for "female-gendered" things)."
Comment 27 Kyle M Hall 2023-12-12 13:49:30 UTC
Created attachment 159740 [details] [review]
Bug 18139: Specify 'all item types' and 'all patron categories' in template
Comment 28 Kyle M Hall 2023-12-12 13:50:11 UTC
(In reply to Katrin Fischer from comment #26)
> (In reply to Kyle M Hall from comment #24)
> > Created attachment 159728 [details] [review] [review] [review]
> > Bug 18139: Change 'All libraries' to 'All' for consistency
> 
> Argh, I am sorry, I was not clear. The "All" is causing a bit of a
> translation headache, I meant to make them all specific. 
> 
> See bug 31735 for context.
> 
> "In French, those different things have different genders and the "All" must
> be in that gender ("tous" for "male-gendered" things and "toutes" for
> "female-gendered" things)."

Np! New patch fixes that!
Comment 29 Tomás Cohen Arazi 2023-12-15 20:09:18 UTC
Created attachment 159904 [details] [review]
Bug 18139: Display the rule critera used to determin the checkout limits to the template

The too many checked out message can be very confusing for librarians
when the limit specified is more or less than the number of checkouts
the patron already has. We should make it clear to the librarian that
his is only counting certain types of checkouts by giving them the
issuing rule criteria used.

Test Plan:
1) Apply this patch
2) Try to check out more items than the circ rules allow for
   regular and on-site checkouts

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 30 Tomás Cohen Arazi 2023-12-15 20:09:23 UTC
Created attachment 159905 [details] [review]
Bug 18139: Add/update unit tests

Signed-off-by: Eric Gosselin <eric.gosselin@inlibro.com>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 31 Tomás Cohen Arazi 2023-12-15 20:09:27 UTC
Created attachment 159906 [details] [review]
Bug 18139: Tidy code

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 32 Tomás Cohen Arazi 2023-12-15 20:09:32 UTC
Created attachment 159907 [details] [review]
Bug 18139: Specify 'all item types' and 'all patron categories' in template

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 33 Katrin Fischer 2023-12-18 14:25:14 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 34 Fridolin Somers 2024-01-03 14:18:09 UTC
Pushed to 23.11.x for 23.11.02
Comment 35 Lucas Gass 2024-01-04 20:08:28 UTC
Doesn't apply cleanly to 23.05.x, will not backport.