Bug 30927 - Improve formatting or iCal files for checkout due dates
Summary: Improve formatting or iCal files for checkout due dates
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords: release-notes-needed
: 18563 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-06-08 17:33 UTC by Kyle M Hall
Modified: 2023-09-15 20:38 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00


Attachments
Bug 30927 - Improve formatting or iCal files for checkout due dates (2.93 KB, patch)
2022-06-08 17:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30927 - Improve formatting or iCal files for checkout due dates (2.99 KB, patch)
2022-06-09 15:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Screenshot of how events were added to Calendar on macOS (33.20 KB, image/png)
2022-06-09 17:34 UTC, Owen Leonard
Details
Bug 30927 - Improve formatting or iCal files for checkout due dates (3.31 KB, patch)
2022-06-13 17:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30927 - Improve formatting or iCal files for checkout due dates (3.35 KB, patch)
2022-06-13 18:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Screenshot from Kyle's testing (673.58 KB, image/png)
2022-06-14 13:57 UTC, Kyle M Hall
Details
ICS file from Kyle's testing (563 bytes, text/calendar)
2022-06-14 13:58 UTC, Kyle M Hall
Details
Bug 30927 - Improve formatting or iCal files for checkout due dates (3.53 KB, patch)
2022-06-14 14:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
ICS file from Kyle's testing (1.30 KB, text/calendar)
2022-06-14 15:30 UTC, Kyle M Hall
Details
Screenshot from Kyle's testing 2 (802.10 KB, image/png)
2022-06-14 15:32 UTC, Kyle M Hall
Details
Bug 30927 - Improve formatting or iCal files for checkout due dates (3.58 KB, patch)
2022-06-14 18:21 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30927 - Improve formatting or iCal files for checkout due dates (3.56 KB, patch)
2022-06-15 12:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 30927 - Improve formatting or iCal files for checkout due dates (3.61 KB, patch)
2022-06-15 14:36 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30927: Improve formatting or iCal files for checkout due dates (3.69 KB, patch)
2022-08-09 09:55 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2022-06-08 17:33:45 UTC
We got some feedback from a patron regarding the default notifications on the iCal export for the patron checkout summary in the OPAC. The notifications send 1 minute before the event (the due date), which is midnight.

It makes more sense for:
A) Checkouts due at midnight to be "All day events"
B) Checkouts due at specific time to start in the morning, and end at the time due.
Comment 1 Kyle M Hall 2022-06-08 17:36:55 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2022-06-08 17:48:03 UTC
I'm getting this error when I click on the iCal link:

Can't call method "hour" on an undefined value at /kohadevbox/koha/opac/opac-ics.pl line 88
Comment 3 Kyle M Hall 2022-06-09 15:56:57 UTC
Created attachment 135907 [details] [review]
Bug 30927 - Improve formatting or iCal files for checkout due dates

We got some feedback from a patron regarding the default notifications on the iCal export for the patron checkout summary in the OPAC. The notifications send 1 minute before the event (the due date), which is midnight.

It makes more sense for:
A) Checkouts due at midnight to be "All day events"
B) Checkouts due at specific time to start in the morning, and end at the time due.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Make a checkout due at 23:59
4) Make a checkout due at 11:00
5) Download the iCal file from the OPAC
6) Import this file into Calendar
7) Note the first checkout is an all day event
8) Note the second checkout starts in the morning and ends at the due time
Comment 4 Kyle M Hall 2022-06-09 15:57:33 UTC
(In reply to Owen Leonard from comment #2)
> I'm getting this error when I click on the iCal link:
> 
> Can't call method "hour" on an undefined value at
> /kohadevbox/koha/opac/opac-ics.pl line 88

Thanks for testing Owen! I think this new version of the patch should fix that.
Comment 5 Owen Leonard 2022-06-09 17:34:42 UTC
Created attachment 135910 [details]
Screenshot of how events were added to Calendar on macOS

The attached screenshot shows how the iCal entries look in my Calendar app. "The Silver Lotus" is due at 8PM on 2022-06-23. "Up!" is due at no specific time (23:59:59).

My calendar thinks the 8PM event spans 24 hours. Google Calendar seems to do the same.

I think what we want is for those entries to span no time at all: Starting time at 8PM and ending time at 8PM on the same day.
Comment 6 Aaron Sakovich 2022-06-10 21:39:13 UTC
RFC 5545 https://datatracker.ietf.org/doc/html/rfc5545 on page 54 (section 3.6.1 Event Component) says the following:

>      ... For cases where a "VEVENT" calendar component
>      specifies a "DTSTART" property with a DATE value type but no
>      "DTEND" nor "DURATION" property, the event's duration is taken to
>      be one day.  For cases where a "VEVENT" calendar component
>      specifies a "DTSTART" property with a DATE-TIME value type but no
>      "DTEND" property, the event ends on the same calendar date and
>      time of day specified by the "DTSTART" property.

What if the DTSTART is a DATE or DATE-TIME, and DTEND and DURATION are left blank? We could set it to be a DATE if the due time is 23:59, and a DATE-TIME otherwise.

That would result in either an all day event as desired per option A) (when due at midnight) or an event with a specific due time and the end time set to the end of the day, the inverse of Option B) (which might be a more palatable solution as event reminders are typical set based on the start time of an event). 

