Skip to:
Content

BuddyPress.org

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3429 closed defect (bug) (fixed)

BP_Embed tweaks

Reported by: r-a-y Owned by:
Milestone: 1.5 Priority: normal
Severity: minor Version: 1.5
Component: Core Keywords: has-patch
Cc:

Description

While working on my updated oEmbed plugin, I found a few more things that I missed.

Attached patch:

  • Properly caches activity comment embeds (due to the new activity comment template code that I forgot to test for)
  • Fixes notice in BP_Embed::shortcode() and improperly named filter in BP_Embed::parse_oembed()
  • Adds an action to pass the activity ID when expanding the truncated activity entry via AJAX. In this patch, I did not add the code to show the embed after expanding. Let me know if you'd like to see another patch with this included.

Attachments (3)

3429.01.patch (4.0 KB) - added by r-a-y 8 years ago.
3429.02.patch (4.6 KB) - added by r-a-y 8 years ago.
3429.03.patch (6.6 KB) - added by r-a-y 8 years ago.

Download all attachments as: .zip

Change History (12)

@r-a-y
8 years ago

#1 @DJPaul
8 years ago

Regarding bullet point 3 (show embed after expanding); I might be misunderstanding, but doesn't it do it already? See http://testbp.org/members/djpaul/ find the discussion with "This thread, r-a-y!" :)

Last edited 8 years ago by DJPaul (previous) (diff)

#2 @r-a-y
8 years ago

Hmm, you're right... let me try duplicating on a fresh install of BP 1.5-beta-1.

Can you check to see if that embed item is saved in the activity meta table?

@r-a-y
8 years ago

#3 @r-a-y
8 years ago

Just tested.

What you're seeing on testbp.org is the bug in point 2. The $id should be empty for an activity comment or an expanding activity item, but because $id is also defined above the code, BP_Embed thinks that the $id exists, but in actual fact it's a string of the embed handler, which isn't numeric.

The second patch makes this a little clearer.

When you patch just point 2, you'll see that embeds for recursive activity comments and expanded activity items will not show up.

The reason why is recursive activity comments and the expanding activity item do not use a real activity loop ( bp_has_activities() ). The current code in 1.5-beta-1 ( bp_activity_embed() ) only checks if an activity ID exists during the activity loop.

#4 @DJPaul
8 years ago

I'm pretty sure we don't want to put something into trunk that causes a regression on existing functionality, whether that was caused by a bug or not :)

@r-a-y
8 years ago

#5 @r-a-y
8 years ago

Gotcha! New patch adds embed functionality when you expand long activity items via the "Read More" link.

Since this is theme-specific, I put that code in bp-default's functions.php.

#6 @DJPaul
8 years ago

  • Milestone changed from Awaiting Review to 1.5
  • Severity changed from normal to minor

#7 @djpaul
8 years ago

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

(In [4937]) Tweak BP_Embed implementation; fix caching and PHP warning. Fixes #3429, props r-a-y

#8 @DJPaul
8 years ago

Thanks for the patch, r-a-y. I made a couple of minor tweaks: the create_function() didn't work, and I decided to put the stuff you had added to functions.php into Activity core.

#9 @r-a-y
8 years ago

Thanks for adding the $activity object in "bp_dtheme_get_single_activity_content" action. I was about to suggest that!

One tiny thing, instead of using intval() maybe cast it instead?
http://hakre.wordpress.com/2010/05/13/php-casting-vs-intval/

Note: See TracTickets for help on using tickets.