Bug 31143 - We should attempt to fix/identify all cases where '0000-00-00' may still remain in the database
Summary: We should attempt to fix/identify all cases where '0000-00-00' may still rema...
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: wainuiwitikapark
QA Contact: Testopia
URL:
Keywords:
Depends on: 18016 28440
Blocks:
  Show dependency treegraph
 
Reported: 2022-07-12 16:05 UTC by Martin Renvoize
Modified: 2023-10-13 10:07 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This new script identifies patron fields that have '0000-00-00' for date fields (for example: dateofbirth) and updates them to 'NULL'. Date fields with a value of '000-00-00' can cause upgrade and API issues, stop the autocomplete patron search working, and generate a 500 internal server error issue with normal patron searching.
Version(s) released in:


Attachments
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (2.42 KB, patch)
2023-08-07 04:41 UTC, wainuiwitikapark
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (2.42 KB, patch)
2023-08-13 22:31 UTC, wainuiwitikapark
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (2.47 KB, patch)
2023-09-20 23:49 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2022-07-12 16:05:40 UTC
With the move to more and more of Koha using the REST API's and our rest API's being suitably strict about date formats, we need to retrospectively correct occurrences of 0000-00-00 dates appear in database fields system wide.

I've had enough occurrences of bad data still in the database after a number of upgrade and it biting me in the UI that I think we should now look at introducing a script to either bulk fix these or at the least identify them and warn they will cause serious issues going forward.
Comment 1 wainuiwitikapark 2023-08-07 04:41:10 UTC
Created attachment 154299 [details] [review]
Bug 31143: Update patrons with dates of 0000-00-00 to NULL

To test:
1) Change a patron's value of one or more of the following to
0000-00-00: dateofbirth dateenrolled dateexpiry date_renewed
2) Run the fix_patron_dates.pl (-c -v)
3) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand
Comment 2 Laura Escamilla 2023-08-07 15:12:03 UTC
Hi! Thank you for working on this patch. I ran into this error:

Missing right curly or square bracket at misc/cronjobs/fix_patron_dates.pl line 53, at end of line
syntax error at misc/cronjobs/fix_patron_dates.pl line 53, at EOF

After adding the curly this works as described! I'm marking this as Failed QA for now, but once the patch is updated I'll be happy to sign off!
Comment 3 wainuiwitikapark 2023-08-13 22:31:08 UTC
Created attachment 154399 [details] [review]
Bug 31143: Update patrons with dates of 0000-00-00 to NULL

To test:
1) Change a patron's value of one or more of the following to
0000-00-00: dateofbirth dateenrolled dateexpiry date_renewed
2) Run the fix_patron_dates.pl (-c -v)
3) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand
Comment 4 wainuiwitikapark 2023-08-13 22:32:40 UTC
(In reply to Laura Escamilla from comment #2)
> Hi! Thank you for working on this patch. I ran into this error:
> 
> Missing right curly or square bracket at misc/cronjobs/fix_patron_dates.pl
> line 53, at end of line
> syntax error at misc/cronjobs/fix_patron_dates.pl line 53, at EOF
> 
> After adding the curly this works as described! I'm marking this as Failed
> QA for now, but once the patch is updated I'll be happy to sign off!

Thanks Laura for pointing that out.

I have added the curly bracket to the end of the file and reuploaded the patch :)
Comment 5 David Nind 2023-09-20 23:49:00 UTC
Created attachment 155964 [details] [review]
Bug 31143: Update patrons with dates of 0000-00-00 to NULL

To test:
1) Change a patron's value of one or more of the following to
0000-00-00: dateofbirth dateenrolled dateexpiry date_renewed
2) Run the fix_patron_dates.pl (-c -v)
3) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2023-09-21 00:16:45 UTC
Testing notes (using KTD):

1. Before applying the patch, I updated a couple of patron records using this SQL (Mary (49) and Henry (19)):
   . koha-mysql kohadev
   . update borrowers set dateofbirth ="0000-00-00" where borrowernumber = "49";
   . update borrowers set date_renewed ="0000-00-00" where borrowernumber = "19";

2. Note that the values updated now have a value of 0000-00-00 (SQL: select * from borrowers where borrowernumber = 19;).

3. In the staff interface, go to patrons. Note that the autocomplete for the search in the header doesn't work, and that you get a 500 error if you use the search on the left-hand side:

  Something went wrong when loading the table.
  500: Internal Server Error. 
  Month out of range.

4. Apply the patch (and restart_all).

5. Run the new script: misc/cronjobs/fix_patron_dates.pl -c -v

6. Output is: 2 wrong dates found

Some other notes:

1. If you run when there are no wrong dates, you get this (with the -c and -v options):

Use of uninitialized value $count in concatenation (.) or string at misc/cronjobs/fix_patron_dates.pl line 53.
 wrong dates found

2. I'm not sure of the protocol, but for many scripts:
   - you can add the -h or --help and get quick help with a description, what the different options do, and some examples.
   - normally the help would have the options, like -v, etc (in the script currently it just has c or v)
   - sometimes with verbose options, it may list the things changed

3. These could probably be added as new bugs, if determined as warranted.
Comment 7 Jonathan Druart 2023-09-29 14:26:52 UTC
This has been done already. Are you sure it's not coming from migration/import instead?

I would add a check in misc/maintenance/search_for_data_inconsistencies.pl instead. IMO it does not make sense to have such specific cronjobs.
Comment 8 Lucas Gass 2023-10-12 17:02:48 UTC
(In reply to Jonathan Druart from comment #7)
> This has been done already. Are you sure it's not coming from
> migration/import instead?

Just curious where this has been dealt with already.
Comment 9 Jonathan Druart 2023-10-13 10:07:28 UTC
(In reply to Lucas Gass from comment #8)
> (In reply to Jonathan Druart from comment #7)
> > This has been done already. Are you sure it's not coming from
> > migration/import instead?
> 
> Just curious where this has been dealt with already.

Search for "sanitize_zero_date" in updatedatabase.pl