Date fields can be programmatically added like this:
$entity->get($field_name)->offsetGet(0)->setValue(array($iso_date));
// or
$entity->get($field_name)->offsetGet(0)->setValue(array('value' =>$iso_date));
But there is an exception when they are added like this:
$entity->get($field_name)->offsetGet(0)->get('value')->setValue($iso_date);
This works for text fields, so I would expect it to work for all fields.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 2019587-15-datetimeitem-test.patch | 4.48 KB | linclark |
| #15 | interdiff.txt | 755 bytes | linclark |
| #13 | 2019587-13-datetimeitem-test.patch | 4.54 KB | berdir |
| #13 | 2019587-13-datetimeitem-test-interdiff.txt | 1.07 KB | berdir |
| #4 | 2019587-04-datetimeitem-test.patch | 4.95 KB | berdir |
Comments
Comment #1
linclark commentedThis patch introduces a DateTimeItemTest. It includes a test for what I think is a bug.
$entity->get($field_name)->offsetGet(0)->setValue(array('value' =>$iso_date));This works for text fields but does not work for date. DateTimeItemTest::testSetValueProperty() tests for this and results in an Exception currently.
Comment #2
linclark commentedChanging the title to reflect the problem.
Comment #2.0
linclark commentedUpdated issue summary.
Comment #4
berdirI knew I've seen this before.
This was initially the case consistently, the date field tests were failing horribly when I started converting them to use an NG entity. And then suddenly, it worked fine and I didn't know why.
The reason is the lazy loading of values. If you pass in the value on the DateItem, then the Date typed data object is never created, the value is simply stored in DateItem and then returned exactly as it was put in. However, if you do a setValue() on the date, then it ensures that it's a DrupalDateTime object and also returns that on ->value. Thats why currently the date fields are working.
I am not sure what to do. As I've said before, I think we need an method that returns a raw/storage/scalar representation of the value that can be used to store it somewhere, as you don't want to persist the date object.
As demonstration, the attached patch removes the DrupalDateTime check in Date::setValue(), this makes this test pass but then TypedDataTest explodes as it expects a date time object there.
Maybe the ongoing refactoring to use interfaces will help here as well, as we could do this: Date implements StringInterface, DateInterface. Then getValue() (or getString() but that's IMHO tricky) will return the date string and getDate() will return a DrupalDateTime object.
Comment #6
berdir#2002102: Move TypedData primitive types to interfaces is the issue that I mentioned and the latest iteration of the Date class there should fix this.
I'd suggest to keep this open and push the interface issue, then we can just keep open as a reminder and to add test-coverage. Or we could move the test coverage over there, but the might might already get quite big...
Comment #7
fagoRelated: #1867888: Improve TypedData date handling
Comment #8
berdir#4: 2019587-04-datetimeitem-test.patch queued for re-testing.
Comment #9
berdir#1: 2019587-01-datetimeitem-test.patch queued for re-testing.
Comment #11
berdirSetting to needs review, test-patch is still running, re-tested the wrong one.
Comment #12
berdirFatal error, field_create_field() no longer exists.
Comment #13
berdirUpdated the field creation, test is green now :)
Which means this is just about adding additional test coverage.
Comment #14
linclark commentedSorry, this is my bad... I'll try to reroll if I get a chance. I'm giving a few talks this afternoon at NYC Camp, so this might not be until later.
Comment #15
linclark commentedOk, removed the comment.
Comment #16
berdirYay more test coverage.
I only changed the field_create_field() call to entity_create(), so I think it's ok if I RTBC this.
Comment #17
alexpottCommitted b11b7fa and pushed to 8.x. Thanks!
Comment #18.0
(not verified) commentedUpdated code example in issue summary.