Bug 8392 - Memberentry is not enforcing birthdate restrictions
Summary: Memberentry is not enforcing birthdate restrictions
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low minor (vote)
Assignee: D Ruth Bavousett
QA Contact: Jonathan Druart
URL:
Keywords:
: 8464 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-07-09 14:01 UTC by D Ruth Bavousett
Modified: 2013-12-05 19:57 UTC (History)
8 users (show)

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


Attachments
Proposed patch (1.47 KB, patch)
2012-07-09 14:21 UTC, D Ruth Bavousett
Details | Diff | Splinter Review
Signed-off patch (1.66 KB, patch)
2012-07-09 15:52 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 8392: Avoid having an untranslatable age range (1.91 KB, patch)
2012-07-09 15:53 UTC, Frédéric Demians
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8392: Avoid having an untranslatable age range (1.99 KB, patch)
2012-07-18 15:12 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description D Ruth Bavousett 2012-07-09 14:01:25 UTC
If dateofbirth is in BorrowerMandatoryFields, and a range of ages is set in the categories, when you enter a member, it's supposed to be checking to see if the patron's age is within that range, and complain if not.  Indeed, the code seems to be trying to do just that (memberentry.pl, lines 267-276, as of 3.8.2), but it's not actually being enforced.

It appears that the redefinition of @field_check for hiding some fields in the template (line 106) is the culprit.  I'll have a patch shortly.
Comment 1 D Ruth Bavousett 2012-07-09 14:21:46 UTC Comment hidden (obsolete)
Comment 2 D Ruth Bavousett 2012-07-09 14:29:29 UTC
This should apply to 3.8.x and Master.  The change to memberentry.pl that *caused* the bug did not enter until 3.8, so it won't be needed for 3.6.x.

-Ruth
Comment 3 Frédéric Demians 2012-07-09 15:52:08 UTC
Created attachment 10718 [details] [review]
Signed-off patch

I wait your sign-off on the following patch before marking this patch as signed-off.
Comment 4 Frédéric Demians 2012-07-09 15:53:01 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2012-07-11 03:57:55 UTC
*** Bug 3410 has been marked as a duplicate of this bug. ***
Comment 6 Adrien SAURAT 2012-07-18 13:27:49 UTC
*** Bug 8464 has been marked as a duplicate of this bug. ***
Comment 7 Owen Leonard 2012-07-18 15:12:07 UTC
Created attachment 10983 [details] [review]
[SIGNED-OFF] Bug 8392: Avoid having an untranslatable age range

[SIGNED-OFF] Bug 8392: Avoid having an untranslatable age range
    
    Currently, error message reports an age range in English. For example '0 to
    17'. With this patch, the 'to' is not in the .pl file anymore.
    
    Ruth could you sign-off this fix?
    
    Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Jonathan Druart 2012-07-23 12:32:18 UTC
QA Comments:

Humm, I think it would have been better to rename the second field_check variable. Thus the initialisation of $dateofbirthmandatory keeps close to it usage (l.~100 and l.~270).

But it works.
Marking as Passed QA
Comment 9 Paul Poulain 2012-07-24 15:26:37 UTC
The patch has been pushed, but I'm wondering if there is not another problem: why should the birthdate be mandatory to deal with age limits ?
Shouldn't we check AgeLimit anyway ?
Shouldn't we have a rule like "check AgeLimit if there is a birthdate, otherwise, it's OK" ?
Comment 10 Owen Leonard 2012-07-24 16:43:37 UTC
(In reply to comment #9)
> Shouldn't we have a rule like "check AgeLimit if there is a birthdate,
> otherwise, it's OK" ?

I wondered the same thing when I was testing it, but I think it makes sense. Using BorrowerMandatoryFields acts as a de-facto system preference for choosing whether or not the age limit should be enforced. If I want to enforce the age limit on a patron category I want to enforce it on all patrons, not just patrons for whom I have entered a birth date.
Comment 11 Paul Poulain 2012-07-24 16:52:03 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > Shouldn't we have a rule like "check AgeLimit if there is a birthdate,
> > otherwise, it's OK" ?
> 
> I wondered the same thing when I was testing it, but I think it makes sense.
> Using BorrowerMandatoryFields acts as a de-facto system preference for
> choosing whether or not the age limit should be enforced. If I want to
> enforce the age limit on a patron category I want to enforce it on all
> patrons, not just patrons for whom I have entered a birth date.

It's a change in the behaviour I was not aware. I think it has been introduced in 3.6. I don't see anything about that in the docs: http://manual.koha-community.org/3.8/en/patscirc.html#patcats

Adding Nicole.

Nicole, please read comment 9 = age limit is checked only if birthdate is a borrower mandatory field !
Comment 12 Chris Cormack 2012-07-25 08:57:55 UTC
Pushed to 3.8.x will be in 3.8.4
Comment 13 Nicole C. Engard 2012-08-06 19:31:52 UTC
Manual updated