move rest filters to init #209
Conversation
pereirinha
left a comment
pereirinha
left a comment
There was a problem hiding this comment.
I'm approving this PR, but I left you a couple of comments that I'd like you to consider.
Also, would you mind to cherry pick d4d7ef2fe3f1298c16a0a4efb6bd6af71b39ac9a? It's the fix for the stored meta.
| */ | ||
| public function match_file_name_with_cloudinary_source( $image_meta, $attachment_id ) { | ||
| if ( $this->has_public_id( $attachment_id ) ) { | ||
| if ( is_array( $image_meta ) && isset( $image_meta['file'] ) && $this->has_public_id( $attachment_id ) ) { |
There was a problem hiding this comment.
This is ok, but you could have used instead ! empty( $image_meta['file'] ) &&
| add_filter( 'rest_prepare_attachment', array( $this, 'filter_attachment_for_rest' ) ); | ||
| $types = get_post_types_by_support( 'editor' ); | ||
| $filter = $this; | ||
| array_map( |
There was a problem hiding this comment.
Since we are refactoring this, I'd like you to consider changing this to a loop. After reading your comments and @dugajean, and after reading more about this, I think this is not a use case for array_map. In PHP documentation for this, you can see that:
array_map() returns an array containing the results of applying the callback to the corresponding index of array1 (and ... if more arrays are provided) used as arguments for the callback. The number of parameters that the callback function accepts should match the number of arrays passed to array_map().
This code is not leveraging any return from this function, and as stated before, it involves some unneeded overhead.
Again, I'm coming back to it only because we are refactoring, and it feels like the right opportunity.
No description provided.