Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

#5983 closed defect (bug) (fixed)

groups can be created with creator_id = 0

Reported by: DJPaul Owned by: djpaul
Milestone: 2.2 Priority: normal
Severity: normal Version:
Component: Groups Keywords:
Cc:

Description

groups_create_group will accept a creator_id of 0. This causes problems in places where BuddyPress expects the creator_id to be set to a valid user (i.e. everywhere).

This was observed in the wild in a unit test, test_total_group_count_groups_accept_membership_request, which recently started failing when tested against WordPress core trunk. I have been unable to identify the commits in WP and BP which caused the test to fail.

https://travis-ci.org/buddypress/BuddyPress/jobs/39704710

There was 1 error:
1) BP_Tests_Groups_Functions::test_total_group_count_groups_accept_membership_request
Trying to get property of non-object
/tmp/wordpress/src/wp-content/plugins/BuddyPress/src/bp-groups/bp-groups-notifications.php:115
/tmp/wordpress/src/wp-content/plugins/BuddyPress/src/bp-groups/bp-groups-functions.php:1221
/tmp/wordpress/src/wp-content/plugins/BuddyPress/tests/phpunit/testcases/groups/functions.php:107
/home/travis/.phpenv/versions/5.2.17/bin/phpunit:46
FAILURES!
Tests: 666, Assertions: 1101, Errors: 1, Skipped: 9.

Change History (4)

#1 @djpaul
6 years ago

In 9117:

Unit Tests, Groups: update the Group factory to handle false return values from groups_create_group.

It currently assumes that the group is always succesfully created, and this isn't true if creator_id is set to 0 (or if there is no logged in user).

See #5983

#2 @djpaul
6 years ago

In 9118:

Unit Tests, Groups: test group creation for authenticated vs anonymous user.

These will fail until I get a subsequent commit in; I wanted these in first so we can easily see if the fix works as expected.

See #5983

#3 @djpaul
6 years ago

In 9120:

Unit Tests, Groups: update the group factory to set the group's creator_id if one was not passed to the factory.

This is a quicker way of updating hundreds of unit tests where we've previously had a bug that was creating Groups without a valid group creator being set. See #5983 for related changes.

A few tests are counting the number of users in specific circumstances, so those eight tests have been individually updated so that they pass a valid creator_id without us having to refactor the entire test.

The test_group_has_no_members test has also been removed as it is impossible for a group to have no members. This test used to work because of this bug.

#4 @djpaul
6 years ago

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

In 9121:

Groups, Unit Tests: a creator_id must be passed to groups_create_group to create a group.

Some of our unit tests were built to assume that they would always run with an authenticated user, but obviously this was incorrect. As a result, Groups were being created with creator_id=0, which was breaking other parts of the codebase, for certain tests.

This change also removes the test_creating_new_group_as_anonymous_user test that I added in r9118 which is now unnecessary.

See r9117, r9118, r9120 for other parts of this fix.
Fixes #5983 and the unit tests. :)

Note: See TracTickets for help on using tickets.