refactor meta key allowed check

pschoffer commented 2 years ago

Description

When meta key is deleted from a post, this does NOT trigger reindex of that post due to the way of allowed metas are detected.

This change makes it so that we have the logic that evaluates if given meta_key is allowed separate from the logic that prepares metas for indexing. That way we can trigger even for meta key that is not being prepared to indexed (because it was just deleted) but is allowed to be in index (was in the indexed document before).

Checklist

Please make sure the items below have been covered before requesting a review:

[ ] This change works and has been tested locally (or has an appropriate fallback).
[ ] This change works and has been tested on a Go sandbox.
[ ] This change has relevant unit tests (if applicable).
[ ] This change has relevant documentation additions / updates (if applicable).
[ ] This change has the fix PRed upstream (if applicable). If not applicable, it has the relevant "// VIP: reason for the discrepancy with upstream" comment in places where the code is discrepant.

Steps to Test

1) Add this filter impl:

add_filter( 'vip_search_post_meta_allow_list', function ( $allow, $post = null ) {
    $allow['testing'] = true;
    return $allow;
}, 10, 2 );

2) Add meta - vip dev-env exec -- wp post meta set 1 testing foobar 3) Check the document in ES - vip dev-env exec -- wp vip-search documents get post 1 --format=json | jq '.[0].meta' 4) Remove meta vip dev-env exec -- wp post meta delete 1 testing 5) Check the document in ES - vip dev-env exec -- wp vip-search documents get post 1 --format=json | jq '.[0].meta'

pschoffer commented 2 years ago

Upstream change: https://github.com/10up/ElasticPress/pull/2862

pschoffer commented 2 years ago

Hopefully we can get the upstream change in https://github.com/10up/ElasticPress/pull/2862 and then pull that in instead.

pschoffer commented 2 years ago

I checked with 10up and they plan to merge the upstream PR next week. The code has quite diverged there so it can't be cleanly cherry-picked, but I try to sync it as much as I could. This is now ready I think.

Automattic / ElasticPress