[buddypress-trac] [BuddyPress Trac] #5919: Xprofile field meta values not deleted with field
noreply at wordpress.org
Sat Oct 4 01:31:02 UTC 2014
#5919: Xprofile field meta values not deleted with field
Reporter: tometzky | Owner:
Type: defect (bug) | Status: new
Priority: high | Milestone: 2.2
Component: XProfile | Version: 2.1
Severity: major | Resolution:
Keywords: needs-patch |
Changes (by boonebgorges):
* keywords: needs-patch good-first-bug => needs-patch
* priority: low => high
* severity: minor => major
* milestone: Future Release => 2.2
tometzky - Thanks for the constructive feedback.
> When there's no meta values to delete, then this function returns a
value which is ==false. It should return a ==true value.
WP's `delete_metadata()` returns `false` when there is no meta to delete.
We are following this convention.
> But this causes that there can be no "highly targeted substring
replacements", as relevant strings are moving targets.
The string replacement was designed to apply very narrowly to a small
number of queries. Your examples show that it's not focused enough - thank
you very much for bringing these sorts of situations to our attention.
> And all this so Wordpress functions like update_meta could be used on
different table than wp_postmeta or wp_usermeta. When simply using
$wpdb->insert() and $wpdb->update() would be much, much simpler and
See https://buddypress.trac.wordpress.org/ticket/4551 for some background
on the decision to move to the WP functions. Short version: previously we
had our own versions of these functions that did not use the WP functions.
It resulted in a huge amount of highly fractured code, inconsistency,
incomplete implementation of caching, and other problems. Making the move
to the WP API was a trade-off: we got rid of the vast majority of this
code (a good thing) but had to do a couple of oddball workarounds to get
it to work (the SQL filtering you refer to). So, I agree with you that
this is not ideal - and this ticket is making me consider how we can
propose some upstream changes that will allow us to phase out our hacks -
but I'm not as confident as you that the old way was much simpler and more
robust. I would be glad to have a continued discussion about the wisdom of
the path we've taken, but we are stuck with the current system at least in
the short term, so I would like to come up with a fix for the problems
you've raised that doesn't require refactoring the entire codebase.
So, down to the task at hand. Given that you don't like what we're doing
here in the first place, you might want to close your eyes for the rest of
this update :) I've written a patch that modifies our SQL filter
callbacks. Instead of blanket calls to `str_replace()`, it does the
* Uses a regular expression to locate all quoted content. This will
include all content (meta_key, meta_value) provided by the user.
* Replaces each instance with a placeholder.
* Does the existing `str_replace()` logic.
* Swaps the placeholders out with the original quoted content.
Unit tests are included to demonstrate how various kinds of quoted content
is blocked from causing the problems that tometzky raises.
I'm going to hijack this ticket for the purposes of fixing this issue.
Once it's solved, we can get back to the original problem: deleting
xprofilemeta when the field is deleted.
Thanks again for your help, tometzky.
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5919#comment:5>
BuddyPress Trac <http://buddypress.org/>
More information about the buddypress-trac