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: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: Main
Hardware: All All
: P1 - high enhancement
Assignee: Wainui Witika-Park
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
Depends on: 18016 28440
Blocks: 38788
  Show dependency treegraph
 
Reported: 2022-07-12 16:05 UTC by Martin Renvoize (ashimema)
Modified: 2024-12-27 08:58 UTC (History)
17 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement: 1. Updates the misc/maintenance/search_for_data_inconsistencies.pl script so that it identifies any date fields that have 0000-00-00 values. 2. Adds a new script misc/maintenance/fix_invalid_dates.pl that fixes any date fields that have '0000-00-00' values (for example: dateofbirth) by updating them to 'NULL'. Patron, item, and other date fields with a value of '000-00-00' can cause errors. This includes: - API errors - stopping the patron autocomplete search working - generating a 500 internal server error: . for normal patron searching . when displaying item data in the holdings table
Version(s) released in:
24.11.00,24.05.06,23.11.11
Circulation function:


Attachments
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (2.42 KB, patch)
2023-08-07 04:41 UTC, Wainui Witika-Park
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, Wainui Witika-Park
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
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (1.57 KB, patch)
2024-07-09 07:05 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (3.68 KB, patch)
2024-07-11 01:48 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (3.76 KB, patch)
2024-07-11 07:53 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (4.15 KB, patch)
2024-07-11 07:57 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Work around subtracting datetime if date_of_birth undef (1.27 KB, patch)
2024-08-06 09:43 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (4.19 KB, patch)
2024-08-26 05:08 UTC, David Nind
Details | Diff | Splinter Review
Bug 31143: Work around subtracting datetime if date_of_birth undef (1.32 KB, patch)
2024-08-26 05:08 UTC, David Nind
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (4.23 KB, patch)
2024-09-20 00:36 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (4.19 KB, patch)
2024-09-20 00:38 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (4.19 KB, patch)
2024-09-20 00:41 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Update patrons with dates of 0000-00-00 to NULL (4.21 KB, patch)
2024-09-20 00:48 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Update items with dates of 0000-00-00 to NULL (4.27 KB, patch)
2024-09-20 00:58 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Update items with dates of 0000-00-00 to NULL (4.27 KB, patch)
2024-09-20 01:02 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Fix/identify all cases where '0000-00-00' may still remain in the database (5.58 KB, patch)
2024-09-24 05:53 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 31143: Work around subtracting datetime if date_of_birth undef (1.37 KB, patch)
2024-09-26 14:24 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 31143: Fix/identify all cases where '0000-00-00' may still remain in the database (5.63 KB, patch)
2024-09-26 14:24 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 31143: Work around subtracting datetime if date_of_birth undef (1.37 KB, patch)
2024-09-27 17:45 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31143: Fix/identify all cases where '0000-00-00' may still remain in the database (5.63 KB, patch)
2024-09-27 17:46 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31143: (QA follow-up) move script to maintenance (620 bytes, patch)
2024-09-27 17:46 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31143: (QA follow-up) Tidy (4.53 KB, patch)
2024-09-27 17:46 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31143: Work around subtracting datetime if date_of_birth undef (1.43 KB, patch)
2024-09-27 17:56 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31143: Fix/identify all cases where '0000-00-00' may still remain in the database (5.69 KB, patch)
2024-09-27 17:56 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31143: (QA follow-up) move script to maintenance (676 bytes, patch)
2024-09-27 17:56 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31143: (QA follow-up) Tidy (4.58 KB, patch)
2024-09-27 17:56 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 31143: (Follow-up) Add description to fix_invalid_dates.pl (921 bytes, patch)
2024-11-04 05:32 UTC, Wainui Witika-Park
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 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 Wainui Witika-Park 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 Wainui Witika-Park 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 Wainui Witika-Park 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 (lukeg) 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
Comment 10 Caroline Cyr La Rose 2024-06-25 13:49:34 UTC
A lot of people seem to have had problems recently with 0000-00-00 dates... There were two different threads on the mailing list about the same thing in the last month.

https://lists.katipo.co.nz/public/koha/2024-June/060716.html

