-
-
Notifications
You must be signed in to change notification settings - Fork 299
feat: Replace Fractal with Laravel Data (Phase 1) #2448
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| <?php | ||
|
|
||
| namespace App\Data; | ||
|
|
||
| use App\Models\ApiKey; | ||
|
|
||
| class ApiKeyData extends Data | ||
| { | ||
| /** | ||
| * @param array<int, string>|null $allowed_ips | ||
| */ | ||
| public function __construct( | ||
| public string $identifier, | ||
| public ?string $description, | ||
| public ?array $allowed_ips, | ||
| public ?string $last_used_at, | ||
| public string $created_at, | ||
| ) {} | ||
|
|
||
| public static function fromModel(ApiKey $model): self | ||
| { | ||
| return new self( | ||
| identifier: $model->identifier, | ||
| description: $model->memo, | ||
| allowed_ips: $model->allowed_ips, | ||
| last_used_at: $model->last_used_at?->toAtomString(), | ||
| created_at: $model->created_at->toAtomString(), | ||
| ); | ||
| } | ||
|
|
||
| public function getResourceName(): string | ||
| { | ||
| return static::getResourceNameStatic(); | ||
| } | ||
|
|
||
| public static function getResourceNameStatic(): string | ||
| { | ||
| return ApiKey::RESOURCE_NAME; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| <?php | ||
|
|
||
| namespace App\Data; | ||
|
|
||
| use Spatie\LaravelData\CursorPaginatedDataCollection as SpatieCursorPaginatedDataCollection; | ||
|
|
||
| class CursorPaginatedDataCollection extends SpatieCursorPaginatedDataCollection | ||
| { | ||
| protected bool $isFractal = false; | ||
|
|
||
| public function setFractal(bool $value = true): static | ||
| { | ||
| $this->isFractal = $value; | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** @var array<string, mixed> */ | ||
| protected array $_additional = []; | ||
|
|
||
| /** | ||
| * @param array<string, mixed> $additional | ||
| */ | ||
| public function additional(array $additional): static | ||
| { | ||
| $this->_additional = array_merge($this->_additional, $additional); | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| public function toArray(): array | ||
| { | ||
| $array = parent::toArray(); | ||
| if ($this->isFractal) { | ||
| $resourceKey = method_exists($this->dataClass, 'getResourceNameStatic') | ||
| ? ($this->dataClass)::getResourceNameStatic() | ||
| : strtolower(class_basename($this->dataClass)); | ||
|
|
||
| $formattedData = []; | ||
| $dataItems = $array['data'] ?? []; | ||
| foreach ($dataItems as $item) { | ||
| if (is_array($item)) { | ||
| unset($item['_additional']); | ||
| } | ||
| $formattedData[] = [ | ||
| 'object' => $resourceKey, | ||
| 'attributes' => $item, | ||
| ]; | ||
| } | ||
|
|
||
| $paginator = $this->items; | ||
| $cursor = [ | ||
| 'current' => $paginator->cursor() ? $paginator->cursor()->encode() : null, | ||
| 'prev' => $paginator->previousCursor() ? $paginator->previousCursor()->encode() : null, | ||
| 'next' => $paginator->nextCursor() ? $paginator->nextCursor()->encode() : null, | ||
| 'count' => $paginator->count(), | ||
| ]; | ||
|
|
||
| $meta = $array['meta'] ?? []; | ||
| unset($meta['path'], $meta['per_page'], $meta['next_page_url'], $meta['prev_page_url']); | ||
|
|
||
| if (isset($this->_additional['meta'])) { | ||
| $meta = array_merge($meta, $this->_additional['meta']); | ||
| } | ||
|
|
||
| $meta['cursor'] = $cursor; | ||
|
|
||
|
Comment on lines
+53
to
+61
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the target file and nearby context
printf '\n== CursorPaginatedDataCollection.php ==\n'
cat -n app/Data/CursorPaginatedDataCollection.php | sed -n '1,160p'
# Look for related cursor/meta handling in the repo
printf '\n== Search for cursor meta handling ==\n'
rg -n "next_cursor|prev_cursor|cursor" app src . -g '!vendor' -g '!node_modules' || true
# Check composer constraints for Laravel version clues
printf '\n== composer.json ==\n'
cat -n composer.json | sed -n '1,220p'Repository: pelican-dev/panel Length of output: 50375 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Determine whether the codebase vendors Laravel paginator classes or references their array shape.
printf '\n== Search for CursorPaginator usage ==\n'
rg -n "CursorPaginator|cursor paginator|toArray\(\)" . -g '!vendor' -g '!node_modules' || true
# If a lockfile exists, inspect installed Laravel component versions.
printf '\n== composer.lock Laravel packages ==\n'
if [ -f composer.lock ]; then
rg -n '"name": "illuminate/(pagination|support|contracts|collections|container)|"version":' composer.lock | sed -n '1,120p'
fiRepository: pelican-dev/panel Length of output: 50374 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '== composer.json relevant deps ==\n'
python3 - <<'PY'
import json
from pathlib import Path
data = json.loads(Path('composer.json').read_text())
for section in ('require', 'require-dev'):
deps = data.get(section, {})
for name in ('laravel/framework', 'illuminate/pagination', 'spatie/laravel-data'):
if name in deps:
print(f'{section}: {name} {deps[name]}')
PY
printf '\n== composer.lock relevant package versions ==\n'
python3 - <<'PY'
import json
from pathlib import Path
p = Path('composer.lock')
if not p.exists():
print('composer.lock missing')
raise SystemExit(0)
data = json.loads(p.read_text())
wanted = {'laravel/framework', 'illuminate/pagination', 'spatie/laravel-data'}
for pkg in data.get('packages', []) + data.get('packages-dev', []):
if pkg.get('name') in wanted:
print(pkg['name'], pkg.get('version'))
PY
printf '\n== repository references to CursorPaginator ==\n'
rg -n "CursorPaginator|next_cursor|prev_cursor" app config database routes tests .github . -g '!vendor' -g '!node_modules' | sed -n '1,120p'Repository: pelican-dev/panel Length of output: 751 🌐 Web query:
💡 Result: In Laravel 13.19, the Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '== app/Data/Data.php ==\n'
cat -n app/Data/Data.php | sed -n '1,220p'
printf '\n== Cursor collection inheritance points ==\n'
rg -n "CursorPaginatedDataCollection|PaginatedDataCollection|toArray\(\): array|additional\(array" app/Data -nRepository: pelican-dev/panel Length of output: 4231 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '== app/Data/Data.php ==\n'
cat -n app/Data/Data.php | sed -n '1,220p'
printf '\n== app/Data directory outline ==\n'
find app/Data -maxdepth 1 -type f | sortRepository: pelican-dev/panel Length of output: 2985 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '== app/Data/PaginatedDataCollection.php ==\n'
cat -n app/Data/PaginatedDataCollection.php | sed -n '1,220p'
printf '\n== app/Data/DataCollection.php ==\n'
cat -n app/Data/DataCollection.php | sed -n '1,220p'Repository: pelican-dev/panel Length of output: 5761 🌐 Web query:
💡 Result: In the spatie/laravel-data package, CursorPaginatedDataCollection is designed to handle cursor-based pagination for data objects [1][2]. When you call toArray on a CursorPaginatedDataCollection, it transforms the underlying CursorPaginator instance into an array representation [3][4]. Regarding the specific metadata keys next_cursor and prev_cursor: 1. Standard Laravel Behavior: The underlying Laravel CursorPaginator class includes next_cursor and prev_cursor in its own toArray method, along with next_page_url and prev_page_url [4]. 2. Spatie Implementation: Spatie's CursorPaginatedDataCollection essentially wraps this cursor-paginated data [3][1]. When converted to an array, the metadata structure typically mirrors the underlying paginator's output, which includes the cursor information necessary for navigation [4]. 3. Key Clarification: While the package provides the cursor values in the metadata [4], note that in some contexts (like TypeScript transformer generation), there has been historical confusion regarding the naming of these properties compared to standard pagination links [5]. However, when using the standard toArray method, the structure returned by the underlying Laravel CursorPaginator—containing next_cursor and prev_cursor—is preserved within the meta key of the final array output [4]. If you find that these keys are missing in your specific output, verify that you are interacting with the collection as a resource, as Spatie's data objects are designed to automatically include this pagination metadata when returned in responses [6]. Citations:
🌐 Web query:
💡 Result: When using CursorPaginatedDataCollection in spatie/laravel-data, calling the toArray method results in a structure that includes the paginated data items and a meta key containing pagination information [1]. This meta object incorporates cursor-based pagination fields, including next_cursor and prev_cursor [2][1]. The underlying behavior is driven by the Laravel CursorPaginator [3][2]. When transformed to an array, the meta section typically includes the following [2][1]: - data: The collection of transformed data objects [2][1]. - meta: - next_cursor: The encoded string representing the cursor for the next page (or null) [2][1]. - prev_cursor: The encoded string representing the cursor for the previous page (or null) [2][1]. - next_page_url: The URL to the next page [2][1]. - prev_page_url: The URL to the previous page [2][1]. - path: The base path for the paginator [2][1]. - per_page: The number of items per page [2][1]. Note that while these fields exist within the Laravel CursorPaginator implementation [2], some users have noted that when using the spatie/laravel-data package, this information is structured under the meta key [1]. The next_cursor and prev_cursor values are strings generated by the underlying Laravel framework's cursor encoding [2][1]. Citations:
Strip 🤖 Prompt for AI Agents |
||
| return [ | ||
| 'object' => 'list', | ||
| 'data' => $formattedData, | ||
| 'meta' => $meta, | ||
| ]; | ||
| } | ||
|
|
||
| return $array; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| <?php | ||
|
|
||
| namespace App\Data; | ||
|
|
||
| use Illuminate\Contracts\Pagination\CursorPaginator; | ||
| use Illuminate\Contracts\Pagination\Paginator; | ||
| use Illuminate\Pagination\AbstractCursorPaginator; | ||
| use Illuminate\Pagination\AbstractPaginator; | ||
| use Spatie\LaravelData\Data as SpatieData; | ||
|
|
||
| abstract class Data extends SpatieData | ||
| { | ||
| public static string $_collectionClass = DataCollection::class; | ||
|
|
||
| public static string $_paginatedCollectionClass = PaginatedDataCollection::class; | ||
|
|
||
| public static string $_cursorPaginatedCollectionClass = CursorPaginatedDataCollection::class; | ||
|
|
||
| protected bool $isFractal = false; | ||
|
|
||
| public function setFractal(bool $value = true): static | ||
| { | ||
| $this->isFractal = $value; | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| public static function collection(mixed $items): DataCollection|PaginatedDataCollection|CursorPaginatedDataCollection | ||
| { | ||
| if ($items instanceof Paginator || $items instanceof AbstractPaginator) { | ||
| /** @var PaginatedDataCollection $collection */ | ||
| $collection = static::collect( | ||
| $items, | ||
| static::$_paginatedCollectionClass | ||
| ); | ||
|
|
||
| return $collection; | ||
| } | ||
|
|
||
| if ($items instanceof CursorPaginator || $items instanceof AbstractCursorPaginator) { | ||
| /** @var CursorPaginatedDataCollection $collection */ | ||
| $collection = static::collect( | ||
| $items, | ||
| static::$_cursorPaginatedCollectionClass | ||
| ); | ||
|
|
||
| return $collection; | ||
| } | ||
|
|
||
| /** @var DataCollection $collection */ | ||
| $collection = static::collect( | ||
| $items, | ||
| static::$_collectionClass | ||
| ); | ||
|
|
||
| return $collection; | ||
| } | ||
|
|
||
| public function toArray(): array | ||
| { | ||
| $array = parent::toArray(); | ||
| if ($this->isFractal) { | ||
| $resourceKey = method_exists($this, 'getResourceName') | ||
| ? $this->getResourceName() | ||
| : strtolower(class_basename($this)); | ||
|
|
||
| $additional = $this->getAdditionalData(); | ||
| foreach ($additional as $key => $value) { | ||
| unset($array[$key]); | ||
| } | ||
|
Comment on lines
+43
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Inspect vendor source (if present) for the real additional-data accessor.
fd -t d 'laravel-data' vendor/spatie 2>/dev/null
rg -n 'function getAdditional|protected array \$additional|function additional\(' vendor/spatie/laravel-data/src 2>/dev/nullRepository: pelican-dev/panel Length of output: 155 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Find the package version pinned in the repo and any local references to the accessor.
rg -n '"spatie/laravel-data"|spatie/laravel-data' composer.json composer.lock 2>/dev/null || true
rg -n 'getAdditionalData|additionalData|setFractal|toArray\(' app src tests 2>/dev/null || trueRepository: pelican-dev/panel Length of output: 17360 🌐 Web query:
💡 Result: In spatie/laravel-data, the ability to use the additional method—and by extension manage additional data—is provided by the AppendableData trait [1]. The specific trait is: Spatie\LaravelData\Concerns\AppendableData [1] This trait is one of the standard concerns implemented by the base Data class [2]. It enables the additional(['key' => 'value']) method, which allows you to append extra properties to your data objects or collections when they are transformed into a resource [3][1][4]. While your query specifically mentions getAdditionalData, please note that the public API provided by the AppendableData trait is primarily additional [1][4]. If you are looking for how the data is stored or retrieved internally, you should inspect the source code of the AppendableData trait within the package's src/Concerns directory [2][1]. As of version 4.23.0, there are no breaking changes or new features introduced that modify the core functionality of the AppendableData trait [5][6]. The version 4.23.0 release focused on validation rule overrides and other bug fixes [5][6]. Citations:
🌐 Web query:
💡 Result: In the context of the spatie/laravel-data package, AppendableData refers to a trait and contract that provides functionality for appending extra properties to data objects when they are transformed [1][2]. The package does not define a specific method named getAdditionalData that returns an AppendableData object. Instead, the package provides an additional method on data objects to add extra properties to the transformed output [3]. Usage Example: SongData::from(Song::first)->additional([ 'year' => 1987, ]); The AppendableData trait is implemented by the base Data class, allowing data objects to include supplemental data alongside their primary attributes when serialized [2]. While you can use additional to pass an array of data, the internal mechanisms for handling these properties are managed by the AppendableData trait and the underlying transformation logic [3][2]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- app/Data/Data.php ---'
cat -n app/Data/Data.php
echo
echo '--- project references to getAdditionalData ---'
rg -n 'function getAdditionalData|getAdditionalData\(' app tests src 2>/dev/null || true
echo
echo '--- related data classes ---'
for f in app/Data/DataCollection.php app/Data/PaginatedDataCollection.php app/Data/CursorPaginatedDataCollection.php; do
echo "### $f"
cat -n "$f"
echo
doneRepository: pelican-dev/panel Length of output: 11285 🌐 Web query:
💡 Result: The code snippet Citations:
Replace
🧰 Tools🪛 PHPMD (2.15.0)[warning] 52-52: Avoid unused local variables such as '$value'. (undefined) (UnusedLocalVariable) 🤖 Prompt for AI Agents |
||
|
|
||
| $response = [ | ||
| 'object' => $resourceKey, | ||
| 'attributes' => $array, | ||
| ]; | ||
|
|
||
| if (isset($additional['meta'])) { | ||
| $response['meta'] = $additional['meta']; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| return $response; | ||
| } | ||
|
|
||
| return $array; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| <?php | ||
|
|
||
| namespace App\Data; | ||
|
|
||
| use Spatie\LaravelData\DataCollection as SpatieDataCollection; | ||
|
|
||
| class DataCollection extends SpatieDataCollection | ||
| { | ||
| protected bool $isFractal = false; | ||
|
|
||
| public function setFractal(bool $value = true): static | ||
| { | ||
| $this->isFractal = $value; | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** @var array<string, mixed> */ | ||
| protected array $_additional = []; | ||
|
|
||
| /** | ||
| * @param array<string, mixed> $additional | ||
| */ | ||
| public function additional(array $additional): static | ||
| { | ||
| $this->_additional = array_merge($this->_additional, $additional); | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| public function toArray(): array | ||
| { | ||
| $array = parent::toArray(); | ||
| if ($this->isFractal) { | ||
| $resourceKey = method_exists($this->dataClass, 'getResourceNameStatic') | ||
| ? ($this->dataClass)::getResourceNameStatic() | ||
| : strtolower(class_basename($this->dataClass)); | ||
|
|
||
| $formattedData = []; | ||
| foreach ($array as $item) { | ||
| // If the item itself has additional data merged, remove it from attributes | ||
| if (is_array($item)) { | ||
| unset($item['_additional']); | ||
| } | ||
| $formattedData[] = [ | ||
| 'object' => $resourceKey, | ||
| 'attributes' => $item, | ||
| ]; | ||
| } | ||
|
|
||
| $response = [ | ||
| 'object' => 'list', | ||
| 'data' => $formattedData, | ||
| ]; | ||
|
|
||
| if (isset($this->_additional['meta'])) { | ||
| $response['meta'] = $this->_additional['meta']; | ||
| } | ||
|
|
||
| return $response; | ||
| } | ||
|
|
||
| return $array; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| <?php | ||
|
|
||
| namespace App\Data; | ||
|
|
||
| use Illuminate\Pagination\LengthAwarePaginator; | ||
| use Spatie\LaravelData\PaginatedDataCollection as SpatiePaginatedDataCollection; | ||
|
|
||
| class PaginatedDataCollection extends SpatiePaginatedDataCollection | ||
| { | ||
| protected bool $isFractal = false; | ||
|
|
||
| public function setFractal(bool $value = true): static | ||
| { | ||
| $this->isFractal = $value; | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** @var array<string, mixed> */ | ||
| protected array $_additional = []; | ||
|
|
||
| /** | ||
| * @param array<string, mixed> $additional | ||
| */ | ||
| public function additional(array $additional): static | ||
| { | ||
| $this->_additional = array_merge($this->_additional, $additional); | ||
|
|
||
| return $this; | ||
| } | ||
|
Comment on lines
+19
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Same This override doesn't delegate to any parent Also applies to: 25-91 🤖 Prompt for AI Agents |
||
|
|
||
| public function toArray(): array | ||
| { | ||
| $array = parent::toArray(); | ||
| if ($this->isFractal) { | ||
| $resourceKey = method_exists($this->dataClass, 'getResourceNameStatic') | ||
| ? ($this->dataClass)::getResourceNameStatic() | ||
| : strtolower(class_basename($this->dataClass)); | ||
|
|
||
| $formattedData = []; | ||
| $dataItems = $array['data'] ?? []; | ||
| foreach ($dataItems as $item) { | ||
| if (is_array($item)) { | ||
| unset($item['_additional']); | ||
| } | ||
| $formattedData[] = [ | ||
| 'object' => $resourceKey, | ||
| 'attributes' => $item, | ||
| ]; | ||
| } | ||
|
|
||
| $paginator = $this->items; | ||
| if (!$paginator instanceof LengthAwarePaginator) { | ||
| throw new \InvalidArgumentException('Paginator must be an instance of LengthAwarePaginator'); | ||
| } | ||
|
|
||
| $pagination = [ | ||
| 'total' => $paginator->total(), | ||
| 'count' => $paginator->count(), | ||
| 'per_page' => $paginator->perPage(), | ||
| 'current_page' => $paginator->currentPage(), | ||
| 'total_pages' => $paginator->lastPage(), | ||
| 'links' => [], | ||
| ]; | ||
| if ($paginator->nextPageUrl()) { | ||
| $pagination['links']['next'] = $paginator->nextPageUrl(); | ||
| } | ||
| if ($paginator->previousPageUrl()) { | ||
| $pagination['links']['previous'] = $paginator->previousPageUrl(); | ||
| } | ||
|
|
||
| $meta = $array['meta'] ?? []; | ||
| unset( | ||
| $meta['current_page'], | ||
| $meta['first_page_url'], | ||
| $meta['from'], | ||
| $meta['last_page'], | ||
| $meta['last_page_url'], | ||
| $meta['next_page_url'], | ||
| $meta['path'], | ||
| $meta['per_page'], | ||
| $meta['prev_page_url'], | ||
| $meta['to'], | ||
| $meta['total'] | ||
| ); | ||
|
|
||
| // Merge custom additional meta | ||
| if (isset($this->_additional['meta'])) { | ||
| $meta = array_merge($meta, $this->_additional['meta']); | ||
| } | ||
|
|
||
| $meta['pagination'] = $pagination; | ||
|
|
||
| return [ | ||
| 'object' => 'list', | ||
| 'data' => $formattedData, | ||
| 'meta' => $meta, | ||
| ]; | ||
| } | ||
|
|
||
| return $array; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Same
additional()bypass asDataCollection.php/PaginatedDataCollection.php.Metadata passed to
->additional([...])is only surfaced whenisFractalis true. Outside fractal mode (return $array;, Line 68), it is silently dropped. Same underlying issue and suggested fix as noted onapp/Data/DataCollection.php.Also applies to: 25-69
🤖 Prompt for AI Agents