Cannot modify $allowedtags per xProfile field type

[needle]

I've written a plugin that creates a custom field type and have discovered that I cannot modify $allowedtags per xProfile field type, but only globally.

To allow this, it seems to me that $this->field_id and not $this->id should be passed to the xprofile_data_value_before_save filter in BP_XProfile_ProfileData->save(), given that xprofile_sanitize_data_value_before_save() expects the second parameter to be $field_id. However, this value is not used in the function, nor is it passed on to subsequent filters where it could be used to discover the field type.

I'm attaching a patch which would allow the field type to be discoverable by passing the $field_id onwards.

[boonebgorges]

I like the idea of this patch, and am happy to do something to make it easier for plugins to adjust the allowedtags array on a more case-by-case basis, but I have a few concerns about this patch as it stands.

• Except in extreme circumstances, we prefer not to change the values that are passed to filters. Doing so will break existing plugins that use these filters, often in unpredictable and harmful ways (think what could happen if the new field_id happens to match the id of an existing xprofile data item). So my inclination is to pass a third value to the 'xprofile_data_value_before_save' filter. (And, while we're at it, the rest of these '_before_save' filters too.)
• In your case, you want to filter based on the field type. But others might want to filter based on the user ID, or the value, or something else. So let's pass the entire field object instead of just $field_id.
• If we go with my suggested change (add a third value to the filter), we can't just do this to pass the value to the callback:

add_filter( 'xprofile_data_value_before_save',          'xprofile_sanitize_data_value_before_save', 1, 3 );

because xprofile_sanitize_data_value_before_save() already takes a third parameter (the somewhat odd $reserialize

This leaves us with two options:

1. Build a new wrapper, and hook to that instead. Something like:

function xprofile_sanitize_value_before_save_callback( $field_value, $data_id, $data_object ) {
    return xprofile_sanitize_value_before_save( $field_value, $data_obj->field_id, $reserialize, $data_object ); // adding a new fourth param to the original function
}
add_filter( 'xprofile_data_value_before_save', 'xprofile_sanitize_value_before_save_callback', 10, 3 );

2. Just add the $data_object as a fourth param to xprofile_sanitize_value_before_save(), and pass a true value to the callback via add_filter():

class BP_XProfile_ProfileData {
    // ...
    public function save() {
         // ...
         $this->value = apply_filters( 'xprofile_data_value_before_save', $this->value, $this->id, true, $this );
    }
}

plus the necessary changes to xprofile_data_value_before_save

I'm leaning toward 2 as the less intrusive option. needle, can you double check my logic and let me know what you think?

[DJPaul]

Boone's 2nd idea sounds good.

[needle]

Thanks for the feedback Boone. Sorry for the delay - was off work last week.

I appreciate the need to keep filter signatures the same for existing plugins but I cannot seem to find a situation where $this->id actually has a value on save. I therefore suspect it would be safe to replace it with $this->field_id which is populated. Nevertheless, I've created a new patch to reflect your concerns - as you rightly say, passing $this is ultimately more flexible and your structure neatly sidesteps the issue.

[djpaul]

In 8665:

xProfile: add extra arguments to xprofile_data_value_before_save and xprofile_filtered_data_value_before_save filters.

This allows plugin developers greater control over filtering profile data in specific circumstances, particularly for custom field types.

Fixes #5742, props needle.