https://lists.katipo.co.nz/public/koha/2024-June/060745.html
Comment 11 Katrin Fischer 2024-06-25 13:50:38 UTC
(In reply to Caroline Cyr La Rose from comment #10)
> A lot of people seem to have had problems recently with 0000-00-00 dates...
> There were two different threads on the mailing list about the same thing in
> the last month.
> 
> https://lists.katipo.co.nz/public/koha/2024-June/060716.html
> 
> https://lists.katipo.co.nz/public/koha/2024-June/060745.html

One bug we have identified is bug 36488 - date of birth during patron self registration.
Comment 12 Fridolin Somers 2024-06-25 18:59:54 UTC
Looks like there are more fields than "dateofbirth dateenrolled dateexpiry date_renewed" to check :
+lastseen
+updated_on
+debarred

> I would add a check in misc/maintenance/search_for_data_inconsistencies.pl instead.
+1
Comment 13 Caroline Cyr La Rose 2024-06-25 19:13:27 UTC
IIUC Wainui's script also corrected the dates to NULL instead of 0000-00-00. Is this something we can do with search_for_data_inconsistencies?

I seem to remember that it corrects item-level itemtypes when the record-level itemtype is present, so I'm guessing yes?
Comment 14 Wainui Witika-Park 2024-07-03 04:12:25 UTC
Apologies I haven't been able to find time to address the comments on this bug report 

Removing myself as the assignee, if someone else wants to pick this up, or I can check back in a few days when I have some time to look at it
Comment 15 Wainui Witika-Park 2024-07-09 07:05:38 UTC
Created attachment 168636 [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 lastseen
updated_on debarred
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand
Comment 16 Jonathan Druart 2024-07-09 13:25:48 UTC
This script is designed to report data inconsistencies but it is not expected to fix them.

If we want we should at least require a flag (--fix or whatever). But in that case... What about the other inconsistencies?

I would suggest to search for 0000-00-00 then display the query to fix them in the hint part.
Comment 17 Jonathan Druart 2024-07-09 13:26:47 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2024-07-09 13:27:44 UTC
(In reply to Caroline Cyr La Rose from comment #13)
> IIUC Wainui's script also corrected the dates to NULL instead of 0000-00-00.
> Is this something we can do with search_for_data_inconsistencies?

Yes, I don't think it is correct.

> I seem to remember that it corrects item-level itemtypes when the
> record-level itemtype is present, so I'm guessing yes?

No, I don't think the script is modifying data for now.
Comment 19 Katrin Fischer 2024-07-09 13:30:36 UTC
Could/should we add this as a command-line option?
Comment 20 Katrin Fischer 2024-07-09 13:38:52 UTC
(In reply to Katrin Fischer from comment #19)
> Could/should we add this as a command-line option?

It would be nice if we could provide people with some help in fixing the issue as it has come up a lot and can only be fixed with SQL right now. A hint could work too, but might require more explanations.
Comment 21 David Nind 2024-07-09 18:25:02 UTC
I had a go at testing this yesterday (see my comments on how I tested previously #c6). (I used KTD and the sample data, with some SQL changes to make some patron records incorrect.

However, for me it didn't fix anything for me or output a list of patrons that have data that needs correction.

I was going to have another go today, but I'll leave it for the moment.

I sort of liked how the previously proposed script worked. 8)
Comment 22 Wainui Witika-Park 2024-07-09 23:14:11 UTC
Hi everyone,

> It would be nice if we could provide people with some help in fixing the issue as it has come up a lot and can only be fixed with SQL right now.

Yeah I could really use some help please with getting this fix through as soon as possible, it is a huge problem with many of our clients since a recent minor point.

> I would suggest to search for 0000-00-00 then display the query to fix them in the hint part.

We would prefer the fix automatically fixes the dates, and not just give a prompt. However, I see that my latest patch shouldn't change data in misc/maintenance/search_for_data_inconsistencies.pl

> I sort of liked how the previously proposed script worked.

I agree, and would be happy to amend it with the extra fields, unless someone can get this working as soon as possible

Thanks,

Wainui and the Catalyst team
Comment 23 Wainui Witika-Park 2024-07-09 23:15:00 UTC
Unassigning myself if someone wants to pick it up? Otherwise I can work on it if I have some guidance :)
Comment 24 Jonathan Druart 2024-07-10 08:00:28 UTC
We can have both.

The search_for_inconsistencies maintenance script can point to the other script.
Also I think the script to fix the dates should not contain "patrons" as we could want to reuse it to fix dates from other database tables.
Comment 25 Blou 2024-07-10 11:36:44 UTC
we have a task opened to make a plugin out of this script.  Mostly as a "learn the ropes" for interns, but it has the advantage of being easily hackable to make it what we want.  Like fix things when there's no doubt as to what must be done.
Comment 26 Wainui Witika-Park 2024-07-11 01:48:36 UTC
Created attachment 168770 [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 lastseen
updated_on debarred
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_patron_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand and
Catalyst IT
Comment 27 David Nind 2024-07-11 03:14:05 UTC
Thanks Wainui!

I had a go at testing.

1. For me, misc/maintenance/search_for_data_inconsistencies.pl does not show anything in the output when I run it - where there are incorrect dates. I'm not sure if this is because of the error it generates when running (see step 2 below) - this occurred both before and after the patch was applied.

2. If the new script is run just with -v, it just says "0 wrong dates found and fixed". I guess that is correct. But if you were running it before using the -c option, I think you would want to know that "X wrong dates were found" (or something similar).

3. On the plus side, the new script does fix 0000-00-00 dates! 8-)

Updated testing notes (using KTD and the sample data):

1. Fix record 369 (Strk̆v̜elser illustreret af Jean Anderson dansk udgave ved Lis Engel [overst̆telse ved Jesper Langer]) so that the misc/maintenance/search_for_data_inconsistencies.pl script doesn't show MARC records in the sample data that needs fixing.

2. Note that before applying the patch, running the search for data inconsistencies script returns an error/warning message:

   misc/maintenance/search_for_data_inconsistencies.pl
   Can't call method "time_zone" on an undefined value at /usr/lib/x86_64-linux-gnu/perl5/5.36/DateTime.pm line 1483.

3. 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";

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

5. 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.

6. Run the search for data inconsistencies script: misc/maintenance/search_for_data_inconsistencies.pl

   NOTE: This shows no information for me about any problems with patron dates with 0000-00-00

7. Output from running the new script:
   - with just -v: 
     misc/cronjobs/fix_invalid_dates.pl -v
     0 wrong dates found and fixed
   - with -c -v: 
     misc/cronjobs/fix_invalid_dates.pl -c -v
     2 wrong dates found and fixed

8. Repeat step 5 - things should now work as expected.

Some other notes:

1. The description in misc/maintenance/search_for_data_inconsistencies.pl needs updating to add:
   * Patrons with 0000-00-00 dates in fields dateofbirth, dateenrolled, dateexpiry, date_renewed, lastseen, updated_on debarred

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 28 Wainui Witika-Park 2024-07-11 07:53:43 UTC
Created attachment 168775 [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 lastseen
updated_on debarred
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_patron_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand and
Catalyst IT
Comment 29 Wainui Witika-Park 2024-07-11 07:57:36 UTC
Created attachment 168776 [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 lastseen
updated_on debarred
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_patron_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand and
Catalyst IT
Comment 30 David Nind 2024-07-16 05:09:46 UTC
I get this error message after I have updated some patrons to have 0000-00-00 dates - both before and after the patch.

   misc/maintenance/search_for_data_inconsistencies.pl
   Can't call method "time_zone" on an undefined value at /usr/lib/x86_64-linux-gnu/perl5/5.36/DateTime.pm line 1483.

Without the patch and without any incorrect dates, the above message doesn't appear.

After the patch and after the dates are fixed with the new script, I get (as expected):

  misc/maintenance/search_for_data_inconsistencies.pl -v
  == 0 invalid dates found ==
Comment 31 Wainui Witika-Park 2024-07-18 01:56:15 UTC
(In reply to David Nind from comment #30)
> I get this error message after I have updated some patrons to have
> 0000-00-00 dates - both before and after the patch.
> 
>    misc/maintenance/search_for_data_inconsistencies.pl
>    Can't call method "time_zone" on an undefined value at
> /usr/lib/x86_64-linux-gnu/perl5/5.36/DateTime.pm line 1483.
> 
> Without the patch and without any incorrect dates, the above message doesn't
> appear.
> 
> After the patch and after the dates are fixed with the new script, I get (as
> expected):
> 
>   misc/maintenance/search_for_data_inconsistencies.pl -v
>   == 0 invalid dates found ==

Thanks for testing and for your notes!

- I changed a date to 0000-00-00
- I ran misc/maintenance/search_for_data_inconsistencies.pl
-> I don't get that error

- I added patch
- I changed a date to 0000-00-00
- I ran misc/maintenance/search_for_data_inconsistencies.pl
-> I don't get that error
-> I get, as expected:
== 1 invalid dates found ==
You may change the date with script: misc/cronjobs/fix_invalid_dates.pl (-c -v)
- I then ran ./misc/cronjobs/fix_invalid_dates.pl -v
-> I got the following, as expected:
1 invalid dates found
- I then ran ./misc/cronjobs/fix_invalid_dates.pl -c -v
-> I got the following, as expected:
1 invalid dates found
1 invalid dates fixed
- Running it again
-> I got, as expected:
0 invalid dates found
0 invalid dates fixed
- Then I checked the date in the db, and it has indeed been fixed (changed to NULL)

So yeah, I can't seem to replicate your issues :/
Comment 32 Wainui Witika-Park 2024-07-18 04:09:03 UTC
(In reply to wainuiwitikapark from comment #31)
> (In reply to David Nind from comment #30)
> > I get this error message after I have updated some patrons to have
> > 0000-00-00 dates - both before and after the patch.
> > 
> >    misc/maintenance/search_for_data_inconsistencies.pl
> >    Can't call method "time_zone" on an undefined value at
> > /usr/lib/x86_64-linux-gnu/perl5/5.36/DateTime.pm line 1483.
> > 
> > Without the patch and without any incorrect dates, the above message doesn't
> > appear.
> > 
> > After the patch and after the dates are fixed with the new script, I get (as
> > expected):
> > 
> >   misc/maintenance/search_for_data_inconsistencies.pl -v
> >   == 0 invalid dates found ==
> 
> Thanks for testing and for your notes!
> 
> - I changed a date to 0000-00-00
> - I ran misc/maintenance/search_for_data_inconsistencies.pl
> -> I don't get that error
> 
> - I added patch
> - I changed a date to 0000-00-00
> - I ran misc/maintenance/search_for_data_inconsistencies.pl
> -> I don't get that error
> -> I get, as expected:
> == 1 invalid dates found ==
> You may change the date with script: misc/cronjobs/fix_invalid_dates.pl (-c
> -v)
> - I then ran ./misc/cronjobs/fix_invalid_dates.pl -v
> -> I got the following, as expected:
> 1 invalid dates found
> - I then ran ./misc/cronjobs/fix_invalid_dates.pl -c -v
> -> I got the following, as expected:
> 1 invalid dates found
> 1 invalid dates fixed
> - Running it again
> -> I got, as expected:
> 0 invalid dates found
> 0 invalid dates fixed
> - Then I checked the date in the db, and it has indeed been fixed (changed
> to NULL)
> 
> So yeah, I can't seem to replicate your issues :/

Oh sorry I have retested and can now replicate the issue
Comment 33 Wainui Witika-Park 2024-07-18 04:20:29 UTC
(In reply to David Nind from comment #30)
> I get this error message after I have updated some patrons to have
> 0000-00-00 dates - both before and after the patch.
> 
>    misc/maintenance/search_for_data_inconsistencies.pl
>    Can't call method "time_zone" on an undefined value at
> /usr/lib/x86_64-linux-gnu/perl5/5.36/DateTime.pm line 1483.
> 
> Without the patch and without any incorrect dates, the above message doesn't
> appear.
> 
> After the patch and after the dates are fixed with the new script, I get (as
> expected):
> 
>   misc/maintenance/search_for_data_inconsistencies.pl -v
>   == 0 invalid dates found ==

What date fields were you changing? Were you changing dateofbirth?

I have done some more testing and I think it is only dateofbirth that gives the error
Comment 34 David Nind 2024-07-18 04:31:58 UTC
(In reply to wainuiwitikapark from comment #33)
> 
> What date fields were you changing? Were you changing dateofbirth?
> 
> I have done some more testing and I think it is only dateofbirth that gives
> the error

Thanks for checking and trying to replicate the issue I was having!

I tested with changed dates for dateofbirth and date_renewed.

The error happens with changing the dates before the patch is applied, so it must be something else already in the script.
Comment 35 Wainui Witika-Park 2024-07-19 01:07:37 UTC
(In reply to David Nind from comment #34)
> (In reply to wainuiwitikapark from comment #33)
> > 
> > What date fields were you changing? Were you changing dateofbirth?
> > 
> > I have done some more testing and I think it is only dateofbirth that gives
> > the error
> 
> Thanks for checking and trying to replicate the issue I was having!
> 
> I tested with changed dates for dateofbirth and date_renewed.
> 
> The error happens with changing the dates before the patch is applied, so it
> must be something else already in the script.

No worries and thanks for clarifying that!
Comment 36 Wainui Witika-Park 2024-08-06 08:31:49 UTC
(In reply to David Nind from comment #30)
> I get this error message after I have updated some patrons to have
> 0000-00-00 dates - both before and after the patch.
> 
>    misc/maintenance/search_for_data_inconsistencies.pl
>    Can't call method "time_zone" on an undefined value at
> /usr/lib/x86_64-linux-gnu/perl5/5.36/DateTime.pm line 1483.
> 
> Without the patch and without any incorrect dates, the above message doesn't
> appear.
> 
> After the patch and after the dates are fixed with the new script, I get (as
> expected):
> 
>   misc/maintenance/search_for_data_inconsistencies.pl -v
>   == 0 invalid dates found ==

Hi David,

Do you have any ideas on how I could potentially fix the time zone issue?

Thanks :)
Comment 37 David Nind 2024-08-06 08:33:50 UTC
(In reply to wainuiwitikapark from comment #36)
> (In reply to David Nind from comment #30)
> > I get this error message after I have updated some patrons to have
> > 0000-00-00 dates - both before and after the patch.
> > 
> >    misc/maintenance/search_for_data_inconsistencies.pl
> >    Can't call method "time_zone" on an undefined value at
> > /usr/lib/x86_64-linux-gnu/perl5/5.36/DateTime.pm line 1483.
> > 
> > Without the patch and without any incorrect dates, the above message doesn't
> > appear.
> > 
> > After the patch and after the dates are fixed with the new script, I get (as
> > expected):
> > 
> >   misc/maintenance/search_for_data_inconsistencies.pl -v
> >   == 0 invalid dates found ==
> 
> Hi David,
> 
> Do you have any ideas on how I could potentially fix the time zone issue?
> 
> Thanks :)

Sorry, I'm not a programmer and don't know Perl at all 8-(.
Comment 38 Paul Derscheid 2024-08-06 09:23:22 UTC
I've looked into this a bit and this would either need:
1) dt_from_string to return a DateTime object instead of undef.
2) A fix for all dt_from_string calls within Koha::Patron.

Pick your poison :D I'd like to hear some opinions.
Comment 39 Paul Derscheid 2024-08-06 09:26:00 UTC
dt_from_string receiving something starting w/ '0000-0' that is.

Koha/DateUtils.pm:62
`return if $date_string and $date_string =~ m|^0000-0|;`
Comment 40 Pedro Amorim 2024-08-06 09:43:15 UTC
Created attachment 170086 [details] [review]
Bug 31143: Work around subtracting datetime if date_of_birth undef

To test:
1) Run the following SQL:
update borrowers set dateofbirth = '0000-00-00' where borrowernumber = 45;
2) Access the borrower edit page through staff UI:
http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=edit_form&destination=circ&borrowernumber=45
3) Notice you get an error
4) Run the script:
perl misc/maintenance/search_for_data_inconsistencies.pl
5) Notice you get an error
6) Apply patch restart plack and repeat -> no errors

This is not perfect but its all I have time for, for now. Follow-ups are welcomed as always.
Comment 41 Fridolin Somers 2024-08-06 10:17:08 UTC
> my $borrowers = Koha::Patrons->search(

Looks very strange to me this var name.
We usually name it $patrons when it is a Koha::Patrons object.
Comment 42 Fridolin Somers 2024-08-06 10:21:19 UTC
> Work around subtracting datetime if date_of_birth undef 
This change will impact unit test 't/db_dependent/Koha/Patrons.t' no ?
Comment 43 Pedro Amorim 2024-08-06 16:04:28 UTC
(In reply to Fridolin Somers from comment #42)
> > Work around subtracting datetime if date_of_birth undef 
> This change will impact unit test 't/db_dependent/Koha/Patrons.t' no ?

prove t/db_dependent/Koha/Patrons.t
t/db_dependent/Koha/Patrons.t .. ok     
All tests successful.
Files=1, Tests=43, 42 wallclock secs ( 0.06 usr  0.01 sys + 19.83 cusr 20.13 csys = 40.03 CPU)
Result: PASS

It doesn't seem to impact t/db_dependent/Koha/Patrons.t. I would expect it to impact it only if that test suite is already considering this possibility (undef $date_of_birth) which I don't believe it is, but I may be wrong.
If the above is true, a new regression test for this should probably be added in t/db_dependent/Koha/Patrons.t
Comment 44 Wainui Witika-Park 2024-08-07 06:54:00 UTC
(In reply to Fridolin Somers from comment #41)
> > my $borrowers = Koha::Patrons->search(
> 
> Looks very strange to me this var name.
> We usually name it $patrons when it is a Koha::Patrons object.

I can change it if you think that's best?
Comment 45 Danyon Sewell 2024-08-23 02:40:01 UTC
Kia ora Wainui

I have just found that this issue doesn't just affect the borrowers table, I just had to fix this for a database where some of the items table also included the bad daterange, specifically the replacementpricedate column.

It may be worth to include the columns within the items table in this script to better capture all of the possible places this bad daterange may end up.

From the items table the following columns could be affected:

dateaccessioned
replacementpricedate
datelastborrowed
datelastseen
damaged_on
itemlost_on
withdrawn_on
deleted_on
onloan

Cheers,
Danyon
Comment 46 Wainui Witika-Park 2024-08-23 03:33:10 UTC
(In reply to Danyon Sewell from comment #45)
> Kia ora Wainui
> 
> I have just found that this issue doesn't just affect the borrowers table, I
> just had to fix this for a database where some of the items table also
> included the bad daterange, specifically the replacementpricedate column.
> 
> It may be worth to include the columns within the items table in this script
> to better capture all of the possible places this bad daterange may end up.
> 
> From the items table the following columns could be affected:
> 
> dateaccessioned
> replacementpricedate
> datelastborrowed
> datelastseen
> damaged_on
> itemlost_on
> withdrawn_on
> deleted_on
> onloan
> 
> Cheers,
> Danyon

Thanks Danyon! I'll look into that!
Comment 47 David Nind 2024-08-26 05:08:54 UTC
Created attachment 170697 [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 lastseen
updated_on debarred
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_patron_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand and
Catalyst IT

Signed-off-by: David Nind <david@davidnind.com>
Comment 48 David Nind 2024-08-26 05:08:57 UTC
Created attachment 170698 [details] [review]
Bug 31143: Work around subtracting datetime if date_of_birth undef

To test:
1) Run the following SQL:
update borrowers set dateofbirth = '0000-00-00' where borrowernumber = 45;
2) Access the borrower edit page through staff UI:
http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=edit_form&destination=circ&borrowernumber=45
3) Notice you get an error
4) Run the script:
perl misc/maintenance/search_for_data_inconsistencies.pl
5) Notice you get an error
6) Apply patch restart plack and repeat -> no errors

This is not perfect but its all I have time for, for now. Follow-ups are welcomed as always.

Signed-off-by: David Nind <david@davidnind.com>
Comment 49 David Nind 2024-08-26 05:21:49 UTC
Testing notes (using KTD):

1. Fix record 369 (Strk̆v̜elser illustreret af Jean Anderson dansk udgave ved Lis Engel [overst̆telse ved Jesper Langer]) so that the misc/maintenance/search_for_data_inconsistencies.pl script doesn't show MARC records in the sample data that needs fixing.

2. Update 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";

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

4. In the staff interface, go to patrons. Note that the autocomplete for the search in the header doesn't work (search for hen), 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.

5. Run the search for data inconsistencies script: misc/maintenance/search_for_data_inconsistencies.pl. You will get this error:
   Can't call method "time_zone" on an undefined value at /usr/lib/x86_64-linux-gnu/perl5/5.36/DateTime.pm line 1483.

6. Apply the second patch: Bug 31143: Work around subtracting datetime if date_of_birth undef

7. Run this command: misc/maintenance/search_for_data_inconsistencies.pl -v
   Result:
    == Patrons with invalid age for category ==
         * Patron borrowernumber=49 has an invalid age of 0 for their category 'PT' (18 to 999)
    => You may change the patron's category automatically with misc/cronjobs/update_patrons_category.pl

8. Apply the first patch.

9. Run this command again: misc/maintenance/search_for_data_inconsistencies.pl -v
   Result: now identifies that some patrons have invalid dates
    == Patrons with invalid age for category ==
         * Patron borrowernumber=49 has an invalid age of 0 for their category 'PT' (18 to 999)
       => You may change the patron's category automatically with misc/cronjobs/update_patrons_category.pl

    == 2 invalid dates found ==
    => You may change the date with script: misc/cronjobs/fix_invalid_dates.pl (-c -v)

10. Run the new script with just the -v option:
     misc/cronjobs/fix_invalid_dates.pl -v
     2 invalid dates found

11. Run the new script with the -c -v options: 
     misc/cronjobs/fix_invalid_dates.pl -c -v
     2 wrong dates found and fixed

12. Repeat step 4 - things should now work as expected.
Comment 50 Wainui Witika-Park 2024-09-20 00:36:20 UTC
Created attachment 171812 [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 lastseen
updated_on debarred
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_patron_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand and
Catalyst IT

Signed-off-by: David Nind <david@davidnind.com>
Comment 51 Wainui Witika-Park 2024-09-20 00:38:42 UTC
Created attachment 171813 [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 lastseen
updated_on debarred
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_patron_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand and
Catalyst IT
Comment 52 Wainui Witika-Park 2024-09-20 00:41:42 UTC
Created attachment 171814 [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 lastseen
updated_on debarred
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_invalid_patron_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand and
Catalyst IT
Comment 53 Wainui Witika-Park 2024-09-20 00:48:09 UTC
Created attachment 171815 [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 lastseen
updated_on debarred
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_invalid_patron_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL

Sponsored by: Toi Ohomai Institute of Technology, New Zealand and
Catalyst IT
Comment 54 Wainui Witika-Park 2024-09-20 00:58:18 UTC
Created attachment 171816 [details] [review]
Bug 31143: Update items 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: dateaccessioned replacementpricedate datelastborrowed datelastseen damaged_on itemlost_on withdrawn_on deleted_on onloan
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_invalid_item_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL
Comment 55 Wainui Witika-Park 2024-09-20 01:02:53 UTC
Created attachment 171817 [details] [review]
Bug 31143: Update items with dates of 0000-00-00 to NULL

To test:
1) Change an item's value of one or more of the following to 0000-00-00: dateaccessioned replacementpricedate datelastborrowed datelastseen damaged_on itemlost_on withdrawn_on deleted_on onloan
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run the fix_invalid_item_dates.pl (-c -v)
4) Notice the value has changed from 0000-00-00 to NULL
Comment 56 Wainui Witika-Park 2024-09-20 01:32:46 UTC
Hi I added in behaviour to fix incorrect item dates too
Comment 57 Jonathan Druart 2024-09-23 08:00:17 UTC
Hello Wainui,
The title of this bug report states that the patches are supposed to "identify all cases in the database". But with those 2 patches we are only dealing with two tables: items and patrons.

I am suggesting the following snippet of code that will allow you to catch invalid dates in the whole database:

use Koha::Database;
my $schema = Koha::Database->new->schema;
# Loop over all the DBIx::Class classes
for my $class ( sort values %{$schema->{class_mappings}} ) {
    # Retrieve the resultset so we can access the columns info
    my $rs = $schema->resultset($class);
    my $columns = $rs->result_source->columns_info;

    # Loop over the columns
    while ( my ( $column, $info ) = each %$columns ) {
        # Next if data type is not date/datetime/timestamp
        my $data_type = $info->{data_type};
        next unless grep { $data_type =~ m{^$_$} } qw( timestamp datetime date );

        # Count the invalid dates
        my $invalid_dates = $rs->search({ $column => '0000-00-00' })->count;

        next unless $invalid_dates;

        # "items.withdrawn_on contains 42 invalid dates"
        say sprintf "Column %s.%s contains %s invalid dates", $rs->result_source->name, $column, $invalid_dates;
    }
}

Let me know if you have any questions about it. I can help you to adjust it to your needs.
Comment 58 Wainui Witika-Park 2024-09-24 00:50:05 UTC
(In reply to Jonathan Druart from comment #57)
> Hello Wainui,
> The title of this bug report states that the patches are supposed to
> "identify all cases in the database". But with those 2 patches we are only
> dealing with two tables: items and patrons.
> 
> I am suggesting the following snippet of code that will allow you to catch
> invalid dates in the whole database:
> 
> use Koha::Database;
> my $schema = Koha::Database->new->schema;
> # Loop over all the DBIx::Class classes
> for my $class ( sort values %{$schema->{class_mappings}} ) {
>     # Retrieve the resultset so we can access the columns info
>     my $rs = $schema->resultset($class);
>     my $columns = $rs->result_source->columns_info;
> 
>     # Loop over the columns
>     while ( my ( $column, $info ) = each %$columns ) {
>         # Next if data type is not date/datetime/timestamp
>         my $data_type = $info->{data_type};
>         next unless grep { $data_type =~ m{^$_$} } qw( timestamp datetime
> date );
> 
>         # Count the invalid dates
>         my $invalid_dates = $rs->search({ $column => '0000-00-00' })->count;
> 
>         next unless $invalid_dates;
> 
>         # "items.withdrawn_on contains 42 invalid dates"
>         say sprintf "Column %s.%s contains %s invalid dates",
> $rs->result_source->name, $column, $invalid_dates;
>     }
> }
> 
> Let me know if you have any questions about it. I can help you to adjust it
> to your needs.

Thanks Jonathan!

I will have a go at implementing this
Comment 59 Wainui Witika-Park 2024-09-24 05:53:56 UTC
Created attachment 171918 [details] [review]
Bug 31143: Fix/identify all cases where '0000-00-00' may still remain in the database

This patch identifies date fields in the database (timestamp, datetime, date) that are 0000-00-00 and sets them to NULL

Test plan -

Examples of setting up some 0000-00-00 date fields:

e.g. Borrowers table
update borrowers set dateexpiry="0000-00-00" where borrowernumber=49;

e.g. Items table
update items set datelastseen="0000-00-00" where itemnumber=12;

To identify the cases:
1) Set up some 0000-00-00 date fields
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run fix_invalid_dates.pl -v

To fix the cases:
1) Run fix_invalid_dates.pl -c -v
2) Notice the value has changed from 0000-00-00 to NULL

e.g. Borrowers table
select dateexpiry from borrowers where borrowernumber=49;

e.g. Items table
select datelastseen from items where itemnumber=12;
Comment 60 Shi Yao Wang 2024-09-26 14:24:12 UTC
Created attachment 171976 [details] [review]
Bug 31143: Work around subtracting datetime if date_of_birth undef

To test:
1) Run the following SQL:
update borrowers set dateofbirth = '0000-00-00' where borrowernumber = 45;
2) Access the borrower edit page through staff UI:
http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=edit_form&destination=circ&borrowernumber=45
3) Notice you get an error
4) Run the script:
perl misc/maintenance/search_for_data_inconsistencies.pl
5) Notice you get an error
6) Apply patch restart plack and repeat -> no errors

This is not perfect but its all I have time for, for now. Follow-ups are welcomed as always.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Comment 61 Shi Yao Wang 2024-09-26 14:24:15 UTC
Created attachment 171977 [details] [review]
Bug 31143: Fix/identify all cases where '0000-00-00' may still remain in the database

This patch identifies date fields in the database (timestamp, datetime, date) that are 0000-00-00 and sets them to NULL

Test plan -

Examples of setting up some 0000-00-00 date fields:

e.g. Borrowers table
update borrowers set dateexpiry="0000-00-00" where borrowernumber=49;

e.g. Items table
update items set datelastseen="0000-00-00" where itemnumber=12;

To identify the cases:
1) Set up some 0000-00-00 date fields
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run fix_invalid_dates.pl -v

To fix the cases:
1) Run fix_invalid_dates.pl -c -v
2) Notice the value has changed from 0000-00-00 to NULL

e.g. Borrowers table
select dateexpiry from borrowers where borrowernumber=49;

e.g. Items table
select datelastseen from items where itemnumber=12;

Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Comment 62 Wainui Witika-Park 2024-09-26 22:28:36 UTC
(In reply to Shi Yao Wang from comment #61)
> Created attachment 171977 [details] [review] [review]
> Bug 31143: Fix/identify all cases where '0000-00-00' may still remain in the
> database
> 
> This patch identifies date fields in the database (timestamp, datetime,
> date) that are 0000-00-00 and sets them to NULL
> 
> Test plan -
> 
> Examples of setting up some 0000-00-00 date fields:
> 
> e.g. Borrowers table
> update borrowers set dateexpiry="0000-00-00" where borrowernumber=49;
> 
> e.g. Items table
> update items set datelastseen="0000-00-00" where itemnumber=12;
> 
> To identify the cases:
> 1) Set up some 0000-00-00 date fields
> 2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
> 3) Run fix_invalid_dates.pl -v
> 
> To fix the cases:
> 1) Run fix_invalid_dates.pl -c -v
> 2) Notice the value has changed from 0000-00-00 to NULL
> 
> e.g. Borrowers table
> select dateexpiry from borrowers where borrowernumber=49;
> 
> e.g. Items table
> select datelastseen from items where itemnumber=12;
> 
> Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>

Thanks so much for signing off!
Comment 63 Nick Clemens (kidclamp) 2024-09-27 17:45:57 UTC
Created attachment 172138 [details] [review]
Bug 31143: Work around subtracting datetime if date_of_birth undef

To test:
1) Run the following SQL:
update borrowers set dateofbirth = '0000-00-00' where borrowernumber = 45;
2) Access the borrower edit page through staff UI:
http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=edit_form&destination=circ&borrowernumber=45
3) Notice you get an error
4) Run the script:
perl misc/maintenance/search_for_data_inconsistencies.pl
5) Notice you get an error
6) Apply patch restart plack and repeat -> no errors

This is not perfect but its all I have time for, for now. Follow-ups are welcomed as always.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Comment 64 Nick Clemens (kidclamp) 2024-09-27 17:46:01 UTC
Created attachment 172139 [details] [review]
Bug 31143: Fix/identify all cases where '0000-00-00' may still remain in the database

This patch identifies date fields in the database (timestamp, datetime, date) that are 0000-00-00 and sets them to NULL

Test plan -

Examples of setting up some 0000-00-00 date fields:

e.g. Borrowers table
update borrowers set dateexpiry="0000-00-00" where borrowernumber=49;

e.g. Items table
update items set datelastseen="0000-00-00" where itemnumber=12;

To identify the cases:
1) Set up some 0000-00-00 date fields
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run fix_invalid_dates.pl -v

To fix the cases:
1) Run fix_invalid_dates.pl -c -v
2) Notice the value has changed from 0000-00-00 to NULL

e.g. Borrowers table
select dateexpiry from borrowers where borrowernumber=49;

e.g. Items table
select datelastseen from items where itemnumber=12;

Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Comment 65 Nick Clemens (kidclamp) 2024-09-27 17:46:04 UTC
Created attachment 172140 [details] [review]
Bug 31143: (QA follow-up) move script to maintenance
Comment 66 Nick Clemens (kidclamp) 2024-09-27 17:46:08 UTC
Created attachment 172141 [details] [review]
Bug 31143: (QA follow-up) Tidy
Comment 67 Nick Clemens (kidclamp) 2024-09-27 17:47:53 UTC
I moved the script from cronjobs to maintenance as I feel it should be avoided to be creating these, not expected to need fixing on a schedule, this can be dropped if desired. I also tidied.

It would be nice to have increased verbosity and list which object were affected, but reporting the column allows for reporting, so this works.

Nice patches, passing QA
Comment 68 Nick Clemens (kidclamp) 2024-09-27 17:56:08 UTC
Created attachment 172143 [details] [review]
Bug 31143: Work around subtracting datetime if date_of_birth undef

