Skip to:
Content

Opened 9 months ago

Closed 6 months ago

#7329 closed defect (bug) (fixed)

Notice: Trying to get property of non-object

Reported by: 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 9 months ago.
The easiest but not elegant fix of this.
7329.02.patch (4.2 KB) - added by r-a-y 7 months ago.
7329.03.patch (8.3 KB) - added by r-a-y 6 months ago.

Download all attachments as: .zip

Change History (22)

#1 @slaFFik
9 months ago

  • Description modified (diff)

#2 @slaFFik
9 months 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 9 months ago by slaFFik (previous) (diff)

@slaFFik
9 months ago

The easiest but not elegant fix of this.

#3 @slaFFik
9 months ago

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

#4 @DJPaul
8 months ago

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

#5 @slaFFik
8 months 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
8 months ago

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

#7 @DJPaul
8 months 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.


8 months ago

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


8 months ago

#10 @DJPaul
8 months ago

  • Milestone changed from 2.7.3 to 2.7.4

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


8 months ago

#12 @r-a-y
7 months 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 months ago by r-a-y (previous) (diff)

@r-a-y
7 months ago

#13 @DJPaul
7 months ago

  • Milestone changed from 2.7.4 to 2.7.5

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


6 months ago

#15 @slaFFik
6 months ago

  • Milestone changed from 2.7.5 to 2.8

@r-a-y
6 months ago

#16 @r-a-y
6 months 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 6 months ago by r-a-y (previous) (diff)

#17 @r-a-y
6 months 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
6 months 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
6 months ago

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