fix: Suppressed the warning for breadcrumbOnly items#25
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the navigation breadcrumb URL generation to avoid emitting debug warnings when generating URLs for breadcrumbOnly items that may not have resolvable route parameters in the current request context.
Changes:
- Extended
Navigation::resolveRoute()with a$silentflag to optionally suppress debug logging on route generation failures. - Updated breadcrumb URL building to call
resolveRoute(..., silent: true)forbreadcrumbOnlyitems (especially those using wildcard params), falling back to'#'without logging. - Added clarifying inline comments explaining why failures are expected for certain ancestor breadcrumb items.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -358,17 +358,21 @@ private function routeMatches( | |||
| * | |||
| * @param array<string, mixed> $params | |||
There was a problem hiding this comment.
The resolveRoute() docblock only documents $params, but the method now accepts a $silent flag that changes behavior (suppresses debug logging on route resolution failures). Please update the PHPDoc to include and describe the $silent parameter so static analysis and IDE hints stay accurate.
| * @param array<string, mixed> $params | |
| * @param array<string, mixed> $params | |
| * @param bool $silent Whether to suppress debug logging when route resolution fails. |
| // For breadcrumb-only items with wildcards, use current route params. | ||
| // Silently fall back to '#' when params can't be resolved — these are ancestor | ||
| // items in a breadcrumb chain whose required params aren't present in the | ||
| // current route, which is expected and not a real error. | ||
| if ($item->breadcrumbOnly && $item->params !== null) { | ||
| $resolvedParams = $this->resolveWildcardParams($item->params, $routeParams); | ||
| $breadcrumbItem['url'] = $this->resolveRoute($item->route, $resolvedParams); | ||
| $breadcrumbItem['url'] = $this->resolveRoute($item->route, $resolvedParams, silent: true); | ||
| } else { | ||
| $breadcrumbItem['url'] = $this->resolveRoute($item->route, $routeParams); | ||
| $breadcrumbItem['url'] = $this->resolveRoute($item->route, $routeParams, silent: $item->breadcrumbOnly); |
There was a problem hiding this comment.
This change intentionally suppresses route error logging for breadcrumbOnly breadcrumb URLs (via silent: true / silent: $item->breadcrumbOnly), but there’s no automated test asserting that Log::warning is not emitted in debug mode for breadcrumbOnly items when URL generation fails (e.g., missing required params causing UrlGenerationException). Add a unit test that mocks Log and verifies breadcrumb generation returns '#' without logging for breadcrumbOnly items.
No description provided.