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.x] Add pluck to query builder #9686

Merged
merged 20 commits into from
Mar 29, 2024
Merged

[5.x] Add pluck to query builder #9686

merged 20 commits into from
Mar 29, 2024

Conversation

JohnathonKoster
Copy link
Contributor

This PR implements pluck method on the query builder.

Note: this implementation currently only resolves values from the Stache indexes, but a fallback to gathering data from entry instances could also be added if desired 🙂

This implementation will resolve values from the Stache indexes
@jasonvarga
Copy link
Member

jasonvarga commented Mar 8, 2024

this implementation currently only resolves values from the Stache indexes

I think this is fine. We could justify it as the same thing happening with Eloquent models and accessors. Calling pluck directly on a query builder only gets the values from the database. Calling pluck on a collection will get accessors too.

class Post extends Model
{
    protected function formattedTitle(): Attribute
    {
        return Attribute::make(
            get: fn ($value, $attributes) => Str::title($attributes['title']),
        );
    }
}

Post::query()->pluck('title'); // ['post one', 'post two']
Post::query()->pluck('formattedTitle'); // QueryException Unknown column formattedTitle
Post::query()->get()->pluck('formattedTitle'); // ['Post One', 'Post Two']

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

There's a bunch of code in this PR that when removed still passes.

If it's necessary, can you please add test coverage?

jasonvarga and others added 12 commits March 26, 2024 09:41
- make getItemValues abstract like getItem is.
- move implementation into BasicStore like getItem
- aggregatestore version delegates to individual stores like getitem
- prevent the query builder from doing Str::after the divider, now that the aggregatestore handles that
- add test to entry query builder test with multiple collections to show it works
- use id- prefix for ids in test to make it more obvious what are keys vs just array indexes. its confusing when debugging.
- make private as its only used in one place now.
- adjust comment
- use 'filter' since 'where' calls filter anyway
@jasonvarga jasonvarga changed the title [5.x] Adds pluck to query builder [5.x] Add pluck to query builder Mar 29, 2024
@jasonvarga jasonvarga merged commit 14da77c into statamic:master Mar 29, 2024
15 checks passed
@jasonvarga jasonvarga deleted the adds-pluck-to-query-builder branch March 29, 2024 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants