jazzband / django-recurrence

Utility for working with recurring dates in Django.
https://django-recurrence.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
486 stars 190 forks source link

function to_dateutil_rruleset() include the dtend in returned array #232

Open LucasGrugru opened 2 years ago

LucasGrugru commented 2 years ago

Hello,

I noticed a behavior that I don't understand. This may be a desired behavior but it brings unwanted data.

the function to_dateutil_rruleset(self, dtstart=None, dtend=None, cache=False) with a dtend always returns the end date, even if the latter is not part of the recurrence rule. The problem is located on file base.py:571

        if dtend is not None:
            rruleset.rdate(dtend)

the test for this part of code is just as mysterious (test_occurrences.py:160):

    assert occurrences == [
        datetime(2014, 1, 1, 0, 0, 0),
        datetime(2014, 1, 2, 0, 0, 0),
        datetime(2014, 1, 3, 0, 0, 0),
        # We always get dtend, for reasons that aren't entirely clear
        datetime(2014, 1, 5, 0, 0, 0),
    ]

The problem occurred when we want to retrieve or counting occurences between two date or after a date. For the function between(self, after, before, inc=False, dtstart=None, dtend=None, cache=False) for example, the before parameter is passed to to_dateutil_rruleset as dtend parameter, and the returned array has not the good value.

I think, we can add paramter to to_dateutil_rruleset for not adding the dtend when we call function from before, after, between, count.

Maybe there's something I just don't understand? Thank you for your time.

RmaxTwice commented 1 year ago

You should label this as a bug, because it is...I've been struggling because of this issue as well...

LucasGrugru commented 1 year ago

I'm not convinced it's a bug; the comment implies that the behavior was intended at the time of development. In all cases, the author appears to have abandoned the project. There has been no response to my post, and there has been no response to the other posts as well...

lino commented 3 weeks ago

No matter if it's a bug or not, changing the default behaviour will incur a breaking change, which is something we should only do on a major version change. Thanks for thinking about this.

LucasGrugru commented 3 weeks ago

Thanks, @lino , for taking the time to look into this issue.

However, I’m not sure I fully understand your point regarding the change in behavior. Adding the proposed parameter keeps the current default behavior intact but gives developers the option to enforce a different behavior, that's all. So, there’s no actual change in behavior, and the existing use cases remain unaffected. Does this really require a major version change?