Cockpit-HQ / Cockpit

Cockpit Core - Content Platform
https://getcockpit.com
Other
388 stars 47 forks source link

Incosistent items API in Content Module #200

Open Tiefseetauchner opened 3 months ago

Tiefseetauchner commented 3 months ago

Howdy! I noticed that the code in modules/Content/api.php is incosistent in the value it returns:

$items = $app->module('content')->items($model, $options, $process);
if (count($items)) {
    $app->trigger('content.api.items', [&$items, $model]);
}
if (isset($options['skip'], $options['limit'])) {
    return [
        'data' => $items,
        'meta' => [
            'total' => $app->module('content')->count($model, $options['filter'] ?? [])
        ]
    ];
}
return $items;

In case a skip and limit are set, it just completely changes the API. Dynamically. This makes it extraordinarily hard to use the pagination feature (which btw, I love that it's a thing in the first place), especially as this is entirely undocumented. The first time I learned that this is a behavior was when my app suddenly started crashing when I implemented pagination.

I would like to fix this. Either by documenting it in the openapi docs or by changing the interface (scary).

I'm gonna make a PR for the first option soon but I wanted to have a better place for discussion so if you say "Yes, let us change the interface" I'll make a new branch and new PR (as I'll use the prior option in my usecase for now and I don't need cutting edge currently)

aheinze commented 3 months ago

Thank you for providing a PR for OpenAPI. The reasoning behind it is based on discussions I had for the previous Cockpit version (<v2). People wanted a simplified response structure (just the content items).

But I also get your concerns. A solution could be an additional parameter to force a specific response data structure 🤔