It seems that Bug 17557 reintroduced problem with invalid birth dates fixed by Bug 17375.
Our current problem are patrons without date of birth, which isn't covered by tests
Created attachment 59172 [details] [review] Bug 17933 - Internal software error when searching patron without birth date When patrons don't have date of birth (which is not required) patron search results on moremember page produce internal server error since we can't convert MySQL invalid date 0000-00-00 to datetime object and call strfdate on it. Additionally, since we assign dates to template variables and after than assign whole $data hash to template, later assigment overrides previous one, so we see birth date field even for patrons which don't have one. This patch fixes both of those problems. Test: 1. edit patron and remove it's birth date 2. try to search for it, and verify server error 3. apply patch 4. repeat search for patron and verify that it works and doesn't have enpty birth date field
I followed your test plan and I cannot reproduce the bug. Can someone else test to confirm that this a valid bug?
I found a way to reproduce it. It happens when 0000-00-00 is in the dateofbirth field in the borrowers table, not with NULL or ''. But the only way I managed to put the invalid date there was SQL. Dobrica, could this be old data we are dealing with here?
Created attachment 59194 [details] [review] Bug 17933 - Internal software error when searching patron without birth date When patrons don't have date of birth (which is not required) patron search results on moremember page produce internal server error since we can't convert MySQL invalid date 0000-00-00 to datetime object and call strfdate on it. Additionally, since we assign dates to template variables and after than assign whole $data hash to template, later assigment overrides previous one, so we see birth date field even for patrons which don't have one. This patch fixes both of those problems. Test: 1. edit patron and remove it's birth date 2. try to search for it, and verify server error 3. apply patch 4. repeat search for patron and verify that it works and doesn't have enpty birth date field Signed-off-by: Grace McKenzie <grace.mcky@gmail.com>
Created attachment 59214 [details] [review] Bug 17933: Add test and return unless defined dob Without this patch get_age return actually 0 instead of
Created attachment 59215 [details] [review] Bug 17933: Do not instanciate a patron if not needed
Hi Dobrica! Since 3.21.00.023 you should not have any 0000-00-00 values in the borrowers table. We should no longer deal with these values in our code. However undef must be handled in the ->get_age method. I also agree with the change in the pl script. Note that these 2 last patches will break again 0000-00-00, will you agree with that? Do you know why you still get these invalid values in DB?
Lowering the severity as it should not happen with valid data.
Most of our invalid birth dates in DB are result of external bulk script to open accounts at beginning of each academic year since source data sometimes doesn't have birth dates, so this is our local problem, and not Koha related. However, it doesn't explain all entries, since some of them are created after bulk script, using Koha's replication from LDAP. We have following mapping in koha-conf.xml: <dateofbirth is="hredupersondateofbirth" ></dateofbirth> and if LDAP entry doesn't have hredupersondateofbirth we are also getting 0000-00-00 for dateofbirth in Koha mosty because of $data = $mapping{$key}->{content} || ''; # default or failsafe '' from ldap_entry_2_hash in C4/Auth_with_ldap.pm. Sould I fix ldap_entry_2_hash as separate bug?
On closer review, our script is using C4::Auth_with_ldap::ldap_entry_2_hash so I opened Bug 18016 to track this issue.
Created attachment 59733 [details] [review] Bug 17933: Do not instanciate a patron if not needed Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>
Created attachment 59734 [details] [review] Bug 17933 - Internal software error when searching patron without birth date When patrons don't have date of birth (which is not required) patron search results on moremember page produce internal server error since we can't convert MySQL invalid date 0000-00-00 to datetime object and call strfdate on it. Additionally, since we assign dates to template variables and after than assign whole $data hash to template, later assigment overrides previous one, so we see birth date field even for patrons which don't have one. This patch fixes both of those problems. Test: 1. edit patron and remove it's birth date 2. try to search for it, and verify server error 3. apply patch 4. repeat search for patron and verify that it works and doesn't have enpty birth date field Signed-off-by: Grace McKenzie <grace.mcky@gmail.com>
Created attachment 59735 [details] [review] Bug 17933: Add test and return unless defined dob Without this patch get_age return actually 0 instead of Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>
Created attachment 59736 [details] [review] Bug 17933: Do not instanciate a patron if not needed Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>
Created attachment 60356 [details] [review] Bug 17933 - Internal software error when searching patron without birth date When patrons don't have date of birth (which is not required) patron search results on moremember page produce internal server error since we can't convert MySQL invalid date 0000-00-00 to datetime object and call strfdate on it. Additionally, since we assign dates to template variables and after than assign whole $data hash to template, later assigment overrides previous one, so we see birth date field even for patrons which don't have one. This patch fixes both of those problems. Test: 1. edit patron and remove it's birth date 2. try to search for it, and verify server error 3. apply patch 4. repeat search for patron and verify that it works and doesn't have enpty birth date field Signed-off-by: Grace McKenzie <grace.mcky@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 60357 [details] [review] Bug 17933: Add test and return unless defined dob Without this patch get_age return actually 0 instead of Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 60358 [details] [review] Bug 17933: Do not instanciate a patron if not needed Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 17.05, thank Dobrica, Jonathan!
Patch doesn't apply cleanly on 16.11.x - please rebase!
Hm, looking at the dependencies more closely - as bug 17557 is not part of 16.11.x this doesn't seem to be needed.