Fix exception when generating changeRequest XML where the oldProperty is null and newProperty is a DateTime #380
Conversation
Fixes an exception that occurs when one property is null and the other is a DateTime object.
| @@ -246,18 +246,22 @@ public static function getDirtyPropertiesFromUser(User $changedObject) | |||
| && !in_array($propertyName, $ignoreProperties) | |||
| ) { | |||
| $newPropertyValue = $changedObject->{'get' . ucfirst($propertyName)}(); | |||
| if (!is_object($oldPropertyValue) || !is_object($newPropertyValue)) { | |||
| if (!is_object($oldPropertyValue) && !is_object($newPropertyValue)) { | |||
There was a problem hiding this comment.
We can move the condition of the next line into this condition.
There was a problem hiding this comment.
That condition should probably stay as is, otherwise the else part starting in line 267 is entered when it shouldn't be.
Classes/Utility/UserUtility.php
Outdated
| if ($oldPropertyValue !== $newPropertyValue) { | ||
| $dirtyProperties[$propertyName]['old'] = $oldPropertyValue; | ||
| $dirtyProperties[$propertyName]['new'] = $newPropertyValue; | ||
| } | ||
| } else { | ||
| if (get_class($oldPropertyValue) === 'DateTime') { | ||
| if (($oldPropertyValue != null && get_class($oldPropertyValue) === 'DateTime') || ($newPropertyValue != null && get_class($newPropertyValue) === 'DateTime')) { |
There was a problem hiding this comment.
Please use type safe comparisons (!==)
There was a problem hiding this comment.
Also, use $oldPropertyValue instanceof \DateTime::class instead of get_class.
There was a problem hiding this comment.
Added your suggestions, but i left out the ::class since \DateTime is already the fully qualified class name.
Classes/Utility/UserUtility.php
Outdated
| $dirtyProperties[$propertyName]['old'] = $oldPropertyValue->getTimestamp(); | ||
| $dirtyProperties[$propertyName]['new'] = $newPropertyValue->getTimestamp(); | ||
|
|
||
| $oldTimestamp = $oldPropertyValue != null ? $oldPropertyValue->getTimestamp() : 0; |
There was a problem hiding this comment.
$oldPropertyValue cannot be null and must therefore be a DateTime object, it's checked in the condition before.
There was a problem hiding this comment.
The condition in line 255 only checks if at least one of the two propertyValues is a DateTime. Therefore either one of them could still be null inside the condition and has to checked again,
resolves #381
Additionally when the value of a DateTime property is
null,0is returned instead of the timestamp.