Bug 30718

Summary: Use flatpickr's altInput option everywhere
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: TemplatesAssignee: Jonathan Druart <jonathan.druart>
Status: Needs documenting --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: lucas, m.de.rooy, martin.renvoize, nick, oleonard, tomascohen, victor
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30717
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29711
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34231
Change sponsored?: --- Patch complexity: Large patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00
Bug Depends on:    
Bug Blocks: 33821, 30310, 31437, 32111, 33345, 33414, 33611, 33901, 34117    
Attachments: Bug 30718: POC - Use flatpickr's altInput
Bug 30718: Use flatpickr's altInput
Bug 30718: Use flatpickr's altInput
Bug 30718: Pass DT to filter_by_lates
Bug 30718: Fix today's date on overdue
Bug 30718: Fix serials/showpredictionpattern.pl
Bug 30718: Add missing USE KohaDates
Bug 30718: Fix holidays
Bug 30718: Fix scheduler
Bug 30718: Fix time display
Bug 30718: Prevent 'reset' icon to appear twice
Bug 30718: Use flatpickr's altInput
Bug 30718: Pass DT to filter_by_lates
Bug 30718: Fix today's date on overdue
Bug 30718: Fix serials/showpredictionpattern.pl
Bug 30718: Add missing USE KohaDates
Bug 30718: Fix holidays
Bug 30718: Fix scheduler
Bug 30718: Fix time display
Bug 30718: Prevent 'reset' icon to appear twice
Bug 30718: Apply flatpickr changes to OPAC
Bug 30718: Fix dates display at the OPAC
Bug 30718: Use flatpickr's altInput
Bug 30718: Pass DT to filter_by_lates
Bug 30718: Fix today's date on overdue
Bug 30718: Fix serials/showpredictionpattern.pl
Bug 30718: Add missing USE KohaDates
Bug 30718: Fix holidays
Bug 30718: Fix scheduler
Bug 30718: Fix time display
Bug 30718: Prevent 'reset' icon to appear twice
Bug 30718: Apply flatpickr changes to OPAC
Bug 30718: Fix dates display at the OPAC
Bug 30718: Fix renewal with due date
Bug 30588: Fix serial claims filters
Bug 30718: Fix serial claims filters
Bug 30718: Use flatpickr's altInput
Bug 30718: Pass DT to filter_by_lates
Bug 30718: Fix today's date on overdue
Bug 30718: Fix serials/showpredictionpattern.pl
Bug 30718: Add missing USE KohaDates
Bug 30718: Fix holidays
Bug 30718: Fix scheduler
Bug 30718: Fix time display
Bug 30718: Prevent 'reset' icon to appear twice
Bug 30718: Apply flatpickr changes to OPAC
Bug 30718: Fix dates display at the OPAC
Bug 30718: Fix renewal with due date
Bug 30718: Fix serial claims filters
Bug 30718: Use flatpickr's altInput
Bug 30718: Pass DT to filter_by_lates
Bug 30718: Fix today's date on overdue
Bug 30718: Fix serials/showpredictionpattern.pl
Bug 30718: Add missing USE KohaDates
Bug 30718: Fix holidays
Bug 30718: Fix scheduler
Bug 30718: Fix time display
Bug 30718: Prevent 'reset' icon to appear twice
Bug 30718: Apply flatpickr changes to OPAC
Bug 30718: Fix dates display at the OPAC
Bug 30718: Fix renewal with due date
Bug 30718: Fix serial claims filters
Bug 30718: (QA follow-up) Apply prevent duplication on OPAC too
Bug 30718: (QA follow-up) KohaDates filter on inputs
Bug 30718: (QA follow-up) Remove double use KohaDates
Bug 30718: Fix imports
Bug 30718: (follow-up) index_records is called more times than we thought
Bug 30718: Fix curbside pickups

Description Jonathan Druart 2022-05-09 08:36:59 UTC
"Show the user a readable date (as per altFormat), but return
something totally different to the server."

It's actually an option we want to use for all our dates, that will
reduce a lot the overhead in our controllers.
Comment 1 Jonathan Druart 2022-05-31 10:11:12 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2022-05-31 10:14:48 UTC
This will be a pain to implement, but it's worth the hassle.

We are basically going to remove a lot of our dt_from_string, output_pref and $KohaDates occurrences.

$KohaDates will be needed for display purpose only (not in flatpickr's input).
We are going to retrieve the iso-formatted date from the DB, send it to the template, and retrieve the iso-formatted date.
Comment 3 Jonathan Druart 2022-06-16 08:46:07 UTC
Who's willing to test and QA this?
I can spend time on providing a patch, but if we want it into 22.11 it must be pushed early in the dev release cycle!
Comment 4 Katrin Fischer 2022-06-17 20:25:15 UTC
I'm happy to help testing, but it would be nice it could be broken up a bit so templates can be tested in smaller 'chunks'.
Comment 5 Jonathan Druart 2022-06-23 07:13:56 UTC
(In reply to Katrin Fischer from comment #4)
> I'm happy to help testing, but it would be nice it could be broken up a bit
> so templates can be tested in smaller 'chunks'.

I don't think it's a good idea.
The flatpickr's config change is done for all the instances, so we will need to flag the instances that have been fixed to differentiate them from others (and when all fixed, remove the flag?).
There is a lot of occurrences, and I think we can catch easily missing ones with `git grep` (KohaDates from tt/inc and output_pref from pl/pm), but it will be hard if only part of the job is done.
Finally I think we need a moment when devs know that we did the switch, or we will never know if we need to escape using KohaDates or html. Also, I am pretty sure this change will take years if we split it into dozens of reports.
Comment 6 Jonathan Druart 2022-07-11 14:20:51 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2022-07-11 14:24:23 UTC
I think this patch is ready for testing, but it's hard to tell what need to be tested!
Basically it's *any* date manipulation all over Koha that needs to be tested, so... lot of testing!

This patch is great has it is removing 400 lines of ugly and unnecessary date manipulation.
114 files changed, 302 insertions(+), 706 deletions(-)
It makes the code so cleaner!

I won't benchmark but I am expecting (non-significant) perf improvements here.
Comment 8 Jonathan Druart 2022-07-11 14:25:32 UTC
Ha, and, this patch has bugs, for sure! Please help me find them!
Comment 9 Jonathan Druart 2022-07-14 10:33:52 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2022-07-14 10:52:09 UTC
Some places to test:

1. KohaDates is used instead of html, as we moved the formatting to the template (and remove the output_pref(dt_from_string)) call from the pl/pm)
2. Places where we retrieve from flatpickr and resent to the template (sticky due date in circ for instance)
3. REST API - I think I made some changes there as well (but maybe it is on a follow-up bug, cannot remember)
4. Flatpickr needs to be tested
5. there is as well the cataloguing plugin that is using altInput already
6. Lot of changes in the reports area (I haven't tested at all this module)
7. There is a block commented in Koha::Report that certainly needs attention
Comment 11 Owen Leonard 2022-07-14 12:54:09 UTC
I've gone through Acquisitions pages and found only one issue:

- lateorders.pl:
   Can't locate object method "ymd" via package "2010-07-02" (perhaps you forgot to load "2010-07-02"?) at /usr/share/perl5/DateTime/Format/MySQL.pm line 100

In that error "2020-07-02" is the "Estimated delivery date from:" value I was trying to filter on.
Comment 12 Jonathan Druart 2022-07-14 13:24:39 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2022-07-14 13:24:57 UTC
(In reply to Owen Leonard from comment #11)
> - lateorders.pl:
>    Can't locate object method "ymd" via package "2010-07-02" (perhaps you
> forgot to load "2010-07-02"?) at /usr/share/perl5/DateTime/Format/MySQL.pm
> line 100

Fixed, thanks!
Comment 14 Owen Leonard 2022-07-14 16:09:37 UTC
More notes after having gone through almost everything and tested the basics:

- lateorders.pl:
   Can't locate object method "ymd" via package "2010-07-02" (perhaps you forgot to load "2010-07-02"?) at /usr/share/perl5/DateTime/Format/MySQL.pm line 100

- circulation.pl
  Template process failed: undef error - The given date (01.06.2022 11:59 PM) does not match the date format (iso) at /kohadevbox/koha/Koha/DateUtils.pm line 205.
 at /kohadevbox/koha/C4/Templates.pm line 127

The above error occurred when checking out and specifying a date in the past.

- overdues.pl The date in the heading is formatted incorrectly: "1 Item overdue as of 2022-07-14T13:53:00"

- serials/claims.pl: "Template process failed: undef error - : filter not found at /kohadevbox/koha/C4/Templates.pm line 127"

- tools/holidays.pl "Template process failed: undef error - : filter not found at /kohadevbox/koha/C4/Templates.pm line 127"

I noticed this in the log for export.pl after testing a date-limited export. The export completed fine as far as I can tell.

[Thu Jul 14 15:46:05.900437 2022] [cgi:error] [pid 83824] [client 172.18.0.1:62854] AH01215: DBIx::Class::Storage::DBI::_gen_sql_bind(): DateTime objects passed to search() are not supported properly (InflateColumn::DateTime formats and settings are not respected.) See ".. format a DateTime object for searching?" in DBIx::Class::Manual::FAQ. To disable this warning for good set $ENV{DBIC_DT_SEARCH_OK} to true at /kohadevbox/koha/Koha/Objects.pm line 312: /kohadevbox/koha/tools/export.pl, referer: http://127.0.0.1:8081/cgi-bin/koha/tools/export.pl

- Tasks scheduler: "Failed to add Tasks"

I didn't see anything in the error log that appeared to be related to this.
Comment 15 Jonathan Druart 2022-07-18 13:28:17 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2022-07-18 13:28:21 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2022-07-18 13:28:26 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2022-07-18 13:28:30 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2022-07-18 13:28:34 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2022-07-18 13:30:31 UTC
(In reply to Owen Leonard from comment #14)

Thanks a lot, Owen, you are awesome! :)

> More notes after having gone through almost everything and tested the basics:
> 
> - lateorders.pl:
>    Can't locate object method "ymd" via package "2010-07-02" (perhaps you
> forgot to load "2010-07-02"?) at /usr/share/perl5/DateTime/Format/MySQL.pm
> line 100
> 
> - circulation.pl
>   Template process failed: undef error - The given date (01.06.2022 11:59
> PM) does not match the date format (iso) at
> /kohadevbox/koha/Koha/DateUtils.pm line 205.
>  at /kohadevbox/koha/C4/Templates.pm line 127
> 
> The above error occurred when checking out and specifying a date in the past.
> 
> - overdues.pl The date in the heading is formatted incorrectly: "1 Item
> overdue as of 2022-07-14T13:53:00"
> 
> - serials/claims.pl: "Template process failed: undef error - : filter not
> found at /kohadevbox/koha/C4/Templates.pm line 127"
> 
> - tools/holidays.pl "Template process failed: undef error - : filter not
> found at /kohadevbox/koha/C4/Templates.pm line 127"

All fixed!

> I noticed this in the log for export.pl after testing a date-limited export.
> The export completed fine as far as I can tell.
> 
> [Thu Jul 14 15:46:05.900437 2022] [cgi:error] [pid 83824] [client
> 172.18.0.1:62854] AH01215: DBIx::Class::Storage::DBI::_gen_sql_bind():
> DateTime objects passed to search() are not supported properly
> (InflateColumn::DateTime formats and settings are not respected.) See "..
> format a DateTime object for searching?" in DBIx::Class::Manual::FAQ. To
> disable this warning for good set $ENV{DBIC_DT_SEARCH_OK} to true at
> /kohadevbox/koha/Koha/Objects.pm line 312: /kohadevbox/koha/tools/export.pl,
> referer: http://127.0.0.1:8081/cgi-bin/koha/tools/export.pl

Also in master, should be fixed (but reported separately).

> - Tasks scheduler: "Failed to add Tasks"
> 
> I didn't see anything in the error log that appeared to be related to this.

Fixed (note that time part is mandatory but the UI does not ask for it).
Comment 21 Jonathan Druart 2022-07-18 13:36:43 UTC
Follow-ups can be squashed of course
Comment 22 Owen Leonard 2022-07-18 16:09:25 UTC
> > - circulation.pl
> >   Template process failed: undef error - The given date (01.06.2022 11:59
> > PM) does not match the date format (iso) at
> >
> > The above error occurred when checking out and specifying a date in the past.

I'm still getting this error when manually selecting a due date in the past.

I now notice that the time portion of the datetime is not appearing in the input field. The same is true on the checkin and offline circulation page.

> > - tools/holidays.pl

There's now a little calendar icon showing above the calendar on this page.

> > - Tasks scheduler: "Failed to add Tasks"
> Fixed (note that time part is mandatory but the UI does not ask for it).

Would it make sense to update the form in this patch set to use a single field with the Flatpickr timepicker enabled?

I notice also that paired date fields (start date & end date) now show an extra "X" icon after the second field, e.g. serial claims or catalog statistics.
Comment 23 Jonathan Druart 2022-07-19 12:37:55 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2022-07-19 12:38:00 UTC Comment hidden (obsolete)
Comment 25 Jonathan Druart 2022-07-19 12:39:13 UTC
(In reply to Owen Leonard from comment #22)
> > > - circulation.pl
> > >   Template process failed: undef error - The given date (01.06.2022 11:59
> > > PM) does not match the date format (iso) at
> > >
> > > The above error occurred when checking out and specifying a date in the past.
> 
> I'm still getting this error when manually selecting a due date in the past.
> 
> I now notice that the time portion of the datetime is not appearing in the
> input field. The same is true on the checkin and offline circulation page.

This was hiding something critical, thanks for the catch.

> > > - tools/holidays.pl
> 
> There's now a little calendar icon showing above the calendar on this page.

Yes, we don't need it there, fixed.

> > > - Tasks scheduler: "Failed to add Tasks"
> > Fixed (note that time part is mandatory but the UI does not ask for it).
> 
> Would it make sense to update the form in this patch set to use a single
> field with the Flatpickr timepicker enabled?

I would consider that different from what we are doing here. Should be fixed separately in my opinion.

> I notice also that paired date fields (start date & end date) now show an
> extra "X" icon after the second field, e.g. serial claims or catalog
> statistics.

Fixed.
Comment 26 Victor Grousset/tuxayo 2022-08-05 02:35:06 UTC
Conflicts, if you don't have time, I can give a try before Monday.
Comment 27 Jonathan Druart 2022-08-05 07:22:22 UTC Comment hidden (obsolete)
Comment 28 Jonathan Druart 2022-08-05 07:22:26 UTC Comment hidden (obsolete)
Comment 29 Jonathan Druart 2022-08-05 07:22:31 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2022-08-05 07:22:35 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2022-08-05 07:22:40 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2022-08-05 07:22:45 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2022-08-05 07:22:49 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2022-08-05 07:22:54 UTC Comment hidden (obsolete)
Comment 35 Jonathan Druart 2022-08-05 07:22:58 UTC Comment hidden (obsolete)
Comment 36 Marcel de Rooy 2022-08-05 07:32:53 UTC
Having a look here right now
Comment 37 Marcel de Rooy 2022-08-05 07:39:28 UTC
testing 9 commit(s) (applied to e544ff4 '45 Bug 29632: (QA follow-up) Fix COMM')

Processing files before patches
|========================>| 118 / 118 (100.00%)
Processing files after patches
|========================>| 118 / 118 (100.00%)

No failures from qa tools
Comment 38 Marcel de Rooy 2022-08-05 08:36:00 UTC
Minor code observations:

Koha/Hold.pm
+    $date &&= dt_from_string($date)->truncate( to => 'day' )->datetime;
Obsure code

clubs/clubs-add-modify.pl
+my $date_start = $cgi->param('date_start') || undef;
Does this || undef really serve any purpose here?
Not the only occurrence btw.
Comment 39 Marcel de Rooy 2022-08-05 08:42:48 UTC
Editing items: Should $w Price effective from be controlled by flatpickr ?
Comment 40 Marcel de Rooy 2022-08-05 08:46:31 UTC
Hmm. I am just globally testing a bit. But this does not provide confidence:

Adding a expiration date for a new hold.
The date does not get saved.
Comment 41 Jonathan Druart 2022-08-05 08:48:24 UTC
(In reply to Marcel de Rooy from comment #38)
> Minor code observations:
> 
> Koha/Hold.pm
> +    $date &&= dt_from_string($date)->truncate( to => 'day' )->datetime;
> Obsure code

Obscure? It's using a basic perl operator...

Prefer the verbose if version

if ( $date ) {
     $date = dt_from_string($date)->truncate( to => 'day' )->datetime;
}
or the ugly ternary?
$date = $date ? dt_from_string($date)->truncate( to => 'day' )->datetime : undef

I don't.

> clubs/clubs-add-modify.pl
> +my $date_start = $cgi->param('date_start') || undef;
> Does this || undef really serve any purpose here?
> Not the only occurrence btw.

I don't know, but trying to prevent regressions by keeping the exact same behaviours.

(In reply to Marcel de Rooy from comment #39)
> Editing items: Should $w Price effective from be controlled by flatpickr ?

Isn't it already?
Comment 42 Marcel de Rooy 2022-08-05 08:50:08 UTC
Allowing future hold dates:

|         77 |             51 | 0000-00-00  |            1 | CPL        |    NULL | NULL             | NULL         | NULL             | NULL                |              |        1 | NULL  | 2022-08-05 08:48:48 |       NULL | NULL        | 0000-00-00     | 0000-00-00             |              0 |       0 | NULL          | NULL     |               0 |            0 |

I am getting 0000-00-00 dates in now. Three in a row . Hmm
Comment 43 Jonathan Druart 2022-08-05 08:52:18 UTC
(In reply to Marcel de Rooy from comment #40)
> Hmm. I am just globally testing a bit. But this does not provide confidence:
> 
> Adding a expiration date for a new hold.
> The date does not get saved.

It does for me, can you provide more detail maybe?

/cgi-bin/koha/reserve/request.pl?biblionumber=117
search edna
select a date in "Hold expires on date:"
place hold
=> Date is in the expiration column
Comment 44 Marcel de Rooy 2022-08-05 08:53:10 UTC
(In reply to Jonathan Druart from comment #43)
> (In reply to Marcel de Rooy from comment #40)
> > Hmm. I am just globally testing a bit. But this does not provide confidence:
> > 
> > Adding a expiration date for a new hold.
> > The date does not get saved.
> 
> It does for me, can you provide more detail maybe?
> 
> /cgi-bin/koha/reserve/request.pl?biblionumber=117
> search edna
> select a date in "Hold expires on date:"
> place hold
> => Date is in the expiration column

I am testing now under dd/mm/yy.
Could that be the cause ?
Comment 45 Marcel de Rooy 2022-08-05 08:54:41 UTC
No with mm/dd/yy I still have a new hold without two dates (hold start and end date entered).
Comment 46 Marcel de Rooy 2022-08-05 08:57:11 UTC
Hey. When I switch to yy-mm-dd only THEN the hold dates are saved !
Comment 47 Marcel de Rooy 2022-08-05 08:57:31 UTC
|         79 |             51 | 2022-08-08  |            1 | CPL        |    NULL | NULL             | NULL         | NULL             | NULL                |              |        1 | NULL  | 2022-08-05 08:56:43 |       NULL | NULL        | 2022-08-15     | 2022-08-15             |              0 |       0 | NULL          | NULL     |               0 |            0 |
Comment 48 Marcel de Rooy 2022-08-05 09:01:34 UTC
Well I give up. Code looks good, nice improvement. But it does not work for me on holds. That is not providing confidence.
Have some more people test it.
Try adding a hold with start and end date in various dateformat settings.

Will not set FQA but would be inclined to..
Comment 49 Jonathan Druart 2022-08-05 09:11:00 UTC Comment hidden (obsolete)
Comment 50 Jonathan Druart 2022-08-05 09:14:04 UTC Comment hidden (obsolete)
Comment 51 Jonathan Druart 2022-08-05 09:15:43 UTC
(In reply to Marcel de Rooy from comment #48)
> Well I give up. Code looks good, nice improvement. But it does not work for
> me on holds. That is not providing confidence.
> Have some more people test it.
> Try adding a hold with start and end date in various dateformat settings.
> 
> Will not set FQA but would be inclined to..

The flatpick's options hadn't been updated at the OPAC. Please try with the last two patches applied.
Comment 52 Victor Grousset/tuxayo 2022-08-07 15:10:33 UTC
Created attachment 138748 [details] [review]
Bug 30718: Use flatpickr's altInput

The idea rely on the KohaDates TT plugin for the date formatting. We
should not have any output_pref calls in pl or pm (there are some
exceptions, for ILSDI for instance).

Also flatpickr will deal with the places where dates are inputed. We
will pass the raw SQL value (what we call 'iso' in Koha::DateUtils), and
the controller will receive the same value, no need to additional
conversion.
Note that DBIC has the capability to auto-deflate DateTime objects,
which makes things way easier. We can either pass the value we receive
from the controller, or pass a DT object to our methods.
Comment 53 Victor Grousset/tuxayo 2022-08-07 15:10:47 UTC
Created attachment 138749 [details] [review]
Bug 30718: Pass DT to filter_by_lates
Comment 54 Victor Grousset/tuxayo 2022-08-07 15:11:13 UTC
Created attachment 138750 [details] [review]
Bug 30718: Fix today's date on overdue
Comment 55 Victor Grousset/tuxayo 2022-08-07 15:11:26 UTC
Created attachment 138751 [details] [review]
Bug 30718: Fix serials/showpredictionpattern.pl

Can't locate object method "ymd" via package "2022-02-01" (perhaps you forgot to load "2022-02-01"?) at /kohadevbox/koha/serials/showpredictionpattern.pl line 100
eval {...} at /kohadevbox/koha/serials/showpredictionpattern.pl line 2
Comment 56 Victor Grousset/tuxayo 2022-08-07 15:11:40 UTC
Created attachment 138752 [details] [review]
Bug 30718: Add missing USE KohaDates
Comment 57 Victor Grousset/tuxayo 2022-08-07 15:11:52 UTC
Created attachment 138753 [details] [review]
Bug 30718: Fix holidays

Template process failed: undef error - The given date (07/18/2022) does not match the date format (iso)
Comment 58 Victor Grousset/tuxayo 2022-08-07 15:12:10 UTC
Created attachment 138754 [details] [review]
Bug 30718: Fix scheduler
Comment 59 Victor Grousset/tuxayo 2022-08-07 15:12:23 UTC
Created attachment 138755 [details] [review]
Bug 30718: Fix time display

Especifically when timeformat=12h
Comment 60 Victor Grousset/tuxayo 2022-08-07 15:12:35 UTC
Created attachment 138756 [details] [review]
Bug 30718: Prevent 'reset' icon to appear twice
Comment 61 Victor Grousset/tuxayo 2022-08-07 15:12:55 UTC
Created attachment 138757 [details] [review]
Bug 30718: Apply flatpickr changes to OPAC
Comment 62 Victor Grousset/tuxayo 2022-08-07 15:13:11 UTC
Created attachment 138758 [details] [review]
Bug 30718: Fix dates display at the OPAC

Found dateofbirth so far, do we have others?
Comment 63 Victor Grousset/tuxayo 2022-08-07 15:15:27 UTC
Another rebase.
Comment 64 Victor Grousset/tuxayo 2022-08-07 22:03:30 UTC
tested:
- checkout with date in the past
- hold expiration date with all formats, staff and OPAC
- making date changes in patron details
- create a report and schedule it
  - issue found: the picker doesn't let pick today as a date
- check-in with custom date
- logs view (date from and date to)

---

(In reply to Marcel de Rooy from comment #48)
> Try adding a hold with start and end date in various dateformat settings.

What is hold start date? The reservedate column, is there an input for that?
I my testing, reservedate along with expirationdate and patron_expiration_date always have values.
Comment 65 Jonathan Druart 2022-08-08 08:09:59 UTC
(In reply to Victor Grousset/tuxayo from comment #64)
> tested:
> - checkout with date in the past
> - hold expiration date with all formats, staff and OPAC
> - making date changes in patron details
> - create a report and schedule it
>   - issue found: the picker doesn't let pick today as a date

This is the behaviour in master.
Comment 66 Victor Grousset/tuxayo 2022-08-09 02:14:02 UTC
tested:
- order: field "d - Date acquired" (dateaccessioned)
- receive shipment: shipmentdate field
- receive order: datereceived field
- finish receiving: Shipment date and Billing date
- Budgets administration > edit budget > start and end dates
- patron categories: Enrollment period
- batch check out due date
- circulation: overdues: from and to dates
- holds to pull: start and end dates
- renew: Renewal due date
  - issue found, with the patches, I don't get the "Item renewed" message box.
    It doesn't renew, I just get back to the same page.
    Without inputting a date, it works.
    logs:
    Invalid date passed to AddRenewal. at /kohadevbox/koha/circ/renew.pl line 101.
Comment 67 Jonathan Druart 2022-08-09 06:45:08 UTC
Created attachment 138879 [details] [review]
Bug 30718: Fix renewal with due date

This was a test added by the following commit:

commit 99eccc18ed4a21fa416a3d61fed3f70825dc2203
Date:   Thu Jun 16 10:10:09 2011 +0100
    Bug 5549 : Handle datetimes on return

It's no longer needed now, we can pass a DateTime or an ISO-formatted
date
Comment 68 Jonathan Druart 2022-08-09 06:45:22 UTC
(In reply to Victor Grousset/tuxayo from comment #66)
> - renew: Renewal due date
>   - issue found, with the patches, I don't get the "Item renewed" message
> box.
>     It doesn't renew, I just get back to the same page.
>     Without inputting a date, it works.
>     logs:
>     Invalid date passed to AddRenewal. at /kohadevbox/koha/circ/renew.pl
> line 101.

Fixed in the last commit.
Comment 69 Victor Grousset/tuxayo 2022-08-09 21:36:39 UTC
Thanks Jonathan, it works :)

tested:
- renew: Renewal due date
- check in: custom return date
- club: start and end date
- Reports Cash > register statistics: from and to dates
- Serials > Check expiration: Expiring before date
- serials > claims: from and to filters
  - issue found: with the patches, any date range gives empty results.
    While without, it works as expected.
Comment 70 Jonathan Druart 2022-08-10 13:20:43 UTC
Created attachment 138980 [details] [review]
Bug 30588: Fix serial claims filters
Comment 71 Victor Grousset/tuxayo 2022-08-12 00:36:28 UTC
Created attachment 139033 [details] [review]
Bug 30718: Fix serial claims filters
Comment 72 Victor Grousset/tuxayo 2022-08-12 00:37:21 UTC
fixed ticket number for patch "Fix serial claims filters"

tested:
- serials > claims: from and to filters
- serials search: Expires before field in only-search-form view and result view (search form on the left)
- serials: new subscription: First issue publication date + Subscription start date + 
Subscription end date
- suggestion list: "Suggestion information" the 3 date filters
- Tools > Tags: date from and to

continuing
Comment 73 Victor Grousset/tuxayo 2022-08-12 21:20:40 UTC
Created attachment 139073 [details] [review]
Bug 30718: Use flatpickr's altInput

The idea rely on the KohaDates TT plugin for the date formatting. We
should not have any output_pref calls in pl or pm (there are some
exceptions, for ILSDI for instance).

Also flatpickr will deal with the places where dates are inputed. We
will pass the raw SQL value (what we call 'iso' in Koha::DateUtils), and
the controller will receive the same value, no need to additional
conversion.
Note that DBIC has the capability to auto-deflate DateTime objects,
which makes things way easier. We can either pass the value we receive
from the controller, or pass a DT object to our methods.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 74 Victor Grousset/tuxayo 2022-08-12 21:20:45 UTC
Created attachment 139074 [details] [review]
Bug 30718: Pass DT to filter_by_lates

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 75 Victor Grousset/tuxayo 2022-08-12 21:20:51 UTC
Created attachment 139075 [details] [review]
Bug 30718: Fix today's date on overdue

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 76 Victor Grousset/tuxayo 2022-08-12 21:20:59 UTC
Created attachment 139076 [details] [review]
Bug 30718: Fix serials/showpredictionpattern.pl

Can't locate object method "ymd" via package "2022-02-01" (perhaps you forgot to load "2022-02-01"?) at /kohadevbox/koha/serials/showpredictionpattern.pl line 100
eval {...} at /kohadevbox/koha/serials/showpredictionpattern.pl line 2

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 77 Victor Grousset/tuxayo 2022-08-12 21:21:06 UTC
Created attachment 139077 [details] [review]
Bug 30718: Add missing USE KohaDates

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 78 Victor Grousset/tuxayo 2022-08-12 21:21:12 UTC
Created attachment 139078 [details] [review]
Bug 30718: Fix holidays

Template process failed: undef error - The given date (07/18/2022) does not match the date format (iso)

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 79 Victor Grousset/tuxayo 2022-08-12 21:21:19 UTC
Created attachment 139079 [details] [review]
Bug 30718: Fix scheduler

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 80 Victor Grousset/tuxayo 2022-08-12 21:21:25 UTC
Created attachment 139080 [details] [review]
Bug 30718: Fix time display

Especifically when timeformat=12h

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 81 Victor Grousset/tuxayo 2022-08-12 21:21:31 UTC
Created attachment 139081 [details] [review]
Bug 30718: Prevent 'reset' icon to appear twice

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 82 Victor Grousset/tuxayo 2022-08-12 21:21:36 UTC
Created attachment 139082 [details] [review]
Bug 30718: Apply flatpickr changes to OPAC

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 83 Victor Grousset/tuxayo 2022-08-12 21:21:42 UTC
Created attachment 139083 [details] [review]
Bug 30718: Fix dates display at the OPAC

Found dateofbirth so far, do we have others?

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 84 Victor Grousset/tuxayo 2022-08-12 21:21:48 UTC
Created attachment 139084 [details] [review]
Bug 30718: Fix renewal with due date

This was a test added by the following commit:

commit 99eccc18ed4a21fa416a3d61fed3f70825dc2203
Date:   Thu Jun 16 10:10:09 2011 +0100
    Bug 5549 : Handle datetimes on return

It's no longer needed now, we can pass a DateTime or an ISO-formatted
date

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 85 Victor Grousset/tuxayo 2022-08-12 21:21:54 UTC
Created attachment 139085 [details] [review]
Bug 30718: Fix serial claims filters

https://bugs.koha-community.org/show_bug.cgi?id=30718
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 86 Victor Grousset/tuxayo 2022-08-12 21:23:01 UTC
tested: (continuation of previous comments)
- tools > news: new entry: Publication date and Expiration date
- calendar: add new holiday: To date field
- patron import: Registration date, Date of birth, Expiry date
- inventory: "Set inventory date to" value isn't honored even without the patches. datelastseen is always set to today.
- inventory: "Last inventory date" filter
- opac: Your personal details: date of birth
- offline circulation, can't test because of bug 31358

That should be it, after going through all the changed pages (listing the .tt from the patches) to try to find all the date inputs there and checking that they behave correctly, it looks good to move on :)
Comment 87 Marcel de Rooy 2022-08-18 09:30:32 UTC
QA scheduled for 19-08-22 :)
Comment 88 Marcel de Rooy 2022-08-19 08:19:10 UTC
Resuming QA
Comment 89 Marcel de Rooy 2022-08-19 08:23:14 UTC
Fixed small rebase conflict on first patch for opac-reserve
Comment 90 Marcel de Rooy 2022-08-19 08:30:56 UTC
Still no QA tools complaints for 119 files now.
Comment 91 Marcel de Rooy 2022-08-19 09:42:37 UTC
(In reply to Victor Grousset/tuxayo from comment #85)
> Created attachment 139085 [details] [review] [review]
> Bug 30718: Fix serial claims filters
> 
> https://bugs.koha-community.org/show_bug.cgi?id=30718
> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

This patch does:
-            var beginDate = Date_from_syspref($("#from").val()).getTime();
-            var endDate   = Date_from_syspref($("#to").val()).getTime();
+            var beginDate = new Date($("#from").val()).getTime();
+            var endDate   = new Date($("#to").val()).getTime();

Still seeing in its context:
                // We make a JS Date Object, according to the locale
                var pdate = Date_from_syspref($(this).text()).getTime();

And it seems the only real remains of this function.
So it probably should be changed too. Not tested however.
While finishing up, leaving it here as TODO.
Comment 92 Marcel de Rooy 2022-08-19 09:43:13 UTC
git grep Date_from_syspref

koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc:                    original_date = Date_from_syspref( original_date ).getTime();
koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt:                var pdate = Date_from_syspref($(this).text()).getTime();
koha-tmpl/intranet-tmpl/prog/js/calendar.js:function Date_from_syspref(dstring) {
koha-tmpl/intranet-tmpl/prog/js/calendar.js:        alert("Date_from_syspref(" + dstring + ") splits to:\n" + dateX.join("\n"));
koha-tmpl/intranet-tmpl/prog/js/calendar.js:    var datetime = Date_from_syspref(date);
koha-tmpl/intranet-tmpl/prog/js/calendar.js:        var from = Date_from_syspref($(params).val());
koha-tmpl/intranet-tmpl/prog/js/calendar.js:        var to = Date_from_syspref(value);
koha-tmpl/intranet-tmpl/prog/js/calendar.js:        var from = Date_from_syspref($(params).val());
koha-tmpl/intranet-tmpl/prog/js/calendar.js:        var to = Date_from_syspref(value);
Comment 93 Marcel de Rooy 2022-08-19 09:44:07 UTC
Created attachment 139453 [details] [review]
Bug 30718: Use flatpickr's altInput

The idea rely on the KohaDates TT plugin for the date formatting. We
should not have any output_pref calls in pl or pm (there are some
exceptions, for ILSDI for instance).

Also flatpickr will deal with the places where dates are inputed. We
will pass the raw SQL value (what we call 'iso' in Koha::DateUtils), and
the controller will receive the same value, no need to additional
conversion.
Note that DBIC has the capability to auto-deflate DateTime objects,
which makes things way easier. We can either pass the value we receive
from the controller, or pass a DT object to our methods.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 94 Marcel de Rooy 2022-08-19 09:44:11 UTC
Created attachment 139454 [details] [review]
Bug 30718: Pass DT to filter_by_lates

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 95 Marcel de Rooy 2022-08-19 09:44:15 UTC
Created attachment 139455 [details] [review]
Bug 30718: Fix today's date on overdue

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 96 Marcel de Rooy 2022-08-19 09:44:20 UTC
Created attachment 139456 [details] [review]
Bug 30718: Fix serials/showpredictionpattern.pl

Can't locate object method "ymd" via package "2022-02-01" (perhaps you forgot to load "2022-02-01"?) at /kohadevbox/koha/serials/showpredictionpattern.pl line 100
eval {...} at /kohadevbox/koha/serials/showpredictionpattern.pl line 2

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 97 Marcel de Rooy 2022-08-19 09:44:25 UTC
Created attachment 139457 [details] [review]
Bug 30718: Add missing USE KohaDates

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 98 Marcel de Rooy 2022-08-19 09:44:29 UTC
Created attachment 139458 [details] [review]
Bug 30718: Fix holidays

Template process failed: undef error - The given date (07/18/2022) does not match the date format (iso)

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 99 Marcel de Rooy 2022-08-19 09:44:34 UTC
Created attachment 139459 [details] [review]
Bug 30718: Fix scheduler

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 100 Marcel de Rooy 2022-08-19 09:44:39 UTC
Created attachment 139460 [details] [review]
Bug 30718: Fix time display

Especifically when timeformat=12h

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 101 Marcel de Rooy 2022-08-19 09:44:43 UTC
Created attachment 139461 [details] [review]
Bug 30718: Prevent 'reset' icon to appear twice

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 102 Marcel de Rooy 2022-08-19 09:44:48 UTC
Created attachment 139462 [details] [review]
Bug 30718: Apply flatpickr changes to OPAC

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 103 Marcel de Rooy 2022-08-19 09:44:53 UTC
Created attachment 139463 [details] [review]
Bug 30718: Fix dates display at the OPAC

Found dateofbirth so far, do we have others?

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 104 Marcel de Rooy 2022-08-19 09:44:58 UTC
Created attachment 139464 [details] [review]
Bug 30718: Fix renewal with due date

This was a test added by the following commit:

commit 99eccc18ed4a21fa416a3d61fed3f70825dc2203
Date:   Thu Jun 16 10:10:09 2011 +0100
    Bug 5549 : Handle datetimes on return

It's no longer needed now, we can pass a DateTime or an ISO-formatted
date

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 105 Marcel de Rooy 2022-08-19 09:45:03 UTC
Created attachment 139465 [details] [review]
Bug 30718: Fix serial claims filters

https://bugs.koha-community.org/show_bug.cgi?id=30718
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 106 Marcel de Rooy 2022-08-19 09:45:08 UTC
Created attachment 139466 [details] [review]
Bug 30718: (QA follow-up) Apply prevent duplication on OPAC too

See former intranet side patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 107 Marcel de Rooy 2022-08-19 09:45:13 UTC
Created attachment 139467 [details] [review]
Bug 30718: (QA follow-up) KohaDates filter on inputs

Found another one on a visible input. Replaced with html filter.
koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep2.tt:                                            <input type="text" class="enrolmentperioddate" name="enrolmentperioddate" id="enrolmentperioddate" value="[% category.enrolmentperioddate | $KohaDates %]" />

But we also have a few hidden ones. First verified that cleanborrowers did not work anymore with expirydate.
Fixed it with passing iso dateformat to resolve.
intranet-tmpl/prog/en/modules/acqui/parcels.tt:            <input type="hidden" name="shipmentdate" value="[% shipmentdate | $KohaDates %]" />
intranet-tmpl/prog/en/modules/serials/subscription-add.tt:                                          <input type="hidden" id="acqui_date" name="firstacquidate" value="[% firstacquidate | $KohaDates %]"/>
intranet-tmpl/prog/en/modules/tools/batch_extend_due_dates.tt:                                    <input type="hidden" name="new_hard_due_date" value="[% new_hard_due_date | $KohaDates %]" />
intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt:            <input type="hidden" name="not_borrowed_since" value="[% not_borrowed_since | $KohaDates %]" />
intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt:            <input type="hidden" name="last_issue_date" value="[% last_issue_date | $KohaDates %]" />
intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt:            <input type="hidden" name="borrower_dateexpiry" value="[% borrower_dateexpiry | $KohaDates %]" />
intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt:                <input type="hidden" name="borrower_lastseen" value="[% borrower_lastseen | $KohaDates %]" />

Test plan:
Try cleanborrowers with expiry date.
cd koha-tmpl; git grep "<input.*\$KohaDates" | grep -v iso

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 108 Marcel de Rooy 2022-08-19 09:45:18 UTC
Created attachment 139468 [details] [review]
Bug 30718: (QA follow-up) Remove double use KohaDates

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 109 Marcel de Rooy 2022-08-19 09:47:51 UTC
Ok Tomas, go ahead and push it. We will find the remaining things. Victor did a great job in testing too. Thx Jonathan.
Comment 110 Katrin Fischer 2022-08-19 09:53:33 UTC
Does this touch any of the coding guidelines or should we have a new one for how we want things to be done?
Comment 111 Tomás Cohen Arazi 2022-08-19 11:54:11 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 112 Tomás Cohen Arazi 2022-08-19 13:12:44 UTC
Created attachment 139490 [details] [review]
Bug 30718: Fix imports

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 113 Katrin Fischer 2022-08-21 11:52:21 UTC
*ping*

(In reply to Katrin Fischer from comment #110)
> Does this touch any of the coding guidelines or should we have a new one for
> how we want things to be done?
Comment 114 Tomás Cohen Arazi 2022-08-22 14:20:22 UTC
Created attachment 139605 [details] [review]
Bug 30718: (follow-up) index_records is called more times than we thought

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 115 Marcel de Rooy 2022-08-23 11:15:05 UTC
(In reply to Tomás Cohen Arazi from comment #114)
> Created attachment 139605 [details] [review] [review]
> Bug 30718: (follow-up) index_records is called more times than we thought
> 
> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Is there any connection with this report ?
Comment 116 Tomás Cohen Arazi 2022-08-23 11:25:03 UTC
(In reply to Marcel de Rooy from comment #115)
> (In reply to Tomás Cohen Arazi from comment #114)
> > Created attachment 139605 [details] [review] [review] [review]
> > Bug 30718: (follow-up) index_records is called more times than we thought
> > 
> > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
> 
> Is there any connection with this report ?

I couldn't find the reason, but git bisect Matthews this bug as responsible.
Comment 117 Lucas Gass 2022-10-14 19:12:31 UTC
Enhancement will not be backported to 22.05.x
Comment 118 Jonathan Druart 2022-10-21 09:57:20 UTC
(This is so AWESOME!)
Comment 119 Jonathan Druart 2022-11-04 11:14:19 UTC
Created attachment 143175 [details] [review]
Bug 30718: Fix curbside pickups

No slots are displayed without this patch
Comment 120 Tomás Cohen Arazi 2022-11-04 11:29:57 UTC
Follow-up pushed. Thanks, Jonathan!