event_listing does not find events spanning multiple days in future mode with whole_event

[wohnlice]

Possibly the same as #297

Plone 5.2.8
plone.app.event 3.2.14 (pinned by this Plone version)

To reproduce:
Create an Event with whole_day selected, a start day before today, and an end day after today. e.g. for today's date of 28 July 2022 create an Event with start 01 July 2022 and end 01 August 2022. Create a Collection with criteria portal_type==Event and give it the "Event Listing" display.

Observed behavior:
This event is not displayed in future mode.

Expected behavior:
This event should be displayed in future mode.

Important note: if whole_day is unselected, I do get the expected behavior. This is the biggest indicator that there is a bug with the logic and it is not a design decision or limitation

Diagnosis:
The event machinery here is very complex, which is not surprising given the wide range of configurability offered to users in Plone. Events are searched to match date range and other criteria and then "expanded" to account for occurrences. Events without recurrences still provide IEventRecurrence and so are subject to this process and that seems to be where the culprit is. In particular it is this if/else block https://github.com/plone/plone.app.event/blob/3.2.14/plone/app/event/recurrence.py#L72. In our example above, the if statement is true because of whole_day so it assigns "event_end" as 21 July 2022 23:59:59. This is obviously wrong.

[ewohnlich]

Side question, the buildout.cfg pins versions at Plone 5.0 which has some python 2 packages (ZODB at the very least). I'd like to put in a PR but I don't even have python 2 on my machine (and can't install it at work because of our security policy). Can't we update this to 5.2.8 or at least 5.2?

Planned patch:

def occurrences(self, range_start=None, range_end=None):
    """ see plone.app.events.recurrence
    """
    event = IEventAccessor(self.context)

    # We try to get IEventBasic start without including recurrence
    event_start = getattr(self.context, 'start', None)
    if not event_start:
        event_start = event.start
    elif getattr(event, 'whole_day', None):
        event_start = dt_start_of_day(event_start)

    # We get event ends by adding a duration to the start. This way, we
    # prevent that the start and end lists are of different size if an
    # event starts before range_start but ends afterwards.
    event_end = getattr(self.context, 'end', None)
    if getattr(event, 'open_end', None):
        duration = datetime.timedelta(hours=23, minutes=59, seconds=59)
    elif getattr(event, 'whole_day', None):
        duration = event_end + datetime.timedelta(hours=23, minutes=59, seconds=59) - event_start
    else:
        duration = event_end - event_start

    starts = recurrence_sequence_ical(event_start,
                                      recrule=event.recurrence,
                                      from_=range_start, until=range_end,
                                      duration=duration)

    # XXX potentially occurrence won't need to be wrapped anymore
    # but doing it for backwards compatibility as views/templates
    # still rely on acquisition-wrapped objects.
    def get_obj(start):
        if pydt(event_start.replace(microsecond=0)) == start:
            # If the occurrence date is the same as the event object, the
            # occurrence is the event itself. return it as such.
            # Dates from recurrence_sequence_ical are explicitly without
            # microseconds, while event.start may contain it. So we have to
            # remove it for a valid comparison.
            return self.context
        return Occurrence(
            id=str(start.date()),
            start=start,
            end=start + duration).__of__(self.context)

    for start in starts:
        yield get_obj(start)

[davisagli]

@ewohnlich That does seem somewhat out of date. plone.app.event is considered part of Plone core, so I am guessing no one noticed the buildout here is out of date because it is more typically developed by adding it to checkouts.cfg in buildout.coredev

From what I see in the 5.2 and 6.0 branches of buildout.coredev, it looks like the 3.2.x branch of plone.app.event is used with Plone 5.2 (and needs to remain compatible with Python 2, because Plone 5.2 still supports it). The master branch of plone.app.event is used with Plone 6 and only needs to support Python 3.

[wohnlice]

Continuing on this, duration = datetime.timedelta(hours=23, minutes=59, seconds=59) for Open End events is arguably not the best calculation for duration. If you have an open end event at 1pm on Nov 1, I would expect it to show in aggregators only on Nov 1. But this calculation gives it an end time of 12:59pm on Nov 2. duration = dt_end_of_day(event_end) - event_start seems more appropriate to me.

[petschki]

@wohnlice I've fixed that here #370