Skip to content

Conversation

dweidner
Copy link

Changelog

🐛 Bug fixes

Keep original case intact when grouping a collection by callable. See issue #7562.

@afbora afbora linked an issue Aug 20, 2025 that may be closed by this pull request
@afbora
Copy link
Member

afbora commented Aug 20, 2025

@dweidner Can you add a unit test for this improvement, please? I can take care if you're not available.

@afbora afbora added this to the 5.1.1 milestone Aug 21, 2025
@dweidner
Copy link
Author

Sorry, it took me a while to find the time. I will add some tests today. Thank you for considering my pull request.

@dweidner
Copy link
Author

Sorry for the noise. Was struggeling with the prettier configuration in VSCode. Should be ready now.

@dweidner dweidner requested a review from afbora August 26, 2025 10:15
afbora
afbora previously approved these changes Aug 26, 2025
Copy link
Member

@afbora afbora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 👍

Optional note: IMHO It's not a big deal, but in unit tests, counting the results of ->group() to check how many results the grouping explicitly returns increases the accuracy of the unit test.

@dweidner
Copy link
Author

Optional note: IMHO It's not a big deal, but in unit tests, counting the results of ->group() to check how many results the grouping explicitly returns increases the accuracy of the unit test.

Was considering this as well, but was not sure whether this should be part of this particular unit test.

@afbora
Copy link
Member

afbora commented Aug 26, 2025

What I mean is that the group size changes with sensitive and insensitive grouping. You do this with the key control. We can also make the unit test more meaningful by checking the group size. Something like that:

public function testGroupByCallableCaseSensitive(): void
{
	$collection = new Collection();

	$collection->taylor = [
		'name' => 'Taylor',
		'genre' => 'Pop',
	];

	$collection->justin = [
		'name' => 'Justin',
		'genre' => 'Pop',
	];

	$collection->aubrey = [
		'name' => 'Aubrey',
		'genre' => 'Hip-Hop',
	];

	$collection->kayne = [
		'name' => 'kayne',
		'genre' => 'hip-hop',
	];

	$groupsCaseInsensitive = $collection->group(fn (array $item) => $item['genre'], true);

	$this->assertCount(2, $groupsCaseInsensitive);
	$this->assertTrue($groupsCaseInsensitive->has('pop'));
	$this->assertTrue($groupsCaseInsensitive->has('hip-hop'));


	$groupsCaseSensitive = $collection->group(fn (array $item) => $item['genre'], false);

	$this->assertCount(3, $groupsCaseSensitive);
	$this->assertTrue($groupsCaseSensitive->has('Pop'));
	$this->assertTrue($groupsCaseSensitive->has('Hip-Hop'));
}

@dweidner
Copy link
Author

That makes perfect sense. Thank you for clarifying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Case preferences are ignored when grouping collections by callable
3 participants