-
Notifications
You must be signed in to change notification settings - Fork 35
Get the formatted_value
from the original field value
#211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
…tted value for rest The `formatted_value` property of the $field['name'] . '_source' should come from the original value, not the REST value. Other plugins might change the REST value using the filter `acf/rest/format_value_for_rest`, which can cause the `formatted_value` to be incorrect.
The formatted_value property of the $field['name'] . '_source' should come from the original value, not the REST value. Other plugins might change the REST value using the filter acf/rest/format_value_for_rest, which can cause the formatted_value to be incorrect.
|
||
// We keep this one for backward compatibility with existing code that expects the field value to be. | ||
$fields[ $field['name'] ] = $value; | ||
$fields[ $field['name'] ] = $rest_value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
Why will this fix your fatal error on the plugin? The change, as far as I can see, is just changing a variable name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for approving the pull request.
Here is my explanation.
The original code from SCF:
// Get raw value.
$value = acf_get_value( $post_id, $field );
// First format for rest API for backward compatibility.
$value = acf_format_value_for_rest( $value, $post_id, $field, $format );
// We keep this one for backward compatibility with existing code that expects the field value to be.
$fields[ $field['name'] ] = $value;
$fields[ $field['name'] . '_source' ] = array(
'label' => $field['label'],
'type' => $field['type'],
// This line re-formats the already formatted REST value instead of the raw value, which causes issues.
'formatted_value' => acf_format_value( $value, $post_id, $field ),
);
I fixed it by renaming the formatted REST value to $rest_value and leaving the original $value untouched. This way, when calling acf_format_value() to populate the formatted_value property of _source, it’s based on the raw value, not on the already formatted value:
// Format value for rest API for backward compatibility.
$rest_value = acf_format_value_for_rest( $value, $post_id, $field, $format );
// We keep this one for backward compatibility with existing code that expects the field value to be.
$fields[ $field['name'] ] = $rest_value;
$fields[ $field['name'] . '_source' ] = array(
'label' => $field['label'],
'type' => $field['type'],
'formatted_value' => acf_format_value( $value, $post_id, $field ),
);
In my plugin (Meta Field Block), I used the 'acf/rest/format_value_for_rest' filter to change the format of the REST value to support displaying all field types (even Group, Repeater, Flexible, Relationship, etc). When SCF re-format some multi-value field types like select, it treats the altered REST value as multiple values, which is why the error occurs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Mr2P, thanks again for engaging here 🙂
After a thorough read of the full method, I agree that the new variable name is helping not to overwrite the existing $value
variable, great! My only concern is that I am not sure about the side effects this effect can have for other users; would you mind adding some tests to ensure backwards compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @priethor,
I’m not sure what needs to be tested for this fix. The root cause of the issue is that, starting with SCF 6.5.2, the wrong value has been assigned to the new formatted_value property.
It should be:
$raw_value = acf_get_value( $post_id, $field );
$fields[ $field['name'] . '_source' ] = array(
'formatted_value' => acf_format_value( $raw_value, $post_id, $field ),
);
not:
$raw_value = acf_get_value( $post_id, $field );
$formatted_value_for_rest = acf_format_value_for_rest( $raw_value, $post_id, $field, $format );
$fields[ $field['name'] . '_source' ] = array(
'formatted_value' => acf_format_value( $formatted_value_for_rest, $post_id, $field ),
);
The value returned by acf_format_value_for_rest is filtered by:
apply_filters( 'acf/rest/format_value_for_rest', $value_formatted, $post_id, $field, $value, $format );
If any third-party plugin uses this filter to change the REST value, the formatted_value property may end up with the wrong value.
The
formatted_value
property of the $field['name'] . '_source' should come from the original value, not the REST value. Other plugins might change the REST value using the filteracf/rest/format_value_for_rest
, which can cause theformatted_value
to be incorrect.