Alternatively, a DURATION could be specified in the latter case, but note that DURATION (section 3.8.2.5) is a _positive_ value, and no mention is made in the spec of a zero duration, so I'd be hesitant to reference a zero-value duration.
Comment 7 Aaron Sakovich 2022-06-10 21:46:16 UTC
Sigh, that didn't take long. My option B is not as I stated -- I misread the spec. Setting no DTEND nor DURATION makes the start and end times coincident, or a zero-duration event.

That would be acceptable.

Therefore, either pass a DTSTART as a DATE and get an all-day event (option A), or as a DATE-TIME and get a zero-duration event (option B).
Comment 8 Kyle M Hall 2022-06-13 17:59:32 UTC
Created attachment 136038 [details] [review]
Bug 30927 - Improve formatting or iCal files for checkout due dates

We got some feedback from a patron regarding the default notifications on the iCal export for the patron checkout summary in the OPAC.
The notifications send 1 minute before the event (the due date), which is midnight.

Test Plan:
0) Make sure your time zone in Koha is set correctly in koha-conf.xml
1) Apply this patch
2) Restart all the things!
3) Make a checkout due at 23:59
4) Make a checkout due at 11:00
5) Download the iCal file from the OPAC
6) Import this file into Calendar
7) Note the first checkout is an all day event
8) Note the second checkout is an "instantaneous" event at the date and time the item is due
Comment 9 Kyle M Hall 2022-06-13 18:04:16 UTC
Created attachment 136039 [details] [review]
Bug 30927 - Improve formatting or iCal files for checkout due dates

We got some feedback from a patron regarding the default notifications on the iCal export for the patron checkout summary in the OPAC.
The notifications send 1 minute before the event (the due date), which is midnight.

Test Plan:
0) Make sure your time zone in Koha is set correctly in koha-conf.xml
1) Apply this patch
2) Restart all the things!
3) Make a checkout due at 23:59
4) Make a checkout due at 11:00
5) Download the iCal file from the OPAC
6) Import this file into Calendar
7) Note the first checkout is an all day event
8) Note the second checkout is an "instantaneous" event at the date and time the item is due
Comment 10 Kyle M Hall 2022-06-13 18:06:22 UTC
I think this latest revision satisfies everyone. With it, all items due at midnight will be all day events, and all items due at a specific time will be instantaneous events!
Comment 11 Owen Leonard 2022-06-13 18:11:59 UTC
Sorry Kyle, now it's saving all the calendar entries as if they are due at the moment I downloaded the iCal file. All items due at the same minute.
Comment 12 Kyle M Hall 2022-06-14 13:57:21 UTC
Created attachment 136069 [details]
Screenshot from Kyle's testing
Comment 13 Kyle M Hall 2022-06-14 13:58:26 UTC
Created attachment 136070 [details]
ICS file from Kyle's testing
Comment 14 Kyle M Hall 2022-06-14 13:59:58 UTC
(In reply to Owen Leonard from comment #11)
> Sorry Kyle, now it's saving all the calendar entries as if they are due at
> the moment I downloaded the iCal file. All items due at the same minute.

That's odd. What version of Calendar are you using? I'm on Version 11.0 (2811.5.1).

Can you take a look at my screen shot and try importing my ICS file? I'm curious if you'll get the same results as me, or the same results as your previous testing.
Comment 15 Owen Leonard 2022-06-14 14:27:16 UTC
Using your ICS file gave me the same result as you show. Here's some new information about my testing, though. Here's the due dates in the account I'm teting:

2022-01-01
2022-05-04
2022-06-01
2022-06-15
2022-06-23
2022-06-23

The items due on 2022-06-23 show on the calendar on 2022-06-22. The one due on 2022-06-15 appears on the calendar on 2022-06-14.

The rest appear on the 14th as well, but each of with a time-due matching the time I imported the file.
Comment 16 Kyle M Hall 2022-06-14 14:50:27 UTC
Created attachment 136071 [details] [review]
Bug 30927 - Improve formatting or iCal files for checkout due dates

We got some feedback from a patron regarding the default notifications on the iCal export for the patron checkout summary in the OPAC.
The notifications send 1 minute before the event (the due date), which is midnight.

Test Plan:
0) Make sure your time zone in Koha is set correctly in koha-conf.xml
1) Apply this patch
2) Restart all the things!
3) Make a checkout due at 23:59
4) Make a checkout due at 11:00
5) Download the iCal file from the OPAC
6) Import this file into Calendar
7) Note the first checkout is an all day event
8) Note the second checkout is an "instantaneous" event at the date and time the item is due
Comment 17 Kyle M Hall 2022-06-14 15:09:23 UTC
Thanks! I hadn't thought about overdues! I was thinking about the overdue and it seems to make sense that they should be all day events for "today". Does that make sense Owen?

Here is a new edition of the patch. The item's are already overdue so 
(In reply to Owen Leonard from comment #15)
> Using your ICS file gave me the same result as you show. Here's some new
> information about my testing, though. Here's the due dates in the account
> I'm teting:
> 
> 2022-01-01
> 2022-05-04
> 2022-06-01
> 2022-06-15
> 2022-06-23
> 2022-06-23
> 
> The items due on 2022-06-23 show on the calendar on 2022-06-22. The one due
> on 2022-06-15 appears on the calendar on 2022-06-14.
> 
> The rest appear on the 14th as well, but each of with a time-due matching
> the time I imported the file.
Comment 18 Owen Leonard 2022-06-14 15:22:58 UTC
(In reply to Kyle M Hall from comment #17)
> it seems to make sense that they should be all day events for "today".

Yes, that makes perfect sense to me. Confusing when I'm thinking in terms of being technically correct but logical from the user's point of view.

What about the 2022-06-23 due date showing up on 2022-06-22? An item without a time due is due at midnight, which implies to me that the calendar should express the idea that the user has all day on the due date to turn it in.
Comment 19 Kyle M Hall 2022-06-14 15:30:44 UTC
Created attachment 136072 [details]
ICS file from Kyle's testing
Comment 20 Kyle M Hall 2022-06-14 15:32:21 UTC
Created attachment 136073 [details]
Screenshot from Kyle's testing 2
Comment 21 Kyle M Hall 2022-06-14 15:34:11 UTC
I think we are in agreement. If an item is due on 2022-06-23 at 23:59, it should show as an all day event for June 23, right?

That appears to be how this latest patch is behaving for me: https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136073

Is it off by a day for you?

(In reply to Owen Leonard from comment #18)
> (In reply to Kyle M Hall from comment #17)
> > it seems to make sense that they should be all day events for "today".
> 
> Yes, that makes perfect sense to me. Confusing when I'm thinking in terms of
> being technically correct but logical from the user's point of view.
> 
> What about the 2022-06-23 due date showing up on 2022-06-22? An item without
> a time due is due at midnight, which implies to me that the calendar should
> express the idea that the user has all day on the due date to turn it in.
Comment 22 Owen Leonard 2022-06-14 17:31:45 UTC
(In reply to Kyle M Hall from comment #21)
> That appears to be how this latest patch is behaving for me

It's not how it's working for me. Here's an example from the ICS file:

BEGIN:VEVENT
DESCRIPTION:Your copy of The best American comics 2010 / (barcode 320000002
 00268) is due back at the library today
DTSTAMP:20220614T172816Z
DTSTART:20220622
SUMMARY:The best American comics 2010 / is due
UID:issue-329@localhost
END:VEVENT
Comment 23 Kyle M Hall 2022-06-14 17:51:10 UTC
Owen, so your still seeing the events as being in the DTSTART date, but at the DTSTAMP time?

(In reply to Owen Leonard from comment #22)
> (In reply to Kyle M Hall from comment #21)
> > That appears to be how this latest patch is behaving for me
> 
> It's not how it's working for me. Here's an example from the ICS file:
> 
> BEGIN:VEVENT
> DESCRIPTION:Your copy of The best American comics 2010 / (barcode 320000002
>  00268) is due back at the library today
> DTSTAMP:20220614T172816Z
> DTSTART:20220622
> SUMMARY:The best American comics 2010 / is due
> UID:issue-329@localhost
> END:VEVENT
Comment 24 Kyle M Hall 2022-06-14 18:21:50 UTC
Created attachment 136074 [details] [review]
Bug 30927 - Improve formatting or iCal files for checkout due dates

We got some feedback from a patron regarding the default notifications on the iCal export for the patron checkout summary in the OPAC.
The notifications send 1 minute before the event (the due date), which is midnight.

Test Plan:
0) Make sure your time zone in Koha is set correctly in koha-conf.xml
1) Apply this patch
2) Restart all the things!
3) Make a checkout due at 23:59
4) Make a checkout due at 11:00
5) Download the iCal file from the OPAC
6) Import this file into Calendar
7) Note the first checkout is an all day event
8) Note the second checkout is an "instantaneous" event at the date and time the item is due
Comment 25 Kyle M Hall 2022-06-14 18:22:29 UTC
Owen, can you give this latest version a try? I've learned a lot about the ical format today! ;)
Comment 26 Owen Leonard 2022-06-15 11:38:21 UTC
My calendar still thinks things are off by a day. Items due on 2022-06-23 are still on 2022-06-22. An item due today, 2022-06-15, appears on 2022-06-14.
Comment 27 Kyle M Hall 2022-06-15 12:36:26 UTC
Created attachment 136094 [details] [review]
Bug 30927 - Improve formatting or iCal files for checkout due dates

We got some feedback from a patron regarding the default notifications on the iCal export for the patron checkout summary in the OPAC.
The notifications send 1 minute before the event (the due date), which is midnight.

Test Plan:
0) Make sure your time zone in Koha is set correctly in koha-conf.xml
1) Apply this patch
2) Restart all the things!
3) Make a checkout due at 23:59
4) Make a checkout due at 11:00
5) Download the iCal file from the OPAC
6) Import this file into Calendar
7) Note the first checkout is an all day event
8) Note the second checkout is an "instantaneous" event at the date and time the item is due
Comment 28 Kyle M Hall 2022-06-15 12:37:04 UTC
We are almost there! Owen, I think this one will fix that!

(In reply to Owen Leonard from comment #26)
> My calendar still thinks things are off by a day. Items due on 2022-06-23
> are still on 2022-06-22. An item due today, 2022-06-15, appears on
> 2022-06-14.
Comment 29 Owen Leonard 2022-06-15 14:36:29 UTC
Created attachment 136095 [details] [review]
Bug 30927 - Improve formatting or iCal files for checkout due dates

We got some feedback from a patron regarding the default notifications
on the iCal export for the patron checkout summary in the OPAC.
The notifications send 1 minute before the event (the due date), which
is midnight.

Test Plan:
0) Make sure your time zone in Koha is set correctly in koha-conf.xml
1) Apply this patch
2) Restart all the things!
3) Make a checkout due at 23:59
4) Make a checkout due at 11:00
5) Download the iCal file from the OPAC
6) Import this file into Calendar
7) Note the first checkout is an all day event
8) Note the second checkout is an "instantaneous" event at the date and
   time the item is due

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 30 Owen Leonard 2022-06-15 14:37:33 UTC
(In reply to Kyle M Hall from comment #28)
> I think this one will fix that!

Looks good, thanks!
Comment 31 Jonathan Druart 2022-08-09 09:54:52 UTC
Please don't forget to run the QA script.

* Commit title does not start with 'Bug XXXXX: ' - 14929d1fe5f
Comment 32 Jonathan Druart 2022-08-09 09:55:22 UTC
Created attachment 138890 [details] [review]
Bug 30927: Improve formatting or iCal files for checkout due dates

We got some feedback from a patron regarding the default notifications
on the iCal export for the patron checkout summary in the OPAC.
The notifications send 1 minute before the event (the due date), which
is midnight.

Test Plan:
0) Make sure your time zone in Koha is set correctly in koha-conf.xml
1) Apply this patch
2) Restart all the things!
3) Make a checkout due at 23:59
4) Make a checkout due at 11:00
5) Download the iCal file from the OPAC
6) Import this file into Calendar
7) Note the first checkout is an all day event
8) Note the second checkout is an "instantaneous" event at the date and
   time the item is due

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 33 Tomás Cohen Arazi 2022-08-09 10:51:39 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 34 Katrin Fischer 2023-09-15 20:38:26 UTC
*** Bug 18563 has been marked as a duplicate of this bug. ***