Skip to:
Content

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#6126 closed defect (bug) (fixed)

Post types activities are not set when directly using BP_Activity_Activity( $id )

Reported by: imath Owned by: djpaul
Milestone: 2.2 Priority: normal
Severity: normal Version:
Component: Activity Keywords: has-patch 2nd-opinion
Cc:

Description

Post types activity actions are not set using the 'bp_register_activity_actions' hook but just when we need them see this comment to understand why we've took this decision.

While working on #6119, i've noticed some unit tests were failing if not running all the tests for instance --group bp_blogs_format_activity_action_new_blog_post. I've found that for these tests we are directly using BP_Activity_Activity( $id ) but as bp_activity_get_actions() has not been called, the post type activities are not set in this case.

I think we should make sure all activity actions are set in BP_Activity_Activity->populate() even if i'm not sure we need it, i think it's safer if someone is directly using BP_Activity_Activity( $id )

Attachments (1)

6126.patch (522 bytes) - added by imath 4 years ago.

Download all attachments as: .zip

Change History (9)

@imath
4 years ago

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


4 years ago

#2 @DJPaul
4 years ago

Sounds like a good catch!

Let's write a test that explicitly unsets $bp->activity->actions just to prove this fix, and get this change in. I can help tomorrow if needed.

#3 @boonebgorges
4 years ago

Yup, sounds good to me.

#4 @djpaul
4 years ago

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

In 9366:

Activity: make sure activity actions are set when creating activity objects.

We can't assume it will already be set because this is normally only done on an ad-hoc basis.
Running certain unit tests in isolation revealed this issue.

Fixes #6126, props imath

#5 @imath
4 years ago

I'm sorry DJPaul, i was a bit long on this one. I've been working on unit tests all day, because i've found that in some tests i wasn't resetting the $bp->activity->actions global the right way which was also explaining why running --group bp_blogs_format_activity_action_new_blog_post was failing while it wasn't when running all tests.
I'll submit a new ticket to fix these tests, but thanks a lot for committing this one :)

#6 follow-up: @boonebgorges
4 years ago

i've found that in some tests i wasn't resetting the $bp->activity->actions global the right way

If you find yourself doing this a lot, consider doing the resetting in the specific class's tearDown(), or even in BP_UnitTestCase::tearDown() (or setUp() I suppose). See how WP does it with post_type, taxonomy, and other globals: https://core.trac.wordpress.org/browser/tags/4.1/tests/phpunit/includes/testcase.php#L22

#7 in reply to: ↑ 6 @imath
4 years ago

Replying to boonebgorges:

If you find yourself doing this a lot, consider doing the resetting in the specific class's tearDown(), or even in BP_UnitTestCase::tearDown() (or setUp() I suppose).

Thanks boonebgorges, now that i've found why i was messing it, i'm going to check in this direction as i think it would be more secure to have this way of resetting the global.

#8 @boonebgorges
4 years ago

In 9382:

In bp_activity_generate_action_string(), use bp_activity_get_actions() rather than touching global directly.

This change ensures that the filters called by bp_activity_get_actions() are
run, which gives a chance for dynamically registered actions to register
themselves just in time whenever an activity string needs to be generated.

This change is a more general fix than the one introduced in [9366], so that
changeset (minus its unit tests) has been rolled back. See #6126.

Fixes #6141.

Note: See TracTickets for help on using tickets.