Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 7 years ago

#7329 closed defect (bug) (fixed)

Notice: Trying to get property of non-object

Reported by: slaffik's profile slaFFik Owned by:
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.7
Component: Administration Keywords: has-patch
Cc:

Description (last modified by slaFFik)

Posted a comment to post, after that opened Activity management page in wp-admin area - saw this notice:

Notice: Trying to get property of non-object in \wp-content\plugins\buddypress\src\bp-activity\bp-activity-template.php on line 2254

Current BuddyPress trunk

Attachments (3)

7329.patch (2.5 KB) - added by slaFFik 7 years ago.
The easiest but not elegant fix of this.
7329.02.patch (4.2 KB) - added by r-a-y 7 years ago.
7329.03.patch (8.3 KB) - added by r-a-y 7 years ago.

Download all attachments as: .zip

Change History (22)

#1 @slaFFik
7 years ago

  • Description modified (diff)

#2 @slaFFik
7 years ago

That's because $activities_template global in bp_activity_get_comment_depth() is empty, because we don't have it in admin area.

Last edited 7 years ago by slaFFik (previous) (diff)

@slaFFik
7 years ago

The easiest but not elegant fix of this.

#3 @slaFFik
7 years ago

  • Keywords 2nd-opinion has-patch added; needs-patch removed

#4 @DJPaul
7 years ago

Is this a regression introduced in 2.7? Did it work in 2.6?

#5 @slaFFik
7 years ago

@DJPaul
I was not able to replicate on WordPress 4.5.x and 4.6.x and BP 2.6.x.
Once I updated to BuddyPress 2.7.2 on Plugins page - instantly got this notice.

So yes, this is a regression in 2.7.

#6 @DJPaul
7 years ago

In future patches, try to keep unrelated code improvements (all the spacing) outside of bug fixes.

#7 @DJPaul
7 years ago

I did some git bisect (best tool on the planet) and discovered this was caused in 2.7 by r10947. @r-a-y do you have time to look at this patch and see if it's a good fix?

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


7 years ago

This ticket was mentioned in Slack in #buddypress by tw2113. View the logs.


7 years ago

#10 @DJPaul
7 years ago

  • Milestone changed from 2.7.3 to 2.7.4

This ticket was mentioned in Slack in #buddypress by slaffik. View the logs.


7 years ago

#12 @r-a-y
7 years ago

  • Version set to 2.7

Thanks @slaFFik for discovering this bug and sorry for getting to this kinda late.

The main thing we need to address here is finding the current activity comment depth when we're not in the activity comment loop.

02.patch is an attempt at this.

This is what is happening in the patch:

  • When we pass $comment to bp_activity_get_comment_depth(), we pull up the root activity update and the entire activity comment tree.
  • Next, we try to iterate over the entire tree to find the current comment. Once we've found the comment, we use the calculated depth from BP_Activity_Activity:get_activity_comments(). This can be kind of intensive depending on how many comments are in the tree.
  • Lastly, since activity items in the Activity dashboard are displayed in stream mode, we need to iterate the depth number so bp_blogs_can_comment_reply() knows when to disable activity comment replies. This is a little bit of a hack, but if we want to avoid this, we can add a conditional so this is only done in the WP admin dashboard. Or, we can also iterate the depth in bp_blogs_can_comment_reply(). I think this makes more sense.
Last edited 7 years ago by r-a-y (previous) (diff)

@r-a-y
7 years ago

#13 @DJPaul
7 years ago

  • Milestone changed from 2.7.4 to 2.7.5

This ticket was mentioned in Slack in #buddypress by slaffik. View the logs.


7 years ago

#15 @slaFFik
7 years ago

  • Milestone changed from 2.7.5 to 2.8

@r-a-y
7 years ago

#16 @r-a-y
7 years ago

  • Keywords 2nd-opinion removed

03.patch includes unit tests and cleans up the code a bit.

Patch also fixes an issue in the admin area with activity comments not being restricted by depth for regular activity types like 'activity_update'. See the code in /bp-activity/classes/class-bp-activity-list-table.php. This part of the code could be cleaned up a bit due to the check for the Blogs component since regular activity updates are not reliant on the Blogs component, but I'm going to leave it as-is for now.

Last edited 7 years ago by r-a-y (previous) (diff)

#17 @r-a-y
7 years ago

In 11413:

Activity: Add parameter to bp_activity_get_comment_depth().

Previously, bp_activity_get_comment_depth() only supported looking for an
activity comment's depth during a queried activity comment loop.

This commit allows a developer to pass either a comment object or an
activity ID to the function so the activity comment depth can be fetched
outside the activity comment loop.

This fixes an issue with a notice displaying in the BuddyPress Activity
admin dashboard.

See #7329.

#18 @r-a-y
7 years ago

In 11414:

Activity Admin: Ensure activity comment depth check is respected.

For blog activity comment replies, we need to change the operator to
"greater or equal to" for the depth check to be accurate, due to activity
items being displayed in stream mode.

For regular activity comment replies, we need to add our activity comment
reply check to BP_Activity_List_Table::can_comment(). This can be
further optimized by moving this out of the Blogs component check in a
later release.

See #7329.

#19 @r-a-y
7 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.