Repository navigation
fix(images): resolve nested definitions and versions through their relations - #178
Open
ericwang401 wants to merge 1 commit into
Open
ericwang401 wants to merge 1 commit into
ericwang401 wants to merge 1 commit into
Conversation
…lations
The admin API is registered with scopeBindings(), which resolves a child
route parameter through a parent relation named after it: `{image_definition}`
under `{image_group}` through ImageGroup::imageDefinitions(), and
`{image_version}` under `{image_definition}` through
ImageDefinition::imageVersions(). Neither exists -- the relations are
definitions() and versions() -- so every route below
/image-groups/{group}/images/{definition} answered 500, taking the Versions
dialog, image edit and version publishing with it.
Map those two parameters onto the existing relations, and cover the routes
over HTTP, including a child requested under the wrong parent still 404ing.
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.
Every admin route under
/image-groups/{group}/images/{definition}answered 500:The admin API is registered with
scopeBindings(), which resolves a child route parameter through a parent relation named after it.{image_definition}under{image_group}therefore goes throughImageGroup::imageDefinitions(), and{image_version}under{image_definition}throughImageDefinition::imageVersions(). Neither exists; the relations aredefinitions()andversions(). This broke the image card's Versions dialog (the version list and Publish version), image Edit, and the definition/version endpoints on/api/application.Fix
ImageGroupandImageDefinitionoverridechildRouteBindingRelationshipName()to map those two parameters onto the existing relations. The routes, their parameter names and the Wayfinder keys the frontend uses are unchanged, and so is scoping: a child requested under the wrong parent still 404s.Tests
tests/Feature/Images/ImageRouteBindingTest.phpcovers the routes over HTTP. Nothing did before, which is how this got through:All four fail without the model change (the 404 cases 500). The full Pest suite passes (1149 tests), and pint and phpstan are clean.
The Playwright suite in #179 found this, and its catalogue specs now read definitions and versions through these routes.