Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding 4 days in end date if no time is true #98

Closed
piyushkantm opened this issue Jun 5, 2017 · 7 comments
Closed

Adding 4 days in end date if no time is true #98

piyushkantm opened this issue Jun 5, 2017 · 7 comments
Labels

Comments

@piyushkantm
Copy link

piyushkantm commented Jun 5, 2017

There is an issue which is caused on the following conditions:

$event = new Event('some UID')
$event->setDtStart($startDate);
$event->setDtEnd($endDate);
$event->setNoTime(true);

What is the point of adding 1 day to the end date if no time is true?
https://github.com/markuspoerschke/iCal/blob/master/src/Component/Event.php#L261

This line gets executed 4 times and increases the end date by 4 days.

@navarr
Copy link
Contributor

navarr commented Jun 5, 2017

A workaround until a fix is implemented would be to create a new $endDate for each iteration.

The problem is, of course, that since the date is mutable when setNoTime increments the date it's incrementing the same object that's being passed around to all the events.

setNoTime indicates that it's an all day event, and so sets the endDate to exactly 24 hours after the start date.

The point of adding 1 day to the end date is documented by #83

@piyushkantm
Copy link
Author

Thanks for quick response. but i do not follow what you mean by

create a new $endDate for each iteration

Even if we change $this->dtEnd->add(new \DateInterval('P1D')); to
$this->dtEnd = (clone $this->dtEnd)->add(new \DateInterval('P1D'));

@markuspoerschke
Copy link
Owner

@piyushkantm Please check the test here. Calling the render method from the component can cause two different results since the date is changed. From your pull request I can see that you fixed it there.

@piyushkantm I applied an updated version of #99 to master that also contains the fix from #100. Can you test that using dev-master of this package?

@piyushkantm Thanks for reporting the issue! :) Also I would like you for your Pull Request.
@navarr Thanks for your support as well! :)

@piyushkantm
Copy link
Author

Yes, the dev-master works fine. Thanks

@markuspoerschke
Copy link
Owner

@piyushkantm cool, then I will create a new release so you can install a stable version.

@markuspoerschke
Copy link
Owner

Available in Version 0.12.1 now.

@piyushkantm
Copy link
Author

thanks. I was already using the dev-master branch 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants