cms icon indicating copy to clipboard operation
cms copied to clipboard

[5.x] Adds EntryRepository#findByIds()

Open nadinengland opened this issue 9 months ago • 2 comments

This PR adds findByIds to the EntryRepository contract and Stache implementation. See docs PR.

$items = \Statamic\Facades\Entry::findByIds([ 3, 1, 0, 2 ]);

$this->assertInstanceOf(\Statamic\Entries\EntryCollection::class, $items);
$this->assertCount(3, $items);
$this->assertEveryItemIsInstanceOf(\Statamic\Contacts\Entries\Entry::class);
$this->assertEquals([ 3, 1, 2 ], $items->map->id());

Entries are returned in order of their id's position in the original input argument. Missing IDs are omitted from resultant collection.

Relavancy to other packages

This is part of a wider piece of work I've start to add a fairly large reduction in DB queries to the statamic/eloquent-driver. The principal idea is to route as many components as possible through the repositories. In doing so we can optimise to load directly from the Blink cache, and only defer to the QueryBuilder when we’re missing items.

As I was adding this to the Eloquent Driver package, parity with the core Repository contract felt good as well.

Future considerations

A future goal would be to see the Entries fieldtype use this method and manage the published statuses itself. This could mean we have more occasion to use the blink cache in the eloquent driver solution and also benefit from eager loading see nadinengland/statamic-eloquent-driver#1.

However, this would certainly too much of a breaking change for 5.x, if even desired at all by your team. In the meantime I can just override the behaviour in my projects, but it may be helpful to highlight that approach here for context:

class Entries extends \Statamic\Fieldtypes\Entries
{
    public function augment($values)
    {
        if (config('statamic.system.always_augment_to_query', false)) {
            return parent::augment($values);
        }

        $items = Entry::findByIds($values);

        if ($this->config('max_items') === 1) {
            return $items->first();
        }

        return $items
            ->where(fn ($entry) => $entry->status() === 'published')
            ->values();
    }
}

nadinengland avatar Apr 05 '25 21:04 nadinengland

Apologies, I might be misunderstanding this change, but why can't you do Entry::query()->whereIn('id', [1, 2, 3])->get();?

duncanmcclean avatar May 23 '25 09:05 duncanmcclean

No worries at all. You certainly can do that, but it won't:

  1. Come back in the same order as requested without manually sorting or using OrderedQueryBuilder.
  2. Benefit from the indirection of using the repository pattern, i.e. in the Eloquent Driver, Entry::find() uses the in-memory Blink cache first, as too does my PR for ::findByIds() which loads from Blink and only queries for what is missing.

nadinengland avatar May 25 '25 20:05 nadinengland

Thanks for your patience. I've made a couple of changes:

  • Renamed method to whereInId to match the other whereSomething methods that return EntryCollections. The find methods imply that you get a single Entry.
  • Commented out the method from the interface as that would make it a breaking change. We can introduce it in v6.

At first I was going to suggest that find could just accept an array of IDs like Eloquent does - but I notice that Eloquent doesn't return them in the order you pass it, which is the important part for you here. I'm fine with adding a specialized method in this case.

jasonvarga avatar Sep 24 '25 20:09 jasonvarga

All sounds perfect. Good catch on the find vs where too, certainly something I hadn't considered, but makes total sense.

nadinengland avatar Sep 24 '25 20:09 nadinengland