Distributor: Post meta is never actually deleted on the receiving end

[helen]

Noticed in #258 / #259:

set_meta() never actually deletes any post meta. This appears to always have been an issue but was exacerbated by a bug we introduced in 1.3.4 that was duplicating empty meta values that are now kind of stuck in place because of the way update_post_meta() works with previous values.

I don't yet know what the right thing to do is. We could:

• Be less performant and just wipe out everything not blacklisted before adding again
• Try to do some trickery with slicing the existing meta arrays down and matching up against the length of the new data and deleting the ones that are no longer represented. The issue with this is that for these situations where we have duplicated values (I don't know if there's ever a time where that's a valid thing people want to do??) it will wipe out all of the post meta with that same value regardless of how many items there are supposed to be. They will get added back, presumably, but that still seems like a weird routine.
• Something else I haven't managed to think of yet

I guess the biggest question to start is: what kind of situations would there be where the distributed post has meta that the origin post did not have and need to keep?

@tlovett1

[avag-novembit]

Created a PR (https://github.com/10up/distributor/pull/419) where implemented the 'delete meta' functionality. Doing checks during post re-distribute (in the SubscriptionsController::receive_item(..) method) by comparing stored previous request meta data (stored in dt_subscription_update post meta key) versus new one and deleting post meta which existed in previous request but don't exist in the current one.

[arsendovlatyan]

We have discussed this issue with @avag-novembit and I would like to add my thoughts, hopefully it will give more info about our approach and explain PR sent by @avag-novembit.

The basis is following - It's a must and it's safe to delete part of the post (in this case post meta), if it has been added/modified by Distributor. In other words, if will delete part of the post, that has been generated by Distributor, it's OK and expected, but if we will delete non-distributed data, somehow generated in directly destination - it's not OK and unexpected.

For example, we may have a plugin in source and destination, and that plugin can add/update a meta on certain events, it may also delete this meta from all posts on deactivation, now, when we will deactivate this plugin in source and then update a post, Distributor will delete this meta in destination as well and this can generate unpredictable issues, but this issues are expected for destination site maintainer, the same way we could have issues when we were adding meta data from source.

Deletion will be unexpected for maintainer if Distributor was never distributed that data, but all of sudden will decide to delete it. That's why we want to make sure that the most recent update from source brought this data, that we want to delete now.

I'm not sure if we need to compare meta values too, in case of duplicated meta keys it can be really important, but from the other side, we have many filters allowing third party code to extend and modify meta values before they will be inserted into DB (for example our ACF addon will map the meta value, if it contains an attachment ID), however, in my point of view, this data is still added by Distributor and has to be deleted, so, comparison with meta values needs to consider this modifications as well.

[jeffpaul]

I'm moving this to the Future Release milestone to be considered alongside a broad approach to how we handle meta removal/deletion from remote sites. Apologies for the delay on response to this one @arsendovlatyan, but I want to be cautious with how we handle meta deletion.