Skip to content

fix: honor return_format in acf_format_value for Image/File fields #530

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.

Sign up for GitHub

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

Closed
faisalahammad wants to merge 1 commit into from
Closed

fix: honor return_format in acf_format_value for Image/File fields #530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions includes/acf-value-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ function acf_format_value( $value, $post_id, $field, $escape_html = false ) {
return $store->get( $cache_name );
}

// Snapshot the raw value so we can detect a missed type-specific
// dispatcher below. If the filter chain leaves the value untouched and the
// field has a non-id return_format, run the formatter directly (#527).
$raw_value = $value;

/**
* Filters the $value for use in a template function.
*
Expand All @@ -190,6 +195,32 @@ function acf_format_value( $value, $post_id, $field, $escape_html = false ) {
*/
$value = apply_filters( 'acf/format_value', $value, $post_id, $field, $escape_html );

// Safety net: if the filter chain left the value unchanged and the field
// expects a formatted output, the type callback was skipped. Run the field
// type's format_value() directly so Image/File fields honor their
// return_format setting (#527).
if (
$value === $raw_value
&& is_numeric( $value )
&& ! is_float( $value )
&& (string) (int) $value === (string) $value
&& ! empty( $field['type'] )
&& ! empty( $field['return_format'] )
&& 'id' !== $field['return_format']
) {
$type = acf_get_field_type( $field['type'] );
if ( $type && method_exists( $type, 'format_value' ) ) {
try {
$candidate = $type->format_value( $value, $post_id, $field );
} catch ( \Throwable $e ) {
$candidate = $value;
}
if ( ! is_numeric( $candidate ) ) {
$value = $candidate;
}
}
}

// Update store.
$store->set( $cache_name, $value );

Expand Down
28 changes: 28 additions & 0 deletions tests/php/includes/fields/test-class-acf-field-file.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,32 @@ public function test_array_format_returns_expected_structure() {
$this->assertArrayHasKey( 'url', $result );
$this->assertEquals( $this->attachment_id, $result['ID'] );
}

/**
* Regression test for #527: get_field() (which routes through
* acf_format_value() and the acf/format_value filter chain) must respect
* every documented return_format on File fields, mirroring the Image fix.
*
* @dataProvider return_format_provider
*
* @param string $return_format The return format setting.
*/
public function test_get_field_pipeline_respects_return_format( $return_format ) {
$field = $this->get_field( array( 'return_format' => $return_format ) );

// The production path get_field() uses.
$result = acf_format_value( $this->attachment_id, $this->post_id, $field );

if ( 'url' === $return_format ) {
$this->assertIsString( $result );
$this->assertStringContainsString( 'test-file.pdf', $result );
} elseif ( 'array' === $return_format ) {
$this->assertIsArray( $result );
$this->assertArrayHasKey( 'ID', $result );
$this->assertArrayHasKey( 'url', $result );
$this->assertEquals( $this->attachment_id, $result['ID'] );
} else {
$this->assertEquals( $this->attachment_id, $result );
}
}
}
33 changes: 33 additions & 0 deletions tests/php/includes/fields/test-class-acf-field-image.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,37 @@ public function test_array_format_returns_expected_structure() {
$this->assertArrayHasKey( 'url', $result );
$this->assertEquals( $this->attachment_id, $result['ID'] );
}

/**
* Regression test for #527: get_field() (which routes through
* acf_format_value() and the acf/format_value filter chain) must respect
* every documented return_format. Previously, only direct calls to
* format_value()/format_value_for_rest() were covered, leaving the real
* filter-chain path untested.
*
* @dataProvider return_format_provider
*
* @param string $return_format The return format setting.
*/
public function test_get_field_pipeline_respects_return_format( $return_format ) {
$field = $this->get_field( array( 'return_format' => $return_format ) );

// The production path get_field() uses: acf_format_value() invokes the
// acf/format_value filter chain, which dispatches to acf_field_image::
// format_value via the type=name=key variation in
// _acf_apply_hook_variations().
$result = acf_format_value( $this->attachment_id, $this->post_id, $field );

if ( 'url' === $return_format ) {
$this->assertIsString( $result );
$this->assertStringContainsString( 'test-image.jpg', $result );
} elseif ( 'array' === $return_format ) {
$this->assertIsArray( $result );
$this->assertArrayHasKey( 'ID', $result );
$this->assertArrayHasKey( 'url', $result );
$this->assertEquals( $this->attachment_id, $result['ID'] );
} else {
$this->assertEquals( $this->attachment_id, $result );
}
}
}
Loading