Make WordPress Core

Opened 4 months ago

Closed 2 weeks ago

Last modified 2 weeks ago

#61181 closed enhancement (fixed)

Implement a filter to customize output on _block_bindings_post_meta_get_value

Reported by: codersantosh's profile codersantosh Owned by: gziolo's profile gziolo
Milestone: 6.7 Priority: normal
Severity: normal Version: 6.5.3
Component: Editor Keywords: has-patch has-unit-tests needs-dev-note commit
Focuses: Cc:

Description

Occasionally, developers may prefer to modify the displayed content of a meta rather than showing the exact data. For instance, if the meta key is "prefix_price" and the value is either 0 or unset, they might want to show the text "Free!" instead. This can also be achieved by incorporating another register_block_bindings_source. However, developers often add meta for specific purposes, and integrating a filter can facilitate their use of core/post-meta bindings while customizing the output according to their needs.

Change History (18)

#1 @audrasjb
4 months ago

  • Keywords needs-patch added

This ticket was mentioned in โ€‹PR #6839 on โ€‹WordPress/wordpress-develop by โ€‹@snehapatil02.


3 months ago
#2

  • Keywords has-patch added; needs-patch removed

## Ticket: https://core.trac.wordpress.org/ticket/61181

## Description

  • This PR introduces a filter to the _block_bindings_post_meta_get_value function, allowing developers to customize the output of post meta values.
  • This enhancement provides greater flexibility for developers who need to modify the displayed content of a meta value, such as showing "Free!" when the meta value is 0 or unset.

## Changes Made

  • Added Filter Hook: Inserted an apply_filters call before returning the meta value in the _block_bindings_post_meta_get_value function.
  • Filter Name: The filter is named _block_bindings_post_meta_value.

โ€‹@santosguillamot commented on โ€‹PR #6839:


3 months ago
#3

Thanks for working on this! ๐Ÿ™‚ I believe something like this could make sense, but I am not sure how it should be structured. Some of the concerns I have at this point:

  • This filter would only run in the server, which means it would show a different value in the editor. I assume that could be a problem.
  • Does this only apply to post meta or to any binding?
  • If it only applies to post meta, should we add the filter to the bindings logic or directly to get_post_meta / get_metadata function? Does this only apply to bindings or each time you want to retrieve the field?

โ€‹@gziolo commented on โ€‹PR #6839:


3 months ago
#4

If it only applies to post meta, should we add the filter to the bindings logic or directly to get_post_meta / get_metadata function? Does this only apply to bindings or each time you want to retrieve the field?

There is already an existing filter in get_metadata_raw used internally:

get_{$meta_type}_metadata

โ€‹@santosguillamot commented on โ€‹PR #6839:


3 months ago
#5

Thinking about this a bit more, I feel more confident that we should probably provide a general filter for bindings that receive the source and the same arguments as โ€‹get_value_callback. Something similar to the render_block filters. This way, it would be possible to change the value not only of post meta but any other source as well.

โ€‹@snehapatil02 commented on โ€‹PR #6839:


3 months ago
#6

@SantosGuillamot Introduced a general filter 'block_bindings_source_value' to allow developers to modify the value returned by any block binding source.

Changes Made:

  • Modified the get_value method in the WP_Block_Bindings_Source class to apply the new filter.
  • The filter is applied after the value is retrieved from the source but before it's returned.

Key Benefits:

  1. Flexibility: Works for all binding sources, not just post meta.
  2. Consistency: Functions identically in both the editor and on the front end.
  3. Non-intrusive: It doesn't interfere with existing metadata filters but provides a new layer of customization specific to block bindings.

โ€‹@bacoords commented on โ€‹PR #6839:


2 months ago
#7

+1 for this concept. I'll give a real world example and you can tell me if you think this feature would help.

I recently wrote โ€‹a block variation for the image block to pull in the featured image (yes there's a "featured image" block, but the image block has more options, like the lightbox control). I couldn't use the core/postmeta field where the featured image is stored because it returns an attachment ID, not a URL. So I had to create a custom binding source.

It'd be much less overhead if I could have modified the value sent to the block binding API to return the URL of the featured image instead of the attachment ID. But now I have a custom binding source for one simple meta field AND there's no editor preview on my variation.

Feels like this would be a great example of when this feature would be useful.

โ€‹@santosguillamot commented on โ€‹PR #6839:


2 months ago
#8

Thanks for making the changes ๐Ÿ™‚ I agree it makes sense to have a filter like this.

I must say that I had this on my to-do list for a while, but I didn't find time yet with the 6.6 release. I'm really sorry for the wait. I would like to take a deeper look and understand better all the use cases.

In the meantime, would it make sense to add some testing? Is this something done for other similar filters?

Feels like this would be a great example of when this feature would be useful.

I'd need to review your implementation, but at first glance, it looks like a good example for this functionality ๐Ÿ™‚

#9 @gziolo
2 months ago

  • Component changed from General to Editor
  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to 6.7
  • Owner set to gziolo
  • Status changed from new to assigned

โ€‹@santosguillamot commented on โ€‹PR #6839:


3 weeks ago
#10

It seems this is close to being ready. Adding some tests and docs, as suggested below, should be enough to include it in the WordPress 6.7 release.

Thanks a lot for working on it @snehapatil2001! Let us know if you'd like some help in that regard ๐Ÿ™‚

โ€‹@cbravobernal commented on โ€‹PR #6839:


2 weeks ago
#11

I left some additional comments clarifying my previous feedback. In addition to that, we still need unit tests to cover the usage as explained in โ€‹#6839 (review).

Changes addressed.
Are we sure we want the filter to return a function instead of a value? It's kind of confusing with the name.

function filter_block_bindings_source_value() {
		return function () {
			return 'Filtered value';
		};
	}

โ€‹@gziolo commented on โ€‹PR #6839:


2 weeks ago
#12

I left some additional comments clarifying my previous feedback. In addition to that, we still need unit tests to cover the usage as explained in โ€‹#6839 (review).

Changes addressed. Are we sure we want the filter to return a function instead of a value? It's kind of confusing with the name.

function filter_block_bindings_source_value() {
		return function () {
			return 'Filtered value';
		};
	}

This is what I tested locally and you don't need to pass a function:

  • tests/phpunit/tests/block-bindings/postMetaSource.php

    ย 
    266266                        'The post content should not include the script tag.'
    267267                );
    268268        }
    ย 269
    ย 270        /**
    ย 271         * Tests that filter `block_bindings_source_value` is applied.
    ย 272         *
    ย 273         * @ticket 61181
    ย 274         */
    ย 275        public function test_filter_block_bindings_source_value() {
    ย 276                register_meta(
    ย 277                        'post',
    ย 278                        'tests_filter_field',
    ย 279                        array(
    ย 280                                'show_in_rest' => true,
    ย 281                                'single'       => true,
    ย 282                                'type'         => 'string',
    ย 283                                'default'      => 'Original value',
    ย 284                        )
    ย 285                );
    ย 286
    ย 287                $filter_value = function ( $value, $source_name, $source_args ) {
    ย 288                        if ( 'core/post-meta' !== $source_name ) {
    ย 289                                return $value;
    ย 290                        }
    ย 291                        return "Filtered value: {$source_args['key']}";
    ย 292                };
    ย 293
    ย 294                add_filter( 'block_bindings_source_value', $filter_value, 10, 3 );
    ย 295
    ย 296                $content = $this->get_modified_post_content( '<p>Fallback value</p>' );
    ย 297
    ย 298                remove_filter( 'block_bindings_source_value', $filter_value );
    ย 299
    ย 300                $this->assertSame(
    ย 301                        '<p>Filtered value: tests_filter_field</p>',
    ย 302                        $content,
    ย 303                        'The post content should show the filtered value.'
    ย 304                );
    ย 305        }
    269306}

I also updated the test to validate that the args get correctly passed. Like I said in โ€‹https://github.com/WordPress/wordpress-develop/pull/6839#discussion_r1741739083, there should be another test in render.php that validates that all args are correctly passed to the filter.

#13 @gziolo
2 weeks ago

  • Keywords has-unit-tests needs-dev-note commit added; needs-unit-tests removed

โ€‹@santosguillamot commented on โ€‹PR #6839:


2 weeks ago
#14

I've been testing it locally and everything seems to be working as expected. Once the changes to the render.php tests are made, I believe this should be ready.

โ€‹@gziolo commented on โ€‹PR #6839:


2 weeks ago
#15

I've been testing it locally and everything seems to be working as expected. Once the changes to the render.php tests are made, I believe this should be ready.

Agreed, the existing test for post meta is nice to keep as it covers 3 params ($filter_value = function ( $value, $source_name, $source_args ) {), but there is one more param to cover, and it can work with a custom source fine, too.

โ€‹@cbravobernal commented on โ€‹PR #6839:


2 weeks ago
#16

I've been testing it locally and everything seems to be working as expected. Once the changes to the render.php tests are made, I believe this should be ready.

Agreed, the existing test for post meta is nice to keep as it covers 3 params ($filter_value = function ( $value, $source_name, $source_args ) {), but there is one more param to cover, and it can work with a custom source fine, too.

Done!

#17 @cbravobernal
2 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 58972:

Block bindings: Adds a filter to customize the output of a block bindings source.

Introduces a filter to the block_bindings_source_value to allow developers to
modify the value returned by any block binding source.

Props snehapatil02, cbravobernal, gziolo, santosguillamot, bacoords, codersantosh.
Fixes #61181.

Note: See TracTickets for help on using tickets.