Bug 26307

We tested old script j2a.pl and seems this problem already existed.

Created attachment 109229 [details] [review]
Bug 26307: Reveal problem in unit test for too_old

Logic :
=======

Looking at Koha::Patron->get_age :
At birthday aniversary, patron has aged 1 year more.
For example :
 - today is 2020-08-27
 - patron with date of birth 2000-08-28 is 19 year old
 - patron with date of birth 2000-08-27 is 20 year old

Looking at Koha::Patron->is_valid_age :
Category min and max limits are inclusive.
For example max=17 means :
 - patron having 17 years old is valid
 - patron having 18 years old is not valid

Test suite :
===========

Changes child3 to have 18 years tomorrow.
Adds a child4 with 18 years today.

Test plan :
Run prove t/db_dependent/Patrons.t
See it fails on 'One under age patron in child category'

Created attachment 109230 [details] [review]
Bug 26307: Fix misc/cronjobs/update_patrons_category.pl --too_old acts a year too early

misc/cronjobs/update_patrons_category.pl --too_old must change category when the age of the patron is greater than (exclusive) limit age of the category.
Actually it changes a year too early : he goes to a min 18 years category having reached 17 years.

Patch also changes the compare to 'lower and equal' and uses date
without time
=> so that update occures at birthday aniversary day

See logic in previous patch.
And code in Koha::Patron : get_age() and is_valid_age.

Test plan :
1) Create a child patron category 'C' with upper limit 17 years
2) Create an adult patron category 'A'
3) Create a patron in child category with date of birth = today - 18
   years + 1 day
4) Check patron as age : 17 years 11 months
5) Without patch run :
misc/cronjobs/update_patrons_category.pl -f=ENF -t=ADULT --too_old -v
=> You see the patron
6) apply patch
7) Re-run 5)
=> You dont see the patron
8) Update database to change patron date of birth = today - 18 years
mysql > update borrowers set dateofbirth='YYYY-MM-DD' where borrowernumber=XXX;
9) Run :
misc/cronjobs/update_patrons_category.pl -f=ENF -t=ADULT --too_old -v --confirm
10) Check patron category is now 'A'
11) Check you dont see message "Patron's age is incorrect for their category"

Created attachment 109231 [details] [review]
Bug 26307: Add a unit test for too_young

In unit test for update patron categories :

Changes $child1 to bee 4 years 11 month old => is under age
Add $child1bis having exactly the lower age limit of 5 years => is not
under age

Run t/db_dependent/Patrons.t
Test is to renforce the test suite, it is OK event without next patch

Created attachment 109232 [details] [review]
Bug 26307: Use same syntax in --too_young

In too_young use dt->strftime("%Y-%m-%d") like in too_old.
This does not change the behavior.

Test plan :
Run prove t/db_dependent/Patrons.t

I don't think this is a bug, or rather, I don't know

I see your point, if you have categories:
Child Upper Age Limit 17
Adult Minimum Age 18

You don't cross until you are 18, however, you can also read that there is a gap between 17 and 18

The way Koha has always worked you have:
Child Upper Age Limit 18
Adult Minimum Age 18

Now there is no gap and you switch at 18

I think changing a long standing behaviour will affect many people, and that makes me nervous. I know that I have helped libraries set this up in the way that works.

I think instead we need to clarify the explanation of the 'Limit'

(In reply to Nick Clemens from comment #6)
> I don't think this is a bug, or rather, I don't know
> 
> I see your point, if you have categories:
> Child Upper Age Limit 17
> Adult Minimum Age 18
> 
> You don't cross until you are 18, however, you can also read that there is a
> gap between 17 and 18
> 
> The way Koha has always worked you have:
> Child Upper Age Limit 18
> Adult Minimum Age 18
> 
> Now there is no gap and you switch at 18
> 
> I think changing a long standing behaviour will affect many people, and that
> makes me nervous. I know that I have helped libraries set this up in the way
> that works.
> 
> I think instead we need to clarify the explanation of the 'Limit'

OK this is the other solution ;)

In your example, a 18 years old patron can be in Child or Adult category without breaking the limits ?

For you limits are inclusive right ?

(In reply to Fridolin SOMERS from comment #7)
> (In reply to Nick Clemens from comment #6)
> > I don't think this is a bug, or rather, I don't know
> > 
> > I see your point, if you have categories:
> > Child Upper Age Limit 17
> > Adult Minimum Age 18
> > 
> > You don't cross until you are 18, however, you can also read that there is a
> > gap between 17 and 18
> > 
> > The way Koha has always worked you have:
> > Child Upper Age Limit 18
> > Adult Minimum Age 18
> > 
> > Now there is no gap and you switch at 18
> > 
> > I think changing a long standing behaviour will affect many people, and that
> > makes me nervous. I know that I have helped libraries set this up in the way
> > that works.
> > 
> > I think instead we need to clarify the explanation of the 'Limit'
> 
> OK this is the other solution ;)
> 
> In your example, a 18 years old patron can be in Child or Adult category
> without breaking the limits ?
> 
> For you limits are inclusive right ?

I think the lower limit is inclusive, the upper is exclusive:
At least X years old
to 
Less than X years old

However, in testing, it was found that the scripts work on way, and the interface another.

Try to enter a patron into a category from 5-17. At 17 and 1 day they can still be entered, at 18 they are prohibited.

My colleges tell me that limits are inclusive and on purpose one must have one year in common between child and adult.

For example :
Child Upper Age Limit 18
Adult Minimum Age 18

So a patron having 18 is OK in both categories.
This makes sens if you manually switch patron category.

If you use update_patrons_category.pl patron will switch from child to adult the day he turns 18.
So he is not really "too old" for the category : he is at the upper limit not over it.

If this is the expected behaviour there is no bug ^^

After thinking about it for a bit, I think the current behaviour makes sense. If CHILD has an upper limit of 18, it means the patron should change category on the day she turns 18. And ADULT has a lower limit of 18, because the day the patron turns 18 they should be moved to this category.

Ok but still I'm going to work on script to avoid changing category if age is not valid in new category.

Forget about that too much work.
Since we have Bug 26311 its OK.