RicLeP / laravel-storyblok

Make Laravel and Storyblok work together beautifully.
https://ls.sirric.co.uk/
MIT License
58 stars 15 forks source link

Cache ignores $page in paginated fetching #27

Closed chrisk-7777 closed 2 years ago

chrisk-7777 commented 2 years ago

Thanks for the awesome library.

Referring to line: https://github.com/RicLeP/laravel-storyblok/blob/master/src/Folder.php#L165

If we have the following two pages we get the same results back, the second from the cache.

// example.org/some-content?page=1
$requestSomeContent = new Folder();
$requestSomeContent->slug('some-content');
$requestSomeContent->perPage(10);
$requestSomeContent->settings([
    'is_startpage' => false,
    'filter_query' => [],
]);

// yields cache key:
// folder-some-content-4893ccccb8f122d63a21febbbb000000

// example.org/some-content?page=2
$requestSomeContent = new Folder();
$requestSomeContent->slug('some-content');
$requestSomeContent->perPage(10);
$requestSomeContent->settings([
  'is_startpage' => false,
  'filter_query' => [],
]);

// yields cache key:
// folder-some-content-4893ccccb8f122d63a21febbbb000000

Note on the second request (page=2), the cache key constructed doesn't account for the current page which gives it the same key, meaning we get the cached data back.

I believe the cache key should include $page in the cache key if set.

If I disable the cache (STORYBLOK_CACHE=false) it works as expected. Looking through telescope I can confirm the cache keys aren't accounting for $page.

RicLeP commented 2 years ago

Hi @chrisk-7777, I’m happy you’re liking the library.

Folders don’t know what page they’re on - you need to tell them in the controller. I don’t think I am covering that in the docs, it’s only mentioned in the Storyblok docs - page parameter.

<?php

namespace App\Http\Controllers;

use App\Storyblok\Folders\News;
use Illuminate\Http\Request;

class NewsController extends Controller
{
    public function index(Request $request)
    {
        $news = new News();
        $news->settings([
            'page' => (int)$request->get('page') ?: 1,
        ]);

        return view('pages.news', [
            'stories' => $news->perPage(14)->read(),
        ]);
    }
}

Doing this should work, or you could extend App\Storyblok\Folder and check the page there.

I think I might add resolving pages automatically to the package it future - they can look up the page.

chrisk-7777 commented 2 years ago

Hey RicLeP thanks for the speedy response.

Can confirm that approach works, wasn't aware $page could be passed through as a setting attribute. Which in turn is correctly serialized. Winner 👍

RicLeP commented 2 years ago

I've improved the docs as it wasn't clear, happy to have assisted!