Skip to content
This repository has been archived by the owner on Oct 31, 2018. It is now read-only.

Fixes #1009: add LAST-MODIFIED to calendardata when it is absent #1010

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arno01
Copy link

@arno01 arno01 commented Dec 31, 2015

to close owncloud/calendar#1009

Suggestions/critics are welcomed as my php experience is very little :-)

$lastModified = $vevent->__get('LAST-MODIFIED');
if (is_null($lastModified)) {
$lastModified = $vevent->add('LAST-MODIFIED');
}
$vevent->__get('LAST-MODIFIED')->setDateTime($now);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't a simple change of

$vevent->__get('LAST-MODIFIED')->setDateTime($now);

to

$vevent->__set('LAST-MODIFIED', $now);

be enough? That should be the identical rewrite of what happens with DTSTAMP in the line after...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx @gvde ! I've tested and it is working for me.

@gvde
Copy link
Contributor

gvde commented Dec 31, 2015

The problem with the LAST-MODIFIED may be present at other places as well. Should be checked everywhere if it's O.K. at all places...

@arno01 arno01 changed the title Fixes #1009: add LAST-MODIFIED to calendardata when missing Fixes #1009: add LAST-MODIFIED to calendardata when it is absent Jan 1, 2016
@gvde
Copy link
Contributor

gvde commented Jan 1, 2016

There may be one problem assigning the same object to two properties: if the object is changed through one property it affects the other property as well (e.g. if the following code would somewhere modify the time zone of DTSTAMP to something else it would also change the timezone of LAST-MODIFIED).

Thus, I guess to be safe, you should assign LAST-MODIFIED not $now but "clone $now" which creates a copy of the object in $now. Then DTSTAMP and LAST-MODIFIED both are set to the current time but using two different objects.

@arno01
Copy link
Author

arno01 commented Jan 2, 2016

@gvde thanks for your valuable comment! I've just pushed the update, please see if everything is okay now from your point of view.

@gvde
Copy link
Contributor

gvde commented Jan 3, 2016

Looking good to me...

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

Successfully merging this pull request may close these issues.

2 participants