Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
It is noted in the documentation here that field_create_instance function should return "The $instance array with the id property filled in.". However it does not. After field instance creation, 'id' property of the instance remains empty.
Comments
Comment #1
Everett Zufelt CreditAttribution: Everett Zufelt commentedFrom what I recall this actually does work as designed, i.e. I do recall that 'id' is returned from field_create_instance().
Can you please provide an example dump of the value returned from field_create_instance()? The 'id' is usually a long way down in the array.
Comment #2
lemark CreditAttribution: lemark commentedIt can not be just because of _field_write_instance function. It takes instance as argument (and it is not passed by reference), write record in DB and return nothing. Also you can open field_create_instance() function declaration and find out that it can't return instance 'id' within the instance array anyway.
Comment #3
webchickI assume that was a typo. :)
Comment #4
lemark CreditAttribution: lemark commentedAlso, I believe it should be fixed in 7.
Comment #5
marcingy CreditAttribution: marcingy commentedItems are fixed in d8 first
Comment #6
lemark CreditAttribution: lemark commentedThis patch should fix that
Comment #7
lemark CreditAttribution: lemark commentedNeeds review.
Patch equal to the previous one. Sorry for that duplicate, I'm steel not very familiar with drupal.org developing pipeline.
Comment #8
yched CreditAttribution: yched commentedLooks good to me. Thanks !
Comment #9
chx CreditAttribution: chx commentedThanks for working on this patch. It could use an automated test before it is committed. If you need help you can ask the friendly people in the #drupal-contribute IRC channel.
Also, I have tried to reroll this patch.
Comment #10
chx CreditAttribution: chx commentedBetter but I doubt perfect. Alas I have ran out of time working on this. Please finish. Thanks!
Comment #12
lemark CreditAttribution: lemark commented-- message deleted --
my mistake
Comment #13
nils.destoop CreditAttribution: nils.destoop commentedFinished the work of chx.
Comment #14
nils.destoop CreditAttribution: nils.destoop commentedComment #16
nils.destoop CreditAttribution: nils.destoop commentedHad to change some tests to support the new parameter by reference.
Comment #17
nils.destoop CreditAttribution: nils.destoop commentedThe t() has been removed from the assertIdentical check. (#500866: [META] remove t() from assert message)
Attachment with the added test + patch that fixes current bug.
Comment #18
Lars Toomre CreditAttribution: Lars Toomre commentedIf this patch gets re-rolled again, could we incorporate the fixes for the following small nits?
Could this be 'Check that the ID key is filled in.'? Seeing abbreviations like ID in comments in lower case appears wrong.
Since we are touching this assertion as well, can we remove t() around this assertion?
Comment #19
nils.destoop CreditAttribution: nils.destoop commentedSounds good. Attached patch with small nits fixed.
Comment #20
swentel CreditAttribution: swentel commentedLooks good to me.
Comment #21
webchickThis looks like good clean-up and comes with tests.
Committed and pushed to 8.x. Thanks!