API page: https://api.drupal.org/api/drupal/modules%21field%21field.crud.inc/funct...

Enter a descriptive title (above) relating to function field_update_instance, then describe the problem you have found:
The example should read:

<?php
 
// Fetch an instance info array.
 
$instance_info = field_info_instance($entity_type, $field_name, $bundle_name);
 
// Change a single property in the instance definition.
 
$instance_info['required'] = TRUE;
 
// Write the changed definition back.
 
field_update_instance($instance_info);
?>

since there is no 'definition' key in the $instance_info array.

The following is the existing, incorrect version:

<?php
 
// Fetch an instance info array.
 
$instance_info = field_info_instance($entity_type, $field_name, $bundle_name);
 
// Change a single property in the instance definition.
 
$instance_info['definition']['required'] = TRUE;
 
// Write the changed definition back.
 
field_update_instance($instance_info['definition']);
?>
Files: 
CommentFileSizeAuthor
#2 update-field_update_instance-example-2054189-2.patch1.25 KBShaunDychko
PASSED: [[SimpleTest]]: [MySQL] 40,348 pass(es).
[ View ]

Comments

Issue tags:+Novice

Hm.

I looked at all the non-test calls to field_update_instance(), and they all use something like this:

$instance = field_read_instance($entity_type, $field_name, $bundle_name);
$instance['required'] = TRUE;
field_update_instance($instance);

So I think we should update the example on field_update_instance() to this instead of what is there now.

This is Drupal 7 only and probably a good Novice project?

Status:Active» Needs review
StatusFileSize
new1.25 KB
PASSED: [[SimpleTest]]: [MySQL] 40,348 pass(es).
[ View ]

How's this?

Status:Needs review» Reviewed & tested by the community

This seems fine to me, thanks!

Status:Reviewed & tested by the community» Needs review

I thought it was preferable to use field_info_instance() whenever possible (for example, unless you're loading an inactive instance or something like that in which case it won't work). And that is basically what the function documentation says too... I think if it's actually correct to use field_read_instance() in this example, the documentation for that function should be changed also.

I am honestly not sure whether it matters or not! I thought it didn't, but it is interesting that - as pointed out above - the Field UI module is using field_read_instance() in these situations.

I bet someone like @yched would know the answer right away. If it becomes complicated, though, we could just fix the 'definition' issue here and figure out field_info_instance() vs. field_read_instance() later?

Well, I looked at what Core is using when it was doing a field_info_update() after first reading in existing data, and based my suggestion above on that (see #1). So this advice is basically what Core is actually doing.

I really don't know the difference between the field_info_instance() and field_read_instance() functions. It seems confusing to have two functions that look like they're doing the same thing... Maybe we should also document that?

2 cents: I think what jhodgdon meant was to say "... when it was doing a field_update_instance() after first reading in existing data..."

#6. Yes. :)

Yeah, the current documentation on field_read_instance() says to "generally" use field_info_instance() instead, which doesn't mean much... I agree we should improve that documentation throughout.

I created #2059229: Clarify documentation around field_info_instance() and field_read_instance() for this. Given that, how about for this issue we just fix the 'definition' key and leave the rest alone? That part should be a very quick/simple fix.

The problem with #8 is that I could not find any information really on how field_info_instance() should be used for this case. Core is always using field_read_instance() for this type of thing when it's followed by field_update_instance(). Can't we just change the example to what core is actually doing rather than trying to puzzle out what might be correct with field_read_instance?