Skip to content
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

Metadata: Allow all metadata to be returned for an object #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pablinos
Copy link

While testing some code that was setting post metadata, I was debugging
it by outputting all the metadata for a post with a call to
get_post_meta( $post_id ). I noticed that the tests started passing
but I was still getting an empty array from the get_post_meta call.

Getting a specific key was working, which led me to look at the Metadata
code here, which currently doesn't deal with the case of the key being
an empty value.

This change refactors the get function, but is probably not optimal,
as we change the shape of the object's metadata using array_reduce on
each call. It illustrates the issue, and I'm happy to optimise it if we
want to go this route.

While testing some code that was setting post metadata, I was debugging
it by outputting all the metadata for a post with a call to
`get_post_meta( $post_id )`. I noticed that the tests started passing
but I was still getting an empty array from the `get_post_meta` call.

Getting a specific key was working, which led me to look at the Metadata
code here, which currently doesn't deal with the case of the key being
an empty value.

This change refactors the `get` function, but is probably not optimal,
as we change the shape of the object's metadata using `array_reduce` on
each call. It illustrates the issue, and I'm happy to optimise it if we
want to go this route.
@leogermani
Copy link
Collaborator

Thanks for this!

I think it's fine if things are not super performant, This is meant to run test suites so a little bit of overload is accepted (as we are removing a lot of overload by not having a database!).

Do you think you could write some tests for this? And then, from there, we can look at the best implementation

$obj_meta = array_reduce(
$this->meta[ $object_id ],
function ( $all_meta, $meta ) {
$all_meta[ $meta['meta_key' ] ] = array_merge( $all_meta[ $meta['meta_key'] ] ?? [], [ $meta['meta_value'] ] );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep 5.6 compatibility while core keeps it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants