niccokunzmann / python-recurring-ical-events

Python library to calculate recurrence times of events, todos and journals based on icalendar RFC5545
https://pypi.org/project/recurring-ical-events/
GNU Lesser General Public License v3.0
92 stars 20 forks source link

Fix range with resource_ids. #171

Closed jabiertxof closed 3 weeks ago

jabiertxof commented 1 month ago

Fixes https://github.com/niccokunzmann/python-recurring-ical-events/issues/75

niccokunzmann commented 1 month ago

Hi, could you create a Pr against the issue-75 branch? that one has the tests in it that need passing.

Looking at the code, we cannot replace the core but somehow need to use one of the modifications when it is time.

niccokunzmann commented 3 weeks ago

Thanks! I merge it and check ...

jabiertxof commented 3 weeks ago

Hi Nicco, I'm not familiar with github, sorry for don't reply at time. Seems my code is removed from master, but current master code not handle the issue, could you explain to me why you remove it? Thanks so much for your work.

Regards, Jabier.

jabiertxof commented 3 weeks ago

Hi Nicco.

If you not know, and as far I see, a event with recurrence-id with "range" datetime must:

This is to work OK not sure if spec define as it.

Thanks in advance for work on!

El lun, 09-09-2024 a las 12:19 -0700, Nicco Kunzmann escribió:

Thanks! I merge it and check ... — Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. You are receiving this because you authored the thread.Message ID: <niccokunzmann/python-recurring-ical- @.***>

[1] view it on GitHub https://github.com/niccokunzmann/python-recurring-ical-events/pull/171#issuecomment-2338897464 [2] unsubscribe https://github.com/notifications/unsubscribe-auth/ABA273EZ4BEGIYX5XBA35SDZVXYD3AVCNFSM6AAAAABNON5CVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZYHA4TONBWGQ

niccokunzmann commented 3 weeks ago

Hello, thanks for coming back to this! I removed the code from the master branch after merge because of

This project's development is driven by tests. - README

The code did not fix the tests however it was a good place to put it. What it did is though that the event with the RANGE parameter would also modify the events before it. I merged it for these reasons:

  1. I value your contribution
  2. I wanted to check if it fixes the tests
  3. I rather want to give it a go than criticize around

I then removed it again because it did not solve the issue.

jabiertxof commented 3 weeks ago

Hi Nicco, thanks for review.

As far I know if the range value (datetime) is == first recurrence to change date + initial event time (as far i know is the spec) it works as spected. I put in other mail the same extended with other spec constrains.

Regards, Jabier

niccokunzmann commented 3 weeks ago

Hi, let's discuss this in the issue, so it is all together in one place.