Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.4] Allow boolean keys for Collection::groupBy method #18674

Merged
merged 1 commit into from
Apr 5, 2017
Merged

[5.4] Allow boolean keys for Collection::groupBy method #18674

merged 1 commit into from
Apr 5, 2017

Conversation

linaspasv
Copy link
Contributor

@linaspasv linaspasv commented Apr 5, 2017

The following PR adds support for boolean keys when used in groupBy method of \lluminate\Support\Collection. Related to #18672 issue.

The following PR adds support for boolean keys when used in ``groupBy`` method of ```\lluminate\Support\Collection``. Related to #18672 problem.
@themsaid
Copy link
Member

themsaid commented Apr 5, 2017

I think you can currently do:

$c = collect([
    ['yes' => true],
    ['yes' => false],
    ['yes' => false],
]);

$c->partition('yes');

This will partition the collection into two groups, trues and falses.

@linaspasv
Copy link
Contributor Author

linaspasv commented Apr 5, 2017

@themsaid it's not the case with an example you have provided. If you just took a look at an example I have provided in #18672 issue, where the is_pickup column in a model is being casted from an integer (value type in database) to boolean. At the moment, to make groupBy work, I need to remove boolean cast for that column in a model, as there is no way for groupBy to accept boolean keys. This somehow breaks an overall integrity between eloquent model and collections in this use-case.

p.s. in general PHP you are allowed to have true or false as an array keys, these convert to 1 and 0 accordingly.

I hope you got my point!

@taylorotwell taylorotwell merged commit 3d92f0d into laravel:5.4 Apr 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants