[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            |  Resolution:
 Keywords:                    |
------------------------------+------------------------------
Description changed by boonebgorges:

Old description:

> 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?

New description:

 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#comment:1>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac