Skip to content

[3.0] Exclude the post groups already loaded by ID, not by post count - #9414

Merged
Sesquipedalian merged 2 commits into
SimpleMachines:release-3.0from
albertlast:3.0/post-groups-known-ids
Aug 14, 2026
Merged

[3.0] Exclude the post groups already loaded by ID, not by post count#9414
Sesquipedalian merged 2 commits into
SimpleMachines:release-3.0from
albertlast:3.0/post-groups-known-ids

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Description

Group::getPostGroups() collects the post groups it already has in memory:

foreach (self::$loaded as $group) {
	if ($group->min_posts !== -1) {
		self::$post_groups[$group->id] = $group->min_posts;
	}
}

then hands that array straight to the query that fetches the rest:

'SELECT id_group, min_posts
FROM {db_prefix}membergroups
WHERE id_group NOT IN ({array_int:known_post_groups})
	AND min_posts != {int:min_posts}',
[
	'min_posts' => -1,
	'known_post_groups' => !empty(self::$post_groups) ? self::$post_groups : [self::NONE],
]

{array_int:} takes the array's values, so id_group is compared against post counts instead of group IDs.

Two consequences. The groups that were already loaded are not excluded, so they come back from the query and get written into the list a second time — harmless. And any post group whose ID happens to equal another post group's min_posts is excluded when it should not be, so it drops out of the list entirely.

The second one costs a group. Say a forum has a custom post group at ID 9 requiring 4 posts, and it is the one already loaded. The exclusion list becomes [4] instead of [9], so the query is asked for everything except ID 4 — and Newbie, the group for members with no posts, never arrives. Run against the two lists on a forum with that group added:

NOT IN (9)   ->  4,5,6,7,8      the IDs, what the query means
NOT IN (4)   ->  5,6,7,8,9      the post counts, what it passes

array_keys() is the fix.

What I did not verify

I have not produced a member ending up in the wrong group through the UI. It needs a specific arrangement — a post group loaded whose min_posts collides with another post group's ID — and I did not stage one end to end. The defect in the query is plain from the two lines that build the array, and the fix does not change behaviour in any other case: with default groups the current code excludes nothing (no group has ID 0, 50, 100, 250 or 500), so the only difference is that duplicates stop being re-fetched.

Found while chasing an unrelated 500 through User::saveBatch(), which is one of this method's callers.

Issues References (Fixes|Related|Closes)

Related to #7933

getPostGroups() collects the post groups it already has as

	self::$post_groups[$group->id] = $group->min_posts;

then hands that array straight to the query that fetches the rest:

	WHERE id_group NOT IN ({array_int:known_post_groups})

{array_int:} takes the values, so id_group is compared against post counts
rather than group IDs. Two consequences: the groups that were already loaded
are not actually excluded, so they come back and are added a second time, and
any post group whose ID happens to equal another post group's min_posts is
excluded when it should not be, so it drops out of the list entirely.

The second one costs a group. With a custom post group at ID 9 requiring 4
posts already loaded, the query is asked for everything except ID 4, and
Newbie - the group for members with no posts - never arrives:

	NOT IN (9)   ->  4,5,6,7,8      the IDs, what the query means
	NOT IN (4)   ->  5,6,7,8,9      the post counts, what it passes

Signed-off-by: Mathias Papenbrock <mathiaspapealbert@hotmail.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Comment thread Sources/Group.php Outdated
@Sesquipedalian
Sesquipedalian merged commit b8011d9 into SimpleMachines:release-3.0 Aug 14, 2026
4 checks passed
@jdarwood007 jdarwood007 modified the milestones: 3.0 Alpha 6, 3.0 Alpha 5 Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants