Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#6552 closed defect (bug) (fixed)

XProfile: don't bail BP_XProfile_Group::save() when wpdb returns 0 rows affected

Reported by: Offereins Owned by: boonebgorges
Milestone: 2.4 Priority: normal
Severity: normal Version: 2.3.0
Component: Extended Profile Keywords: has-patch
Cc:

Description

As per r9677 the save() method in class-bp-xprofile-group.php checks for an empty result of wpdb::query(). But unlike how class-bp-xprofile-field.php handles it (compare with !== null*), it just checks for empty(). This creates an issue, which is very aptly described in the inline comments of the latter file:

   /**
    * Check for null so field options can be changed without changing any
    * other part of the field. The described situation will return 0 here.
    */

I guess, since very little users of BP generate meta for XProfile groups, this issue hasn't shown up untill now. Suggested is to follow the logic present in BP_XProfile_Field::save(). Patch is attached.

*) Since wpdb::query() returns false on errors, rather than null, I think we should better check with !== false. Also, can wpdb::query() return a WP_Error? The phpDoc doesn't mention it.

Attachments (1)

6552.patch (1.3 KB) - added by Offereins 5 years ago.

Download all attachments as: .zip

Change History (4)

@Offereins
5 years ago

#1 @DJPaul
5 years ago

Yep. I discovered this very recently, see the patch in #6081 (the rest of that patch relies on this being fixed).

#2 @boonebgorges
5 years ago

  • Milestone changed from Awaiting Review to 2.4

#3 @boonebgorges
5 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 10002:

BP_XProfile_Group::save() should not return false when a save is successful but no rows are updated.

It ought to be semantically possible to save a fieldgroup without making any
changes to the group. Otherwise the return values of the save() method are
inconsistent, and the 'xprofile_group_after_save' hook is not fired reliably.

Props Offereins.
Fixes #6552.

Note: See TracTickets for help on using tickets.