fix: resolve string media keys in the glider component - #723
Merged
Conversation
A media id reaches `<x-curator-glider :media="...">` as a string whenever it comes back out of a json column, a settings array or a query string. The constructor only matched `is_int()`, so `"10"` fell through to handleString(), was read as a path with extension `10`, failed the previewable check and rendered the document placeholder instead of the image. Casting to `(int)` at the call site was the reported workaround. A bare key is now looked up rather than taken for a path. The check covers the key shapes curator issues — digits, and uuids/ulids for installs that took the `--use-uuid` option — and a path always carries an extension, so neither shape is ambiguous. `handleInt()` stays as a deprecated delegate to `handleId()`. That placeholder was also mislabelling every file it rendered: document.blade defaults `extension` to `pdf` and glider.blade never passed one, so a .docx came out labelled PDF. The extension now travels on MediaDTO from all three sources (record, path, fallback), and the default is gone. Also reported: a checkout that never ran `curator:install` has no Glide token, and `GlideManager::getToken()` failed with a bare return type error. It now names the missing variable and the command that generates it. Fixes #719 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #719.
1. The reported bug:
:media="$image"renders a placeholder,:media="(int) $image"renders the imageThe reporter had it right in their third comment.
Glider::__construct()only matchedis_int(), so a string key fell through tohandleString(), which reads the value as a path:isPreviewable("10")is false, so the component rendered the document placeholder for a perfectly good image. Their data is['banners' => ["10", "7"]]in a json settings column — no relationship or model override involved, which is what wasn't adding up in the thread. Any id that round-trips through json, a query string or a settings array arrives as a string.A bare key is now looked up instead of being taken for a path:
isMediaKey()matches the key shapes curator issues — digits, plus uuids and ulids, sincecurator:install --use-uuidwrites$table->uuid('id')->primary(). A path always carries an extension, so the two shapes don't overlap;'10.jpg','banner.jpg'and'media/2024/10'all still take the path branch, and there are tests pinning that.handleInt()is kept as a deprecated delegate to the widenedhandleId(), so anything extending the component keeps working. A string key that doesn't resolve reaches the fallback, same as an int that doesn't.2. The placeholder labels everything "PDF"
Worth fixing while in here, and it's half of what the reporter was looking at.
resources/views/components/display/document.blade.phpdeclares'extension' => 'pdf'as its prop default, andglider.blade.phpnever passed one — so every non-previewable file the glider rendered came out labelled PDF, and the video branch in that component was unreachable. (display/index.blade.phpwas already doing this correctly with$item->ext.)MediaDTOnow carriesext, populated from all three sources — record, path, and fallback (via itstype(), falling back to the source's extension) — and the hardcoded default is gone. A.docxnow renders labelleddocx.3. The Glide token error
Docs for this landed in dafd5c2 already, so this is only the error itself.
GlideManager::getToken(): stringreturnedconfig('curator.glide_token')straight, so a checkout that never rancurator:installgot:It now names the missing variable and the command that generates it. This is exactly the reporter's "1 pc has it, the other doesn't" situation.
Tests
tests/src/Feature/Components/GliderMediaKeyTest.php— 16 cases covering key resolution, the path branch staying put, fallback on an unresolvable string key, extension propagation from each source, and a rendered-html assertion that the placeholder saysdocxand notpdf. All but the "a path is still treated as a path" no-regression guard fail on5.x.composer testis green: 218 passed, PHPStan clean, no Pint or Rector drift.🤖 Generated with Claude Code