fix(#3265781): add resource type route default for single-bundle views - #14
fix(#3265781): add resource type route default for single-bundle views#14Decipher wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughGenerated JSON:API view routes now resolve bundle resource types. Single-bundle views receive a singular ChangesView route resource type
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.x-1.x #14 +/- ##
===========================================
+ Coverage 94.11% 94.28% +0.16%
===========================================
Files 3 3
Lines 136 140 +4
===========================================
+ Hits 128 132 +4
Misses 8 8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
18d585a to
7eb9fda
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Decipher
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Routing/Routes.php`:
- Around line 80-85: Update the route-building logic around the
bundle_resource_types mapping to remove unresolved NULL ResourceType results
before constructing resource_types and route defaults. Skip the route when no
bundle resource types resolve, and ensure single-bundle resource_type handling
never assigns NULL; add a Kernel test covering a view whose bundle resource type
is missing.
In `@tests/src/Kernel/JsonapiViewsResourceKernelTest.php`:
- Line 315: Update the assertion for JsonApiRoutes::RESOURCE_TYPE_KEY in the
multi-bundle route test to use hasDefault() and assert the default is absent,
rather than relying on getDefault() returning NULL. Preserve the existing route
setup and other assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 55d5c820-ad18-4c4c-8d89-fd0cce38ca93
📒 Files selected for processing (3)
AGENTS.mdsrc/Routing/Routes.phptests/src/Kernel/JsonapiViewsResourceKernelTest.php
| $bundle_resource_types = array_map(fn(int|string $bundle) => $this->resourceTypeRepository->get($entity_type, $bundle), $bundles); | ||
|
|
||
| if (empty($resource_types)) { | ||
| if (empty($bundle_resource_types)) { | ||
| continue; | ||
| } | ||
| $resource_types = array_map(fn(?ResourceType $resource_type) => $resource_type?->getTypeName(), $bundle_resource_types); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env sh
set -eu
# Run the Kernel suite after adding an unresolved-ResourceType regression case.
make test-kernelRepository: Decipher/jsonapi_views
Length of output: 202
🏁 Script executed:
#!/usr/bin/env sh
set -eu
printf 'Repository files matching Routes.php / Resource type files:\n'
git ls-files | rg '(^|/)(Routes\.php|.*ResourceType.*\.php)$' || true
printf '\nRoutes.php outline:\n'
ast-grep outline src/Routing/Routes.php || true
printf '\nRelevant Routes.php lines:\n'
cat -n src/Routing/Routes.php | sed -n '1,140p'
printf '\nSearch for resource type repository interfaces/classes and tests mentioning _jsonapi_resource_types:\n'
rg -n "ResourceTypeRepository|resourceTypeRepository|ResourceTypeRepositoryInterface|resource_type|resource_types|_jsonapi_resource_types|routes\(\)" src tests phpunit.xml makefile Makefile -S . || trueRepository: Decipher/jsonapi_views
Length of output: 9336
🌐 Web query:
Drupal jsonapi ResourceTypeRepositoryInterface get() return type NULL get($entity_type_id, $bundle)
💡 Result:
The get method of the Drupal JSON:API ResourceTypeRepositoryInterface returns either an instance of \Drupal\jsonapi\ResourceType\ResourceType or NULL [1][2]. Specifically, the method is defined to return the requested JSON:API resource type if it exists, and NULL if it does not [2][3]. The method signature accepts two parameters: $entity_type_id (the entity type ID) and $bundle (the ID for the bundle, or the entity type ID if the entity does not have a bundle) [2][4]. Note that this interface is marked as @internal, meaning it is not part of the stable PHP API and may change at any time [3]. Developers are advised that the JSON:API module's primary API is the HTTP API [3].
Citations:
- 1: https://api.drupal.org/api/drupal/core%21modules%21jsonapi%21src%21ResourceType%21ResourceTypeRepositoryInterface.php/function/ResourceTypeRepositoryInterface%3A%3Aget/11.x
- 2: https://api.drupal.org/api/drupal/core%21modules%21jsonapi%21src%21ResourceType%21ResourceTypeRepositoryInterface.php/interface/ResourceTypeRepositoryInterface/main
- 3: https://api.drupal.org/api/drupal/core%21modules%21jsonapi%21src%21ResourceType%21ResourceTypeRepositoryInterface.php/main
- 4: https://api.drupal.org/api/drupal/core%21modules%21jsonapi%21src%21ResourceType%21ResourceTypeRepositoryInterface.php/function/ResourceTypeRepositoryInterface%3A%3Aget/9
🏁 Script executed:
#!/usr/bin/env sh
set -eu
printf 'Kernel test relevant section:\n'
cat -n tests/src/Kernel/JsonapiViewsResourceKernelTest.php | sed -n '260,380p'
printf '\nRepository files for tests and config:\n'
git ls-files tests src vendor composer.json composer.lock phpunit.xml.* Makefile makefile AGENTS.md | sed -n '1,200p'
printf '\nDRUSH/MODULE commands availability:\n'
command -v drush || true
command -v php || true
php -v || trueRepository: Decipher/jsonapi_views
Length of output: 5814
Skip routes with unresolved bundle resource types.
ResourceTypeRepositoryInterface::get() can return NULL; with at least one bundle present, empty($bundle_resource_types) is FALSE, and the route gets NULL values in _jsonapi_resource_types. A single-bundle route can set resource_type to NULL as well. Filter resolved resource types before building route defaults, and add a Kernel test for a view whose bundle resource type does not exist.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Routing/Routes.php` around lines 80 - 85, Update the route-building logic
around the bundle_resource_types mapping to remove unresolved NULL ResourceType
results before constructing resource_types and route defaults. Skip the route
when no bundle resource types resolve, and ensure single-bundle resource_type
handling never assigns NULL; add a Kernel test covering a view whose bundle
resource type is missing.
Source: Coding guidelines
7eb9fda to
9e0f9d0
Compare
1 participant
Fixes #3265781.
jsonapi_views-generated routes never carried the
resource_typeroute default that route/resource introspection code (for example the OpenAPI module's JSON:API discovery) reads to resolve a route to a singleResourceType.This does not use the community patch's/MR !18's approach. Both the original 2022 patch and the 2026 MR add
new ResourceType('view', 'view', ViewsResource::class)- a fake type unrelated to the view's real entity type/bundle(s). I installeddrupal/openapi_jsonapiand called its generator directly against a real site: that fake type produces zero-schema, generically-tagged ("View") OpenAPI entries for every exposed view, regardless of its real bundle(s). It avoids a crash but replaces it with silently wrong documentation. I also found the originally reported crash no longer reproduces against currentopenapi/openapi_jsonapireleases at all - their own null-guard (added 2020, ingetPaths()) already excludes routes with noresource_typedefault rather than erroring.Instead: a view can span multiple bundles (
_jsonapi_resource_types, plural, already reflects this), so a singleresource_typedefault only has one unambiguous answer when the view's entity type has exactly one bundle. For that case, this sets the realResourceTypealready returned byresourceTypeRepository->get()- no fabrication needed. For a view spanning several bundles, the default is left unset rather than reporting a made-up type.Verified manually against the real
openapi/openapi_jsonapimodules (not just the Kernel test assertions): a single-bundle diagnostic view resolves a real schema ($ref: #/definitions/user--user); a multi-bundle diagnostic view is correctly excluded.Adds Kernel test coverage for both the single-bundle and multi-bundle cases.
Verified on Drupal 10 and 11 locally via
make test-kernelandmake lint.Summary by CodeRabbit
Bug Fixes
Tests
Documentation