To test:
1) Run the following SQL:
update borrowers set dateofbirth = '0000-00-00' where borrowernumber = 45;
2) Access the borrower edit page through staff UI:
http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=edit_form&destination=circ&borrowernumber=45
3) Notice you get an error
4) Run the script:
perl misc/maintenance/search_for_data_inconsistencies.pl
5) Notice you get an error
6) Apply patch restart plack and repeat -> no errors

This is not perfect but its all I have time for, for now. Follow-ups are welcomed as always.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 69 Nick Clemens (kidclamp) 2024-09-27 17:56:11 UTC
Created attachment 172144 [details] [review]
Bug 31143: Fix/identify all cases where '0000-00-00' may still remain in the database

This patch identifies date fields in the database (timestamp, datetime, date) that are 0000-00-00 and sets them to NULL

Test plan -

Examples of setting up some 0000-00-00 date fields:

e.g. Borrowers table
update borrowers set dateexpiry="0000-00-00" where borrowernumber=49;

e.g. Items table
update items set datelastseen="0000-00-00" where itemnumber=12;

To identify the cases:
1) Set up some 0000-00-00 date fields
2) Run the misc/maintenance/search_for_data_inconsistencies.pl script
3) Run fix_invalid_dates.pl -v

To fix the cases:
1) Run fix_invalid_dates.pl -c -v
2) Notice the value has changed from 0000-00-00 to NULL

e.g. Borrowers table
select dateexpiry from borrowers where borrowernumber=49;

e.g. Items table
select datelastseen from items where itemnumber=12;

Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 70 Nick Clemens (kidclamp) 2024-09-27 17:56:14 UTC
Created attachment 172145 [details] [review]
Bug 31143: (QA follow-up) move script to maintenance

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 71 Nick Clemens (kidclamp) 2024-09-27 17:56:17 UTC
Created attachment 172146 [details] [review]
Bug 31143: (QA follow-up) Tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 72 Katrin Fischer 2024-10-23 17:41:39 UTC
I think given the amount of times we had this issue come up in chat and elsewhere, this will be much appreciated. Thanks for your perseverance Wainui!

One small thing:
I think there is no way to have the script tell you what it does. Could we add some kind of description about the purpose of the script to it?
Comment 73 Katrin Fischer 2024-10-23 18:03:08 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 74 Katrin Fischer 2024-10-24 07:13:00 UTC
After pushing this I realized that Koha doesn't like borrowers.dateexpiry to be NULL :(

The patron auto-complete search dies:
Uncaught TypeError: item.expiry_date is null

The error is the same if you use the auto-complete in the top search or when you transfer a list to another patron (found this while testing bug 30955)

What I did was:

* UPDATE borrowers set dateexpiry = "0000-00-00";
* Ran the script without -c  to check that it would not change data
* Then forgot I had not fixed the data and kept testing things...
* I tested bug 30955, realized the problem and ran it with -c
* auto-complete was still broken with the fixed data
* I manually ran: UPDATE borrowers set dateexpiry = NULL; Still no luck.

We either need to fix the auto-complete patron search or have the script re-calculate the dateexpiry from the patron category. I am leaning towards making the patron search deal with NULL.
Comment 75 Jonathan Druart 2024-10-24 07:19:23 UTC
(In reply to Katrin Fischer from comment #74)
> We either need to fix the auto-complete patron search or have the script
> re-calculate the dateexpiry from the patron category. I am leaning towards
> making the patron search deal with NULL.

borrowers.dateexpiry can be NULL so the code needs to be fixed.

I have not tried it but this would be the idea:

iff --git a/koha-tmpl/intranet-tmpl/prog/js/staff-global.js b/koha-tmpl/intranet-tmpl/prog/js/staff-global.js
index 73a4a0251e1..28060fb35da 100644
--- a/koha-tmpl/intranet-tmpl/prog/js/staff-global.js
+++ b/koha-tmpl/intranet-tmpl/prog/js/staff-global.js
@@ -694,10 +694,13 @@ function patron_autocomplete(node, options) {
         }
         let new_date = new Date();
         let today = Date.parse($date_to_rfc3339($date(new_date.toString())));
-        let expiration = Date.parse(
-            $date_to_rfc3339($date(item.expiry_date.toString()))
-        );
-        let expired = today > expiration;
+        let expired = false;
+        if (item.expiry_date){
+            let expiration = Date.parse(
+                $date_to_rfc3339($date(item.expiry_date.toString()))
+            );
+            expired = today > expiration;
+        }
         return $("<li></li>")
             .addClass(loggedInClass)
             .data("ui-autocomplete-item", item)
Comment 76 Pedro Amorim 2024-10-24 08:23:25 UTC
(In reply to Katrin Fischer from comment #74)
> We either need to fix the auto-complete patron search or have the script
> re-calculate the dateexpiry from the patron category. I am leaning towards
> making the patron search deal with NULL.

We have an installation currently where auto complete is erroring because a couple recently created patrons have updated_on as '0000-00-00'. I'm still investigating the cause (as I can't reproduce) but just adding this information here as well.
Comment 77 Katrin Fischer 2024-10-24 09:14:24 UTC
(In reply to Pedro Amorim from comment #76)
> (In reply to Katrin Fischer from comment #74)
> > We either need to fix the auto-complete patron search or have the script
> > re-calculate the dateexpiry from the patron category. I am leaning towards
> > making the patron search deal with NULL.
> 
> We have an installation currently where auto complete is erroring because a
> couple recently created patrons have updated_on as '0000-00-00'. I'm still
> investigating the cause (as I can't reproduce) but just adding this
> information here as well.

I have filed a new bug for the auto-complete issue:
Bug 38254 - Patron auto-complete search explodes when dateexpiry is NULL
Comment 78 Wainui Witika-Park 2024-10-25 02:41:14 UTC
(In reply to Katrin Fischer from comment #72)
> I think given the amount of times we had this issue come up in chat and
> elsewhere, this will be much appreciated. Thanks for your perseverance
> Wainui! 

Thanks Katrin!!

> One small thing:
> I think there is no way to have the script tell you what it does. Could we
> add some kind of description about the purpose of the script to it?

Are you still wanting me to do this?
Comment 79 Katrin Fischer 2024-10-25 08:06:00 UTC
(In reply to Wainui Witika-Park from comment #78)
> (In reply to Katrin Fischer from comment #72)
> > I think given the amount of times we had this issue come up in chat and
> > elsewhere, this will be much appreciated. Thanks for your perseverance
> > Wainui! 
> 
> Thanks Katrin!!
> 
> > One small thing:
> > I think there is no way to have the script tell you what it does. Could we
> > add some kind of description about the purpose of the script to it?
> 
> Are you still wanting me to do this?

Yes please! I am leaving "additional_work_needed" as a keyword on anything with questions or a follow-up request. We moved the dateexpiry issue to another spot, but I think giving people a good clue about the script who didn't start out at the inconsistencies one would still be helpful.
Comment 80 Pedro Amorim 2024-10-25 15:08:34 UTC
(In reply to Pedro Amorim from comment #76)
> We have an installation currently where auto complete is erroring because a
> couple recently created patrons have updated_on as '0000-00-00'. I'm still
> investigating the cause (as I can't reproduce) but just adding this
> information here as well.

Leaving info here that this relates to bug 36822
Comment 81 Wainui Witika-Park 2024-11-04 05:32:52 UTC
Created attachment 173900 [details] [review]
Bug 31143: (Follow-up) Add description to fix_invalid_dates.pl
Comment 82 Katrin Fischer 2024-11-14 15:38:49 UTC
Sorry Wainui, I had missed the follow-up somehow! Pushed to main now.
Comment 83 Lucas Gass (lukeg) 2024-11-15 16:03:43 UTC
Backported to 24.05.x for upcoming 24.05.06
Comment 84 Katrin Fischer 2024-11-15 16:47:23 UTC
(In reply to Lucas Gass (lukeg) from comment #83)
> Backported to 24.05.x for upcoming 24.05.06

Please be aware of bug 38254 - currently we break auto-complete with the script, although it does the right thing. We need this fixed too.
Comment 85 Lucas Gass (lukeg) 2024-11-15 16:56:41 UTC
> Please be aware of bug 38254 - currently we break auto-complete with the
> script, although it does the right thing. We need this fixed too.

Thanks. Do you think I should revert this for now?
Comment 86 Katrin Fischer 2024-11-15 16:57:22 UTC
(In reply to Lucas Gass (lukeg) from comment #85)
> > Please be aware of bug 38254 - currently we break auto-complete with the
> > script, although it does the right thing. We need this fixed too.
> 
> Thanks. Do you think I should revert this for now?

it's broken patron search vs. broken auto-complete - both are not great. 
I would really prefer a patch for bug 38254 :)
Comment 87 Wainui Witika-Park 2024-12-02 01:56:47 UTC
If anyone else is interested:

We have been setting up the cronjob in cron.daily/koha-custom

#!/bin/sh
sudo koha-foreach --enabled /usr/share/koha/bin/maintenance/fix_invalid_dates.pl -c -v
Comment 88 Fridolin Somers 2024-12-02 08:34:54 UTC
Pushed to 23.11.x for 23.11.11
Comment 89 Aditya Sethi 2024-12-23 09:59:24 UTC
I see nobody's talking about the error when the below line gets executed

my $invalid_dates = $rs->search( { $column => '0000-00-00' } )->count;

This is throwing the error as below:
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Incorrect TIMESTAMP value: '0000-00-00' at ./fix_invalid_dates.pl line 65

I have made a quick fix for this. Please include to the next minor update (after the line:- my $schema = Koha::Database->new->schema;) if the similar is not implemented. 


# Disable strict mode for this session
$schema->storage->dbh_do(
    sub {
        my ($storage, $dbh) = @_;
        $dbh->do("SET SESSION sql_mode = ''");
    }
);
Comment 90 Katrin Fischer 2024-12-23 10:02:28 UTC
(In reply to Aditya from comment #89)
> I see nobody's talking about the error when the below line gets executed
> 
> my $invalid_dates = $rs->search( { $column => '0000-00-00' } )->count;
> 
> This is throwing the error as below:
> DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> execute failed: Incorrect TIMESTAMP value: '0000-00-00' at
> ./fix_invalid_dates.pl line 65
> 
> I have made a quick fix for this. Please include to the next minor update
> (after the line:- my $schema = Koha::Database->new->schema;) if the similar
> is not implemented. 
> 
> 
> # Disable strict mode for this session
> $schema->storage->dbh_do(
>     sub {
>         my ($storage, $dbh) = @_;
>         $dbh->do("SET SESSION sql_mode = ''");
>     }
> );

Hi Aditya, since this is already in a release, we need a new bug number for the fix. Can you please file one and add a test plan? It might depend on DBMS version, so that might be why we didn't see that in testing. I remember we had a similar error. Please also add your DBMS and the version to the report!
Comment 91 Aditya Sethi 2024-12-27 07:45:02 UTC
(In reply to Katrin Fischer from comment #90)
> (In reply to Aditya from comment #89)
> > I see nobody's talking about the error when the below line gets executed
> > 
> > my $invalid_dates = $rs->search( { $column => '0000-00-00' } )->count;
> > 
> > This is throwing the error as below:
> > DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> > execute failed: Incorrect TIMESTAMP value: '0000-00-00' at
> > ./fix_invalid_dates.pl line 65
> > 
> > I have made a quick fix for this. Please include to the next minor update
> > (after the line:- my $schema = Koha::Database->new->schema;) if the similar
> > is not implemented. 
> > 
> > 
> > # Disable strict mode for this session
> > $schema->storage->dbh_do(
> >     sub {
> >         my ($storage, $dbh) = @_;
> >         $dbh->do("SET SESSION sql_mode = ''");
> >     }
> > );
> 
> Hi Aditya, since this is already in a release, we need a new bug number for
> the fix. Can you please file one and add a test plan? It might depend on
> DBMS version, so that might be why we didn't see that in testing. I remember
> we had a similar error. Please also add your DBMS and the version to the
> report!

Please use bug number 38788