Bug 12134 - Members are restricted even if the debarment is ended
Summary: Members are restricted even if the debarment is ended
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 2720
Blocks: 12292 13107 13108
  Show dependency treegraph
 
Reported: 2014-04-24 15:21 UTC by Jonathan Druart
Modified: 2019-06-27 09:24 UTC (History)
7 users (show)

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


Attachments
Bug 12134 - Members are restricted even if the debarment is ended (2.34 KB, patch)
2014-04-24 15:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12134 - Members are restricted even if the debarment is ended (2.41 KB, patch)
2014-04-24 15:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12134: Add unit tests to reflect the change in IsDebarred (1.18 KB, patch)
2014-04-24 15:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 12134 - Members are restricted even if the debarment is ended (2.48 KB, patch)
2014-04-25 08:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 12134: Add unit tests to reflect the change in IsDebarred (1.25 KB, patch)
2014-04-25 08:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12134: (follow-up) Add unit tests to reflect the change in IsDebarred (1.42 KB, patch)
2014-04-25 10:49 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 Jonathan Druart 2014-04-24 15:21:59 UTC
Introduced by bug 2720.
Comment 1 Kyle M Hall 2014-04-24 15:24:25 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-04-24 15:38:40 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2014-04-24 15:38:45 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-04-24 15:42:15 UTC
Good candidate for 3.14.6!
Comment 5 Martin Renvoize 2014-04-25 08:54:10 UTC
Created attachment 27581 [details] [review]
[PASSED QA] Bug 12134 - Members are restricted even if the debarment is ended

Test Plan:
1) Modify borrowers.debarred for a borrower and set the date in the past
2) Try to check out an item to that patron
3) Note you cannot
4) Apply this patch
5) Repeat step 2
6) Note you can now check out items the patron

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2014-04-25 08:54:41 UTC
Created attachment 27582 [details] [review]
[PASSED QA] Bug 12134: Add unit tests to reflect the change in IsDebarred

Before Kyle's patch, the test failed.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2014-04-25 08:55:30 UTC
Passes tests, code makes sense and fixes an important bug.
Comment 8 Jonathan Druart 2014-04-25 10:49:34 UTC
Created attachment 27607 [details] [review]
Bug 12134: (follow-up) Add unit tests to reflect the change in IsDebarred

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 9 Galen Charlton 2014-04-28 17:15:20 UTC
Pushed to master, along with a follow-up that repairs one of the new tests.  Thanks, Jonathan and Kyle!

I think serious thought should be given to removing borrowers.debarred, by the way, in favor of consulting borrower_debarments directly.  borrowers.debarred was retained as a performance measure, but I'd like to see proof that querying borrower_debarments is too slow to be worth the risk of bugs like this one.
Comment 10 Fridolin Somers 2014-04-30 14:28:49 UTC
Patches pushed to 3.14.x, will be in 3.14.06