Skip to content

[12.x] Allow rendering paginated data where cursor parameters are not in the response #55524

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

Open
wants to merge 1 commit into
base: 12.x
Choose a base branch
from

Conversation

cosmastech
Copy link
Contributor

@cosmastech cosmastech commented Apr 23, 2025

If you attempt to render a CursorPaginator, but do not have the keys/aliases as properties on the underlying data, a warning is raised. On our production environments, this returns a 500.

Take for instance something like this:

User::query()
    ->orderBy('created_at', 'asc')
    ->cursorPaginate(10)
    ->through(function(User $user) {
        return [
            'uuid' => $user->uuid,
            'name' => $user->name,
        ];
    })
    ->toArray();

I do not want to expose the integer ID nor the created at timestamps in my response, but this will produce a warning. The reason the warning is raised is because through transforms the items, but AbstractCursorPagination::getParametersForItem() looks to find the cursor parameters as properties on the modified data.

With this PR's changes, I can now do this:

User::query()
    ->orderBy('created_at', 'asc')
    ->cursorPaginate(10)
    ->setTransformer(function(User $user) {
        return [
            'uuid' => $user->uuid,
            'name' => $user->name,
        ];
})->toArray();

This is particularly meaningful if I want to transform my data into Responsable DTOs (with something Laravel Data). While Laravel-Data does offer a paginated data collection class, it's not necessarily as easy to work with as the one that comes with the framework.
If I have a Responsable object like this:

class UserResponse extends Data
{
    public function __construct(
        public string $uuid,
        public string $name
    ) { }

    public static function fromModel(User $user): self
    {
        return new self($user->uuid, $user->name);
    }
}

Then cursor pagination will fail because there's no id field in the UserResponse.

Other possibilites

Change the through method so it does not immediately act on the collection, but instead only does it at toArray() time. This would potentially be a breaking change for users that relied on this behavior.

Additionally, the signature of through could be changed to:

public function through(callable $callback, bool $onlyBeforeRender = false)

Though this is a breaking change for anyone who has extended CursorPaginator. We could take the pseudo-argument approach where we check for a second argument being passed in, but not change the signature.


We could also create the cursor if one isn't set when the paginator is constructed. That could be done inside of setItems or directly after it. I am starting to think that this is cleaner.

Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@cosmastech cosmastech changed the title [12.x] Allow transforming cursor paginated data before rendering [12.x] Allow rendering paginated data where cursor parameters are not in the response Apr 23, 2025
@cosmastech cosmastech marked this pull request as ready for review April 23, 2025 20:01

return $item;
});

$this->assertInstanceOf(CursorPaginator::class, $p);
$this->assertSame([['id' => 6], ['id' => 7]], $p->items());
$this->assertSame([['zid' => 6], ['zid' => 7]], $p->items());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: If I were to call $p->toArray() it would produce a warning.

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.

1 participant