Bug 7134 - patron records getting funny birthdays
Summary: patron records getting funny birthdays
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) major (vote)
Target Milestone: ---
Assignee: Kyle M Hall
QA Contact: Bugs List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-01 20:20 UTC by Nicole C. Engard
Modified: 2013-12-05 19:59 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Proposed Patch (1.83 KB, patch)
2011-11-17 15:40 UTC, Ian Walls
Details | Diff | Splinter Review
[Signed Off] Bug 7134: patron records getting corrupted birthdates (2.62 KB, patch)
2011-11-17 19:27 UTC, Liz Rea
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-11-01 20:20:03 UTC
When updating an existing patron an error message comes up about the birthday being invalid. It will let us save the record but puts in a birth date of 00/02/0.

There shouldn't be an error about the birthday field if it's not required and it shouldn't be populating it if we don't want it to.
Comment 1 Owen Leonard 2011-11-02 13:01:09 UTC
I can confirm this: If you edit a patron record which has a NULL dateofbirth, and leave the birth date field blank, after saving the record the dateofbirth column contains "0000-00-00"
Comment 2 Owen Leonard 2011-11-02 13:27:33 UTC
git bisect blames the fix for Bug 6253.
Comment 3 Ian Walls 2011-11-17 15:40:09 UTC Comment hidden (obsolete)
Comment 4 Liz Rea 2011-11-17 19:27:59 UTC
Created attachment 6330 [details] [review]
[Signed Off] Bug 7134: patron records getting corrupted birthdates

If no date of birth is provided in a new member form, the borrower gets the birthdate of "00/02/0", which
is obviously incorrect (the result of passing 0 to the database directly)

The fix is to remove a conditional check buried deep in C4::SQLHelper.  This is in a helper subroutine that was changed
for the Unified Member Search dev (bug 6253).  Reverting this small change to the conditional does not seem to break
search, but testing will be required.

To test:
1. apply patch, and create a new patron with blank birthdate.  The new patron should now have a blank birthdate instead of '00/02/0'
2. search for a borrower by 1 part surname, confirm search works
3. search for a borrower by 2 part surname, confirm search works
4. search for a borrower across firstname AND surname; confirm
5. search for a borrower and include categorycode/branchcode filters

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
I confirm all working tests 1-5
1. Created patron with blank birthdate - birthdate was blank
2. Searched for patron "henry james acevedo III" by using "acevedo" - result was correct
3. Searched for mr. acevedo using "Acevedo III" - result was correct
4. Searched for Henry James Acevedo III, Henry Acevedo and James Acevedo - results were correct
5. Searched for Mr Acevedo with the branch he was not in, with the branch he is in, with a category he was not, and a category he was. All results were correct.

also noted: inner searching seems to be working now? Example: Henry James (firstname) Acevedo III (surname) - searching James Acevedo brings back the correct patron.
Not complaining, just noting. :)
Comment 5 Liz Rea 2011-11-17 19:30:45 UTC
I should note I confirmed the bug itself before I applied the patch - nasty bugger.
Comment 6 Paul Poulain 2011-11-18 22:03:01 UTC
QA comment : small patch, addresses the problem, perfect test plan

Patch pushed, please test
Comment 7 Chris Nighswonger 2011-12-06 15:27:51 UTC
It appears that this patch broke the db dependent version of SQLHelper.t It would be great if we could get a follow-up patch to correct this so Jenkins will stop complaining. :-)
Comment 8 Ian Walls 2011-12-06 15:53:52 UTC
Deep and confusing problem in SQLHelper.pm. Not sure if the test can be corrected; logic still looks right.  Any any change to SQLHelper will likely lead to more bugs elsewhere in the system.

Not an easy issue to squelch.
Comment 9 Paul Poulain 2011-12-06 16:01:07 UTC
(In reply to comment #8)
> Deep and confusing problem in SQLHelper.pm. Not sure if the test can be
> corrected; logic still looks right.  Any any change to SQLHelper will likely
> lead to more bugs elsewhere in the system.

Ian, i'm not sure the problem comes from SQLHelper here. It's probably the test that must be fixed. I had a look, but I don't understand well how it works, I still have to investigate more.
Comment 10 Chris Nighswonger 2011-12-06 16:15:06 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > Deep and confusing problem in SQLHelper.pm. Not sure if the test can be
> > corrected; logic still looks right.  Any any change to SQLHelper will likely
> > lead to more bugs elsewhere in the system.
> 
> Ian, i'm not sure the problem comes from SQLHelper here. It's probably the test
> that must be fixed. I had a look, but I don't understand well how it works, I
> still have to investigate more.

Perhaps hdl could look into this since he wrote SQLHelper?
Comment 11 Chris Cormack 2011-12-06 23:32:35 UTC
I have fixed on bug 5327
Comment 12 Jared Camins-Esakov 2012-05-23 12:37:42 UTC
This was included in the 3.6 branch prior to 3.6.4.