Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

#5083 closed defect (bug) (fixed)

Activity Meta doesn't recognize 0 as a valid value

Reported by: saurabhshukla Owned by: saurabhshukla
Milestone: 1.9 Priority: low
Severity: minor Version:
Component: Core Keywords: needs-patch
Cc:

Description

A meta key not existing and a meta key with a value of 0 are two different things and this distinction can be useful at times.

I attempted storing a privacy meta as 0 and I could never check for it. I wanted to distinguish between activities that have a deliberate privacy set to 0 and activities that never had any privacy set.

Can't do that anyway I tried, so I had to set it as -1 which makes me do extra bending around. This is in contrast to how WordPress post meta is stored and retrieved.

In fact I was mapping posts to activities and also mapping the corresponding metas.

The empty() check in the meta functions is the culprit. Are we open to replace this approach with something else?

Change History (7)

#1 @saurabhshukla
6 years ago

  • Keywords dev-feedback added
  • Owner set to saurabhshukla
  • Status changed from new to assigned

#2 @r-a-y
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from assigned to closed

Don't use an empty check.

Use a strict comparison check.

Like:

// check if meta is integer 0
if ( $meta === 0 ) {

See if that works for you.

I'm closing this because this doesn't sound like a bug with BP. But do feel to continue posting to see if what I stated works for you or not.

If it does end up being a BP problem, I'll reopen.

#3 @saurabhshukla
6 years ago

I'm not the one using a check. BuddyPress checks for empty($activity_meta) and hence I can't use 0 as a value for meta. As soon as I get my hands off rtMedia, I'll submit a patch.

in function bp_activity_update_meta() in bp-activity-functions.php

if ( empty( $meta_value ) )
		return bp_activity_delete_meta( $activity_id, $meta_key );

should've been

if ( $meta_value===false )
		return bp_activity_delete_meta( $activity_id, $meta_key );

If I try and store 0 as a value, the key gets deleted, offsetting the architecture of the code that I'm trying to write.

In fact, I don't see the sense in this code, even with the strict comparison. If I pass a false value for a meta (which could also be legitimate), the key itself gets deleted.

Hope, I've come across more clearly this time.

#4 @saurabhshukla
6 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

#5 @boonebgorges
6 years ago

  • Keywords needs-patch added; dev-feedback removed
  • Milestone set to 1.9
  • Priority changed from normal to low
  • Severity changed from normal to minor

saurabhshukla is correct that this is an issue. See #3139 for the corresponding report in the Groups component.

I don't mind putting in a fix for this particular issue, though it will require a once-over of the codebase to make sure that we're not breaking anything. I'll put this in the 1.9 milestone as a preliminary measure.

Also related: #3907, which proposes turning our custom meta functions into wrappers for update_metadata(), etc.

#6 @r-a-y
6 years ago

I'm not the one using a check. BuddyPress checks for empty($activity_meta) and hence I can't use 0 as a value for meta.

Good call, saurabhshukla. I should have looked through the codebase before posting!

#7 @boonebgorges
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 7576:

Allow 0 to be stored in activitymeta

Previously, any value where empty( $value ) evaluated to true would result
in the deletion of the row. This change allows for 0 to be stored as a
legitimate value.

This is a stopgap until BP's meta functions are refactored to use WP's core
*_metadata() functions.

Fixes #5083

Note: See TracTickets for help on using tickets.