[buddypress-trac] [BuddyPress Trac] #8732: Member-type-specific XProfile fields override 'exclude_fields' parameter in `bp_has_profile()` stack

[buddypress-trac]

#8732: Member-type-specific XProfile fields override 'exclude_fields' parameter in
`bp_has_profile()` stack
------------------------------+-----------------------------
 Reporter:  boonebgorges      |      Owner:  (none)
     Type:  defect (bug)      |     Status:  new
 Priority:  normal            |  Milestone:  Awaiting Review
Component:  Extended Profile  |    Version:
 Severity:  normal            |   Keywords:
------------------------------+-----------------------------
 Imagine a setup where you have a member type `'foo'` and a profile field
 `123` that's associated with the member type.

 Then, run the following sort of query:

 {{{#!php
 <?php
 $profile_args = [
   'member_type' => 'foo',
   'exclude_fields' => [ 123 ],
 ];

 if ( bp_has_profile( $profile_args ) ) {
 ...
 }}}

 You'll end up with SQL that looks like this, in
 `BP_XProfile_Group::get_group_field_ids()`:

 `SELECT id FROM wp_bp_xprofile_fields WHERE ... id NOT IN (123) AND id IN
 (123)...`

 As a result, field `123` *will appear*` in the results (`IN` has
 precedence over `NOT`; see https://dev.mysql.com/doc/refman/5.7/en
 /operator-precedence.html)

 This feels counterintuitive to me. Explicit exclusion via `exclude_fields`
 should take precedence over implicit inclusion via `member_type`.
 Moreover, in the current setup, there's no reliable way to exclude `123`,
 except via filter.

 The problematic SQL is generated at
 https://buddypress.trac.wordpress.org/browser/tags/10.4.0/src/bp-
 xprofile/classes/class-bp-xprofile-group.php?marks=554,573-586#L533. I
 think the proper fix is probably something like this:

 1. When querying for member-type-linked fields, save them in a separate
 `member_type_fields` variable.
 2. Compare this field to those fields passed into `exclude_fields`
 3. Remove any matches from `$member_type_fields`
 4. Only then can you merge `$member_type_fields` into
 `$include_field_ids`.

 I can write a patch, but first:
 a. Do others agree with my intuition that the current behavior is
 incorrect?
 b. Does my proposed updated logic seem correct?

-- 
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/8732>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac